Skip to content

Fix migration concurrency and recovery - #120

Open
ericlitman wants to merge 10 commits into
mainfrom
codex/fix-install-migration-timeout
Open

Fix migration concurrency and recovery#120
ericlitman wants to merge 10 commits into
mainfrom
codex/fix-install-migration-timeout

Conversation

@ericlitman

@ericlitman ericlitman commented Aug 4, 2026

Copy link
Copy Markdown
Owner

What changed

  • Registers the controller's actual native runtime task ID from its marked first current-task title call instead of treating Codex's creation/supervision handle as title authority.
  • Runs migration writes in bounded concurrent waves with fresh activation reads and authoritative inventory reconciliation.
  • Keeps unknown native outcomes pending and recoverable, while allowing clean uninstall from stopped failures and quiescent pre-controller partial installs.
  • Captures the persistent home's title before hook installation, restores bare or decorated home sentinels, preserves unowned icon-prefixed titles before migration, and upgrades real v2.2.0 state that lacks the new original-title field.
  • Allows only the exact persisted home or controller to attest a fully terminal failed wave; only controller-owned proposals at exact Prior, exact Proposed, or disappeared targets can reconcile. Unknown results and drift remain fail-closed.
  • Keeps the installer guide, installed skill, tests, architecture, changelog, and experiment registry aligned.

Root cause

Codex Desktop can expose two IDs for one delegated controller: a creation/supervision handle and the native runtime task ID used by hooks and title calls. ThreadBear previously required them to match exactly, so a healthy controller could fail registration.

Recovery had several independent gaps: partial pre-controller installs could not enter uninstall, first-install tasks might not reload the new hook in time to capture their original title, decorated home sentinels were not recognized during cleanup, terminal failed waves had no reachable settlement path from the persistent home, and legacy v2.2.0 state had no Original field. The final correction handles each at the narrowest authoritative boundary without adding retries or trusting guessed identities.

User impact

ThreadBear accepts the controller identity Codex itself supplies from the marked controller task while preserving the meaningful safety boundaries: exact home delegation, one controller, exact native targets, pending-operation reconciliation, durable uninstall ownership, archive restoration, and binary-last teardown. Classifier JSON exists only in temporary installer tasks and is not present in ordinary ongoing sessions.

Validation

  • Final correction commit: 44d6b87
  • Final candidate binary SHA-256: 3b87d7fa5857c2e914759a437344d0e881fa322fbf6dc7dec215cf941fb96dce
  • go test ./... -count=1
  • go test -race ./... -count=1
  • go vet ./...
  • focused recovery and real-v2.2.0-upgrade regressions: 100/100
  • Darwin arm64/amd64 builds and candidate self-test
  • experiment-registry validation, install-guide parity, formatting, and git diff --check
  • shipped executable logic: 2,000 lines (2,000-line CI ceiling)
  • review-agent final pass: No findings
  • Fable final advisory: Ship

Two full live install/uninstall cycles against candidate 09fc5241… (the normal migration write path is unchanged in the final candidate):

Cycle Full 98-title migration Setup through active automation Uninstall Result
1 5:27.999 6:23.650 2:00.696 98/98 applied; zero remaining; home restored; all owned artifacts absent
2 5:03.690 5:41.080 2:10.504 98/98 applied; zero remaining; home restored; all owned artifacts absent

The accepted ceiling is full-list migration under six minutes; both cycles pass. Cycle 1's broader setup-to-active-automation time exceeded six minutes and is reported separately.

The later recovery-only production deltas were tested separately rather than misattributed to those timed runs:

  • Candidate 15027fdc…: fresh same-turn install captured the exact home title before any newly installed hook could run, then failed-state uninstall restored it and removed every artifact.
  • Final candidate 3b87d7fa…: a tagged v2.2.0 binary created genuine format-3 state; its real installed Pre/Post hook persisted the sentinel itself as the home subject; the final candidate upgraded that state to format 4 with the safe clean fallback, prepared uninstall before controller registration, restored the decorated home through a real native Pre/Post cleanup transaction, removed every artifact, and left a real unowned -prefixed canary title byte-exact.

Four terminal-state screenshots from the two full cycles are retained in /Users/ericlitman/.codex/visualizations/2026/08/04/019fcca7-7093-70f3-a18d-260951571103/.

Reviews and upstream boundary

  • review-agent: No findings on the exact final diff after all identified recovery gaps were fixed and re-reviewed.
  • Fable: Ship. It found that the verified controller alias preserves exact-target and transaction safety without adding retry/session machinery. Its residual risk is the honestly disclosed use of targeted live canaries, rather than another full 98-title cycle, for the final recovery-only delta.
  • Authoritative native setters and inventory reconciled all 98 titles. Codex Desktop still retained stale cached labels for some mounted sidebar rows; BEAR-114 records that upstream repaint boundary. Per product direction, it is disclosed but does not block ThreadBear's release.

@ericlitman
ericlitman marked this pull request as ready for review August 4, 2026 15:25
@ericlitman
ericlitman marked this pull request as draft August 4, 2026 15:43
@ericlitman
ericlitman marked this pull request as ready for review August 4, 2026 15:43
@ericlitman
ericlitman marked this pull request as draft August 4, 2026 15:57
@ericlitman
ericlitman marked this pull request as ready for review August 4, 2026 15:57

@mobilyze-open-swe-studio2 mobilyze-open-swe-studio2 Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Open SWE Review found 2 potential issues.

3 additional findings can be viewed in the web app.

Open in Web

Comment thread cmd/threadbear/install.go Outdated
Comment thread cmd/threadbear/state.go Outdated

@mobilyze-open-swe-studio2 mobilyze-open-swe-studio2 Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Open SWE Review found 2 potential issues.

Open in Web

Comment thread cmd/threadbear/hook.go
Comment thread cmd/threadbear/install.go

@mobilyze-open-swe-studio2 mobilyze-open-swe-studio2 Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Open SWE Review found 1 potential issue.

Open in Web

Comment thread cmd/threadbear/hook.go
proposed = map[bool]string{true: seed, false: renderTitle(status, subject, action)}[status == ""]
record.Pending = &pendingProposal{ToolUseID: toolUseID, BaseSubject: subject, Prior: task.Title, Proposed: proposed, Status: status, Action: action}
if status == "" && seed == homeTitle && attempt == "" {
record.Original, record.Last = cmp.Or(record.Original, stripStatusIcons(current)), homeTitle

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Fresh uninstall cannot restore home title

Original is captured only when this Pre hook handles the install-time home sentinel. The installed skill explicitly supports a fresh install where Codex has not reloaded the newly written hook; in that supported path the native home-title call runs with no hook, so no task record or Original value is stored. Later uninstall cleanup sees the live title equal to homeTitle, but the fallback is stripStatusIcons(homeTitle), which is still 🧵🐻 ThreadBear 🐻🧵; uninstall can therefore complete while leaving the former home permanently branded instead of restoring its pre-install title. The original title needs to be captured outside the newly installed hook (or hook participation must be required) before the sentinel mutation.

(Refers to line 174)


Your feedback helps Open SWE learn. React with 👍 or 👎 to tell us if this review comment was useful.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 44d6b87. Confirmed install now reads the authoritative persisted home task before binary replacement and records its title on first claim. Follow-up review also exposed decorated-sentinel and legacy-v2.2.0 cases: cleanup now restores bare or decorated sentinels, real format-3 state gets an upgrade-safe subject fallback, and a tagged-v2.2.0 live canary exercised the exact Pre/Post upgrade-through-uninstall path. Focused regressions passed 100x; full test/race/vet and the 2,000-line gate pass.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@openswe Please re-evaluate this finding against the current head 44d6b87 now that the reviewer model service is restored. The fix and live canary evidence are in the preceding reply and PR body; no further code has changed.

@ericlitman
ericlitman enabled auto-merge August 5, 2026 17:41
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.

1 participant