Fix MCP update tool bypassing close approval workflow

- Detect when update is called with status='closed'
- Redirect to close_issue to preserve approval workflow
- Ensures closing tasks always requires same approval regardless of tool used

Fixes #90

Amp-Thread-ID: https://ampcode.com/threads/T-9eab3a82-18f3-4ae3-a2d5-d114811383c1
Co-authored-by: Amp <amp@ampcode.com>
This commit is contained in:
Steve Yegge
2025-10-20 23:47:56 -07:00
parent f41cd8816e
commit f4dd9e09a1
2 changed files with 6 additions and 0 deletions

View File

@@ -86,6 +86,7 @@
{"id":"bd-179","title":"bd uses wrong default prefix when database filename indicates different prefix","description":"When creating issues without explicit --id flag, bd defaults to 'bd-' prefix even when the database filename and existing issues use a different prefix (e.g., 'wy-').","design":"## Bug Report\n\n**Environment:**\n- bd version: 0.9.11 (based on metadata)\n- Database: .beads/wy-.db\n- Directory: /Users/stevey/ai/flutter/wyvern\n\n**Observed Behavior:**\nWhen running `bd create task \"title\"` without --id flag, new issues get assigned `bd-*` IDs even though:\n- Database filename is `.beads/wy-.db` (implies wy- prefix)\n- All existing issues use `wy-*` prefix (wy-1 through wy-77)\n- No prefix metadata stored in database\n\n**Evidence:**\n```sql\nsqlite3 .beads/wy-.db \"SELECT * FROM issue_counters;\"\nwy|72\nbd|29\n```\n\nThe bd counter was incremented when I accidentally created bd-23 through bd-29 (now deleted).\n\n**Expected Behavior:**\nbd should derive the default prefix from:\n1. Database filename (`wy-.db` → `wy-` prefix), OR\n2. Most recently used prefix in the database, OR \n3. Prefix stored in metadata table\n\nAccording to `bd init --help`:\n\u003e -p, --prefix string Issue prefix (default: current directory name)\n\nBut database filename convention appears to be `\u003cprefix\u003e.db`, so wy-.db should mean wy- is the intended prefix.\n\n**Workaround:**\nUse explicit `--id` flag: `bd create task \"title\" --id wy-73`\n\n**Reproduction:**\n```bash\n# In a directory with .beads/wy-.db containing wy-* issues\nbd create task \"test\"\n# Creates bd-* issue instead of wy-* issue\n```\n\n**Suggested Fix:**\n1. Store the intended prefix in metadata table during `bd init`\n2. When creating issues, check metadata for preferred prefix\n3. Fallback to extracting prefix from database filename\n4. Last resort: use directory name\n\n**Related Code:**\nLikely in issue counter/ID generation logic where new IDs are assigned.","status":"closed","priority":2,"issue_type":"task","created_at":"2025-10-20T22:10:24.611471-07:00","updated_at":"2025-10-20T22:16:25.606694-07:00","closed_at":"2025-10-20T22:16:25.606694-07:00"}
{"id":"bd-18","title":"Fix: bd init --prefix test -q flag not recognized","description":"The init command doesn't recognize the -q flag. When running 'bd init --prefix test -q', it fails silently or behaves unexpectedly. The flag should either be implemented for quiet mode or removed from documentation if not supported.","status":"closed","priority":2,"issue_type":"bug","created_at":"2025-10-16T20:46:08.971822-07:00","updated_at":"2025-10-20T16:02:06.032275-07:00","closed_at":"2025-10-17T00:09:18.921816-07:00"}
{"id":"bd-180","title":"bd import should create database if it doesn't exist","description":"When running 'bd import file.jsonl' and the database doesn't exist, the command reports '0 created, 0 updated' but doesn't actually create the database file. This makes it confusing to reset/recreate a database from JSONL.\n\nExpected behavior: If .beads/vc.db doesn't exist, 'bd import .beads/issues.jsonl' should:\n1. Create the database file\n2. Initialize the schema\n3. Import all issues from the JSONL\n\nCurrent workaround: Restore an old database first, then import updates it.\n\nUse case: During VC dogfooding, we wanted to recreate the database with updated schema after schema.go changes. The workflow should be:\n1. mv .beads/vc.db .beads/vc.db.backup\n2. bd import .beads/issues.jsonl # Should create fresh DB\n3. Ready to go\n\nInstead, step 2 silently does nothing.","acceptance_criteria":"bd import creates database if missing, imports all issues successfully, returns count of created issues","status":"closed","priority":2,"issue_type":"bug","created_at":"2025-10-20T23:06:14.27608-07:00","updated_at":"2025-10-20T23:12:16.245609-07:00","closed_at":"2025-10-20T23:12:16.245609-07:00"}
{"id":"bd-181","title":"MCP update tool bypasses user approval when closing tasks","description":"GH #90: update with status='closed' should trigger same approval workflow as close tool. Currently undermines user control since close requires approval but update doesn't check parameter changes.","status":"closed","priority":1,"issue_type":"bug","created_at":"2025-10-20T23:40:17.69494-07:00","updated_at":"2025-10-20T23:40:42.293483-07:00","closed_at":"2025-10-20T23:40:42.293483-07:00"}
{"id":"bd-19","title":"Implement storage driver interface for pluggable backends","description":"Create abstraction layer for storage to support multiple backends (SQLite, Postgres, Turso, in-memory testing, etc.).\n\n**Current state:** All storage logic hardcoded to SQLite in internal/storage/sqlite/sqlite.go\n\n**Proposed design:**\n\n```go\n// internal/storage/storage.go\ntype Store interface {\n // Issue CRUD\n CreateIssue(issue *Issue) error\n GetIssue(id string) (*Issue, error)\n UpdateIssue(id string, updates *Issue) error\n DeleteIssue(id string) error\n ListIssues(filter *Filter) ([]*Issue, error)\n \n // Dependencies\n AddDependency(from, to string, depType DependencyType) error\n RemoveDependency(from, to string, depType DependencyType) error\n GetDependencies(id string) ([]*Dependency, error)\n \n // Counters, stats\n GetNextID(prefix string) (string, error)\n GetStats() (*Stats, error)\n \n Close() error\n}\n```\n\n**Benefits:**\n- Better testing (mock/in-memory stores)\n- Future flexibility (Postgres, cloud APIs, etc.)\n- Clean architecture (business logic decoupled from storage)\n- Enable Turso or other backends without refactoring everything\n\n**Implementation steps:**\n1. Define Store interface in internal/storage/storage.go\n2. Refactor SQLiteStore to implement interface\n3. Update all commands to use interface, not concrete type\n4. Add MemoryStore for testing\n5. Add driver selection via config (storage.driver = sqlite|turso|postgres)\n6. Update tests to use interface\n\n**Note:** This is valuable even without adopting Turso. Good architecture practice.\n\n**Context:** From GH issue #2 RFC evaluation. Driver interface is low-cost, high-value regardless of whether we add alternative backends.","status":"closed","priority":2,"issue_type":"feature","created_at":"2025-10-16T20:46:08.971822-07:00","updated_at":"2025-10-20T16:02:06.032562-07:00","closed_at":"2025-10-17T23:46:22.447301-07:00"}
{"id":"bd-2","title":"Sub-task under A","description":"","status":"closed","priority":1,"issue_type":"task","created_at":"2025-10-16T20:46:08.971822-07:00","updated_at":"2025-10-20T16:02:06.032798-07:00","closed_at":"2025-10-16T10:07:34.130096-07:00"}
{"id":"bd-20","title":"Investigate auto-export debounce not triggering","description":"Auto-export to JSONL did not trigger automatically after creating bd-33 and bd-17. Had to manually run 'bd export' to sync.\n\n**Expected behavior:** Auto-export should trigger ~5 seconds after CRUD operations (per CLAUDE.md documentation).\n\n**Actual behavior:** Issues bd-33 and bd-17 were created but JSONL was not updated until manual 'bd export' was run.\n\n**Investigation needed:**\n1. Check if auto-flush goroutine is running\n2. Verify debounce timer is being triggered on CreateIssue()\n3. Check for errors/panics in background export\n4. Verify auto-flush is enabled by default\n5. Check if there's a race condition with shutdown\n\n**Impact:** HIGH - Data loss risk if users create issues and don't realize they haven't synced to Git.\n\n**Testing:**\n```bash\n# Create issue and wait 10 seconds\nbd create \"Test\" -p 4\nsleep 10\ngrep \"Test\" .beads/issues.jsonl # Should find it\n```\n\n**Workaround:** Manually run 'bd export' after CRUD operations.\n\n**Context:** Discovered during GH issue #2 RFC evaluation while creating bd-33 and bd-17.","status":"closed","priority":1,"issue_type":"bug","created_at":"2025-10-16T20:46:08.971822-07:00","updated_at":"2025-10-20T16:02:06.049244-07:00","closed_at":"2025-10-20T16:02:06.049244-07:00"}

View File

@@ -339,6 +339,11 @@ async def update_issue(
workspace_root: str | None = None,
) -> Issue:
"""Update an existing issue."""
# If trying to close via update, redirect to close_issue to preserve approval workflow
if status == "closed":
issues = await beads_close_issue(issue_id=issue_id, reason="Closed via update")
return issues[0] if issues else None
return await beads_update_issue(
issue_id=issue_id,
status=status,