fix(setup): consume journal-quarantine permits (Group E live-QA regression) - #2631
Conversation
…m to the executor
Live-QA regression from the v5.260723.1 dogfood: a stale activation journal
from the prior generation classifies as intent-mismatch, whose consent chain
correctly mints a JOURNAL-QUARANTINE permit — but setup routed every granted
permit into executeCodexActivation, which refuses non-activation capability
('permit lacks activation capability'), making the state's own recovery
('quarantine the mismatched intent after a fresh assertion, then re-observe')
permanently unreachable.
Setup now routes quarantine-capability permits to A's store.quarantineIntent
under the same setup-activation lifecycle lease the executor holds, prints the
content-addressed quarantine target, re-observes with one fresh pass (the new
state prompts its own consent for any actual activation), and refuses to loop
on a second quarantine grant in the same invocation.
Covered three ways: unit tests through the REAL consent/authorize chain
(quarantine → activation, skipped quarantine, loop guard) and a new PTY stage
reproducing the live failure end-to-end (corrupt journal → one consent →
quarantined sibling on disk → 'already current' → exit 0).
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ac67fa503a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| codexHome, | ||
| command: codexPath, | ||
| }); | ||
| quarantined = store.quarantineIntent(lease, permit); |
There was a problem hiding this comment.
If the intent changes after the initial observation/consent but before this lease is acquired — for example, another genie setup --codex run reaches the executor, leaves a valid planned/command-started recovery journal on failure, and releases the lifecycle lease — this stale journal-quarantine permit is still passed to quarantineIntent, which renames whatever regular file is currently at the intent path. Unlike beginActivation, this path does not re-observe and fingerprint-match the state under the lease, so it can remove a valid activation journal belonging to a different operation; please revalidate that the current journal is still the invalid/mismatched one before quarantining.
Useful? React with 👍 / 👎.
| if (!lease.ok) { | ||
| console.error(` \x1b[31m✖\x1b[0m Another Genie lifecycle command holds the Codex lease: ${lease.detail}`); | ||
| return { exitCode: 2, activated: false, code: 'busy' }; |
There was a problem hiding this comment.
When a stale-journal recovery races with another Codex lifecycle command holding the lease, this new branch returns exit 2 after printing only human stderr. The other setup activation busy path goes through the executor and emits the stable codex-lifecycle-busy JSON trailer, so automation that relies on the trailer to distinguish retryable lifecycle contention will break specifically for quarantine recovery contention; emit the same busy trailer here before returning.
Useful? React with 👍 / 👎.
…s, IN_PROGRESS The H3 SessionStart line surfaced ledger drift: waves 1+2 shipped 2026-07-22 (PRs automagik-dev#2625/automagik-dev#2626/automagik-dev#2628/automagik-dev#2629, per-group SHIP reviews, validations reproduced) but their evidence and criteria were never recorded; status still read APPROVED with execution long underway. - status APPROVED -> IN_PROGRESS - 41 evidence-backed criteria ticked (5 -> 46/67): Groups A-D ACs + global criteria owned by merged groups; deliberately NOT ticked: the two live 'codex mcp get genie --json' operator proofs, Group F/G criteria, and all post-merge QA rows - appended the waves-1+2 evidence block, the seven live-QA fixes (automagik-dev#2631-automagik-dev#2634, automagik-dev#2636), Group E's actual merge commit (4be6917), and the open automagik-dev#2633-deferred follow-up (pre-A route-arm retirement) Adversarially verified pre-commit by the pm-ledger-verify workflow: 0 must-fix; its 4 advisory findings (defect count, untracked follow-up, unrecorded E merge) are incorporated above.
1 participant
Live-QA regression from the v5.260723.1 dogfood (Group E follow-up)
Felipe's post-release ritual hit this on
genie setup --codex:Root cause: a stale activation journal from the prior generation classifies as
intent-mismatch(authorityjournal-quarantine-only). The consent chain correctly mints a journal-quarantine permit and A's store even exposesquarantineIntent(lease, permit)— but setup routed every granted permit intoexecuteCodexActivation, whosebeginActivationrefuses non-activation capability. The state's own prescribed recovery was permanently unreachable: re-running setup reproduced the refusal forever.Fix: setup routes quarantine-capability permits to
store.quarantineIntentunder the samesetup-activationlifecycle lease the executor holds, prints the content-addressed quarantine target, then re-observes with one fresh pass (the fresh state prompts its own consent for any actual activation). A second quarantine grant in the same invocation refuses instead of looping. New typed outcomequarantine-skippedcovers the fail-closed skip arms (symlink/non-regular journal path). No A/B/D contract changes — the fix is pure consumption of A's existing API.Coverage:
already current→ exit 0, and asserts the old "permit lacks activation capability" message never appears.Validation (all self-run): 172/0 across setup/doctor/executor/PTY suites; 284/0 with codex stripped from PATH (CI condition); typecheck + lint clean.
Also carries the wish-ledger commit from the merged #2630 cycle (Group E execution-review SHIP evidence) via the dev merge-back.