johno/beads
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Close bd-224: status/closed_at invariant fully enforced

Browse Source
This commit is contained in:
Steve Yegge
2025-10-15 18:03:58 -07:00
parent 737f354066
commit e4394df604
1 changed files with 1 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
.beads/issues.jsonl
View File

@@ -137,7 +137,7 @@
{"id":"bd-221","title":"P0: Transaction state corruption in first fix attempt","description":"First attempt at fixing bd-89 had a critical flaw: used 'ROLLBACK; BEGIN IMMEDIATE' which executed as two separate statements. After ROLLBACK, the Go tx object was invalid but continued to be used, causing undefined behavior.\n\nRoot cause: database/sql connection pooling. Without acquiring a dedicated connection, subsequent queries could use different connections from the pool, breaking the transaction.\n\nCorrect fix: Use conn := s.db.Conn(ctx) to acquire a dedicated connection, then execute BEGIN IMMEDIATE, all operations, and COMMIT on that single connection.\n\nThis bug was caught during code review and fixed before merging.","status":"closed","priority":0,"issue_type":"bug","created_at":"2025-10-15T01:18:10.027168-07:00","updated_at":"2025-10-15T16:27:21.985806-07:00","closed_at":"2025-10-15T01:18:16.200472-07:00"}
{"id":"bd-222","title":"P2: Consider batching API for bulk issue creation","description":"Current CreateIssue acquires a dedicated connection for each call. For bulk imports or agent workflows creating many issues, a batched API could improve performance:\n\nCreateIssues(ctx, issues []*Issue, actor string) error\n\nThis would:\n- Acquire one connection\n- Use one IMMEDIATE transaction\n- Insert all issues atomically\n- Reduce connection overhead\n\nNot urgent - current approach is correct and fast enough for typical use.","status":"open","priority":2,"issue_type":"feature","created_at":"2025-10-15T01:18:46.4512-07:00","updated_at":"2025-10-15T16:27:21.98622-07:00"}
{"id":"bd-223","title":"P3: Consider early context check in CreateIssue","description":"CreateIssue starts an IMMEDIATE transaction before checking if context is cancelled. If a client cancels early, we've already acquired the write lock.\n\nConsider adding:\n if err := ctx.Err(); err != nil {\n return err\n }\n\nBetween validation (line 318) and acquiring connection (line 331).\n\nLow priority - the busy_timeout and deferred cleanup handle this gracefully, but an early check would be slightly more efficient.","status":"open","priority":3,"issue_type":"task","created_at":"2025-10-15T01:20:18.796324-07:00","updated_at":"2025-10-15T16:27:21.986637-07:00"}
{"id":"bd-224","title":"Data model allows inconsistent status/closed_at states","description":"Issue bd-89 demonstrates a data model inconsistency: an issue can have status='open' but also have a closed_at timestamp set. This creates a liminal state that violates the expected invariant that closed_at should only be set when status='closed'.\n\nRoot causes:\n1. Import (bd import) updates status field independently from closed_at field\n2. UpdateIssue allows status changes without managing closed_at\n3. No database constraint enforcing the invariant\n4. Export includes both fields independently in JSONL\n\nCurrent behavior:\n- bd close: Sets status='closed' AND closed_at (correct)\n- bd update --status open: Sets status='open' but leaves closed_at unchanged (creates inconsistency)\n- bd import: Can import inconsistent data from JSONL\n\nImpact:\n- 'bd ready' shows issues that appear closed (have closed_at)\n- Confusing for users and downstream tools\n- Stats may be inaccurate\n\nPotential solutions:\nA) Add CHECK constraint: (status = 'closed') = (closed_at IS NOT NULL)\nB) Update import/update logic to enforce invariant in application code\nC) Add a 'reopened' event that explicitly clears closed_at\nD) Remove closed_at field entirely (calculate from events or use status only)\n\nSee bd-89 for concrete example.","notes":"Label management fully implemented with comprehensive test coverage (10 test cases). Provides foundation for better data organization and filtering.","status":"in_progress","priority":1,"issue_type":"bug","created_at":"2025-10-15T01:36:21.971783-07:00","updated_at":"2025-10-15T17:51:09.108813-07:00","dependencies":[{"issue_id":"bd-224","depends_on_id":"bd-222","type":"blocks","created_at":"2025-10-15T14:22:17.171659-07:00","created_by":"stevey"}]}
{"id":"bd-224","title":"Data model allows inconsistent status/closed_at states","description":"Issue bd-89 demonstrates a data model inconsistency: an issue can have status='open' but also have a closed_at timestamp set. This creates a liminal state that violates the expected invariant that closed_at should only be set when status='closed'.\n\nRoot causes:\n1. Import (bd import) updates status field independently from closed_at field\n2. UpdateIssue allows status changes without managing closed_at\n3. No database constraint enforcing the invariant\n4. Export includes both fields independently in JSONL\n\nCurrent behavior:\n- bd close: Sets status='closed' AND closed_at (correct)\n- bd update --status open: Sets status='open' but leaves closed_at unchanged (creates inconsistency)\n- bd import: Can import inconsistent data from JSONL\n\nImpact:\n- 'bd ready' shows issues that appear closed (have closed_at)\n- Confusing for users and downstream tools\n- Stats may be inaccurate\n\nPotential solutions:\nA) Add CHECK constraint: (status = 'closed') = (closed_at IS NOT NULL)\nB) Update import/update logic to enforce invariant in application code\nC) Add a 'reopened' event that explicitly clears closed_at\nD) Remove closed_at field entirely (calculate from events or use status only)\n\nSee bd-89 for concrete example.","notes":"Label management fully implemented with comprehensive test coverage (10 test cases). Provides foundation for better data organization and filtering.","status":"closed","priority":1,"issue_type":"bug","created_at":"2025-10-15T01:36:21.971783-07:00","updated_at":"2025-10-15T17:53:30.166936-07:00","closed_at":"2025-10-15T17:53:30.166936-07:00","dependencies":[{"issue_id":"bd-224","depends_on_id":"bd-222","type":"blocks","created_at":"2025-10-15T14:22:17.171659-07:00","created_by":"stevey"}]}
{"id":"bd-225","title":"Ultrathink: Choose solution for status/closed_at inconsistency (bd-224)","description":"Deep analysis of solution options for bd-224 data model inconsistency issue.\n\nContext:\n- Target users: individual devs and small teams\n- Future: swarms of agent workers\n- Brand-new codebase with few users (can break things)\n- Issue: status='open' with closed_at!=NULL creates liminal state\n\nOptions to evaluate:\nA) Database CHECK constraint\nB) Application-level enforcement \nC) Add explicit reopened event\nD) Remove closed_at field entirely\n\nAnalysis framework:\n1. Simplicity (mental model for devs)\n2. Robustness (hard to break, especially for agent swarms)\n3. Migration cost (schema changes, data cleanup)\n4. Future extensibility\n5. Performance\n6. Consistency guarantees\n\nNeed to determine best approach given tradeoffs.","notes":"Analysis complete. Recommendation: Hybrid approach with DB CHECK constraint + smart UpdateIssue + import enforcement + reopen command.\n\nKey insights:\n- DB constraint provides defense-in-depth perfect for agent swarms\n- Statistics calculation currently uses 'closed_at IS NOT NULL' which is BROKEN by inconsistent data\n- UpdateIssue and Import don't manage the invariant\n- EventReopened exists but is unused\n\nSee ULTRATHINK_BD224.md for full analysis.","status":"closed","priority":1,"issue_type":"task","created_at":"2025-10-15T01:47:25.564925-07:00","updated_at":"2025-10-15T16:27:21.989017-07:00","closed_at":"2025-10-15T01:49:43.078431-07:00","dependencies":[{"issue_id":"bd-225","depends_on_id":"bd-224","type":"discovered-from","created_at":"2025-10-15T01:47:25.567014-07:00","created_by":"stevey"}]}
{"id":"bd-226","title":"Epic: Fix status/closed_at inconsistency (bd-224 solution)","description":"Implement hybrid solution to enforce status/closed_at invariant:\n- Database CHECK constraint\n- Smart UpdateIssue logic\n- Import enforcement\n- Reopen command\n\nThis is a data integrity issue that affects statistics and will cause problems for agent swarms. The hybrid approach provides defense-in-depth.\n\nSee ULTRATHINK_BD224.md for full analysis and rationale.\n\nParent of all implementation tasks for this fix.","status":"closed","priority":1,"issue_type":"epic","created_at":"2025-10-15T01:58:41.041574-07:00","updated_at":"2025-10-15T16:27:21.989539-07:00","closed_at":"2025-10-15T13:36:08.12242-07:00"}
{"id":"bd-227","title":"Audit and document all inconsistent issues in database","description":"Before we add the constraint, we need to know what data is inconsistent.\n\nSteps:\n1. Query database for status/closed_at mismatches\n2. Document each case (how many, which issues, patterns)\n3. Decide on cleanup strategy (trust status vs trust closed_at)\n4. Create SQL script for cleanup\n\nOutput: Document with counts and cleanup SQL ready to review.\n\nThis unblocks the migration work.","status":"closed","priority":1,"issue_type":"task","created_at":"2025-10-15T01:58:50.908363-07:00","updated_at":"2025-10-15T16:27:21.990024-07:00","closed_at":"2025-10-15T13:08:48.165755-07:00","dependencies":[{"issue_id":"bd-227","depends_on_id":"bd-226","type":"parent-child","created_at":"2025-10-15T01:58:50.909081-07:00","created_by":"stevey"}]}