Skip to content

[Bugfix #826] iter-7: Fix release blocker — LOCAL_SCHEMA CREATE INDEX vs migration order - #834

Merged
waleedkadous merged 1 commit into
mainfrom
builder/bugfix-826-iter7
May 23, 2026
Merged

[Bugfix #826] iter-7: Fix release blocker — LOCAL_SCHEMA CREATE INDEX vs migration order#834
waleedkadous merged 1 commit into
mainfrom
builder/bugfix-826-iter7

Conversation

@waleedkadous

Copy link
Copy Markdown
Contributor

Summary

Follow-up to merged PR #827 (Fixes #826) — fixes a release blocker discovered during local manazil verification: every v3.1.1 user's upgrade would have crashed at startup.

Root cause

ensureLocalDatabase runs db.exec(LOCAL_SCHEMA) BEFORE migrations on every open. PR #827's LOCAL_SCHEMA contained:

CREATE INDEX IF NOT EXISTS idx_architect_workspace ON architect(workspace_path);

On any pre-v11 install, the architect table lacks the workspace_path column. The CREATE INDEX threw no such column: workspace_path and aborted ensureLocalDatabase before migration v11 could run. The schema stayed at v10 and every subsequent setArchitectByName call failed with the same error.

Architect caught it on the user's actual codev/.agent-farm/state.db:

[WARN] Failed to persist architect verify-826-test to state.db: no such column: workspace_path

Fresh installs would have worked (LOCAL_SCHEMA's CREATE TABLE would have created the v11 shape from scratch) — but upgrade installs would not.

Fix

  1. Drop the CREATE INDEX statement from schema.ts LOCAL_SCHEMA.
  2. Add it to db/index.ts migration v11 block, OUTSIDE the inner if (!alreadyMigrated) guard but inside the outer if (!v11) guard.
    • Upgrade install: inner block runs (architect → architect_v11 rename); then CREATE INDEX IF NOT EXISTS creates the index on the new table.
    • Fresh install: LOCAL_SCHEMA created the v11-shaped table; inner block is a no-op (workspace_path column already present); CREATE INDEX still runs in the outer block.
  3. IF NOT EXISTS makes subsequent opens (after v11 is recorded in _migrations) a no-op.

Tests

3 new in bugfix-826-migration.test.ts:

  1. db.exec(LOCAL_SCHEMA) against a pre-v11 architect table no longer throws. Pre-iter-7 this test would have failed with no such column; it's the literal release-blocker repro.
  2. Full upgrade sequence: v10 schema → LOCAL_SCHEMA → migrate. Asserts the table is migrated AND the index exists.
  3. Fresh-install path: LOCAL_SCHEMA creates v11 shape → migration inner block skipped → CREATE INDEX still runs in the outer block. Asserts the index exists.

The existing migration tests didn't catch this because they set up the v10 schema directly in SQL and called the migration body — they never exercised LOCAL_SCHEMA's db.exec sequence, where the bug lived. Architect explicitly called out this test gap.

Test results

  • 1853 agent-farm unit tests pass.
  • pnpm exec tsc --noEmit clean.

Diff stat

3 files, +136 / -3.

CMAP

Will run 3-way after PR creation per the BUGFIX protocol.

… vs migration order

ROOT CAUSE: ensureLocalDatabase runs `db.exec(LOCAL_SCHEMA)` BEFORE
migrations on every open. LOCAL_SCHEMA contained
`CREATE INDEX idx_architect_workspace ON architect(workspace_path)`.
On any pre-v11 install (every v3.1.1 user upgrading to v3.1.2), the
architect table lacks the workspace_path column, so the CREATE INDEX
threw 'no such column: workspace_path' and aborted ensureLocalDatabase
before migration v11 could run. The schema stayed at v10 and every
subsequent setArchitectByName call failed with the same error.

Release blocker — would have broken every v3.1.1 user's upgrade.

FIX:
1. Drop the CREATE INDEX statement from schema.ts LOCAL_SCHEMA.
2. Add it to db/index.ts migration v11 block, OUTSIDE the inner
   `if (!alreadyMigrated)` guard but inside the outer `if (!v11)` guard.
   - Upgrade install: inner block runs (architect → architect_v11
     rename), then CREATE INDEX creates the index.
   - Fresh install: LOCAL_SCHEMA created the v11-shaped table, inner
     block is a no-op (workspace_path column already present), CREATE
     INDEX still runs (idempotent via IF NOT EXISTS).
3. CREATE INDEX uses `IF NOT EXISTS` so subsequent opens (after v11 is
   recorded) don't matter — the index is just there.

TESTS (3 new in bugfix-826-migration.test.ts):
1. `db.exec(LOCAL_SCHEMA)` against a pre-v11 architect table no longer
   throws. Pre-iter-7 this test would have failed with 'no such column';
   it's the literal release-blocker repro.
2. Full upgrade sequence: v10 schema → LOCAL_SCHEMA → migrate. Asserts
   the table is migrated AND the index exists.
3. Fresh-install path: LOCAL_SCHEMA creates v11 shape → migration inner
   block skipped (alreadyMigrated=true) → CREATE INDEX still runs in
   the outer block. Asserts the index exists.

The existing migration tests didn't catch this because they set up the
v10 schema directly in SQL and called the migration body — they never
exercised LOCAL_SCHEMA's db.exec sequence, where the bug lived.

1853 agent-farm tests pass. `tsc --noEmit` clean.
@waleedkadous

Copy link
Copy Markdown
Contributor Author

iter-7 CMAP 3-way result: unanimous APPROVE

  • Gemini (147.9s): approved. Verified the full upgrade sequence correctly adds the column and creates the index; verified the fresh-install sequence correctly skips the inner migration but still creates the index.
  • Codex (51.0s): approved. KEY_ISSUES: None.
  • Claude (73.9s): approved. Confirmed 1853 unit tests pass and tsc --noEmit clean.

No REQUEST_CHANGES, no flagged concerns. The release blocker is closed for both upgrade and fresh-install paths.

@waleedkadous — ready for re-merge.

@waleedkadous
waleedkadous merged commit 646caf6 into main May 23, 2026
6 checks passed
waleedkadous added a commit that referenced this pull request May 23, 2026
Critical hotfix on the Jacobean line. Closes #826: cross-workspace
sibling-architect leak introduced by v3.1.1's #786 multi-architect lifecycle
work. Schema-level fix (PR #827 — workspace_path column on state.db.architect
with composite PK, all state accessors take workspacePath, realpath canonicalization
at the boundary) plus migration-ordering hotfix (PR #834 — CREATE INDEX moved
out of LOCAL_SCHEMA into the migration block so upgrade installs don't crash).

7-iteration CMAP-driven development. Architect's independent CMAP caught a
real bug at every iteration that the builder's own CMAP missed: original
cross-workspace fix had a race, then stop+start regression, then exit-handler
delete leakage, then getTerminalsForWorkspace stale-cleanup, then migration
backfill non-determinism, then path canonicalization, then the upgrade-install
release blocker. Single-CMAP review on this PR would have shipped a regression.

End-to-end verified locally before publish: added a sibling to codev, confirmed
manazil (the original report workspace) showed only its own main and did not
leak the codev sibling. Migration v11 ran cleanly against the architect's
real v10 state.db.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CRITICAL: Sibling architects leak across workspaces — launchInstance reconcile reads global state.db.architect without workspace filtering

1 participant