Repository navigation
feat(codex): plugin update handoff — permit-gated activation, protocol-safe delivery + rollback - #2617
Conversation
Group A foundation for the codex-plugin-update-handoff activation protocol: - codex-activation-persistence.ts: bounded regular-file reads that fail closed on symlink/non-regular/oversize, atomic backup-first fsync-before-rename writes, and non-overwriting renames for quarantine and stale-lease supersession. - codex-lifecycle-lease.ts: single-host O_EXCL lifecycle lease with fresh 128-bit operation IDs, typed codex-lifecycle-busy refusals naming the holder kind, dead-pid supersession (one retry, rename evidence retained), fail-closed handling of symlinked/oversized/invalid lease files, and operation-ID fencing (assertOperation) for store transitions. Tests include a real two-process O_EXCL race proving exactly one winner.
Group A core for codex-plugin-update-handoff: one deep, fail-closed activation protocol whose state and authorization decisions are pure and total, with unforgeable consent/permit APIs. - observeCodexActivation(): bounded reads only — canonical payload version/digest, codex plugin list --json bounded to 5s/64KiB with exact single-JSON-value + duplicate rejection + ANSI/OSC sanitisation, symlink-rejecting cache parity, downgrade receipt/delivery/tombstone facts, and a cache-family witness snapshotted before+after the query to prove observation is inert. - classifyCodexActivation(): pure, total truth-table classifier (first match) over every design row incl. intent-target-current dominance and all four refresh-intent phases. - authorizeCodexActivation(): pure, no I/O; consumes a runtime-branded RetirementAssertion and returns a process-local, fingerprint-bound ActivationPermit. Brands are WeakSet-tracked so forgery, persisted consent, booleans, and test construction fail at runtime. - requestRetirementAssertion(): the only brand source — owns TTY/env/flag guards and the affirmative N→T prompt. - CodexActivationStore: only writer of delivery/intent/receipt/tombstone; raw paths private; publishDelivery, withRevalidatedDeliveryRoot (revalidates + rejects escaped capabilities + refuses GENIE_BUNDLE_ROOT), beginActivation (re-observes + exact fingerprint match, zero mutation on stale), fenced journal transitions, crash-safe finalize, and quarantine. - Stable human/JSON projections, doctor integrationSummary schema-1, the exit-2 result-trailer type + single canonical serializer, and the setup exit overlay. 64 table-driven tests under isolated GENIE_HOME/CODEX_HOME fixtures.
…e-private factories
…+ H3 smoke Add src/lib/codex-activation-executor.ts as the single permit-gated route to Codex plugin activation mutation. It acquires and holds the lifecycle lease, begins activation through A's store (fingerprint-checked), drives the supported codex plugin add via A's typed phase transitions, verifies full physical N+1 parity strictly inside withRevalidatedDeliveryRoot, runs the exact bounded no-shell H3 SessionStart smoke, restores the observed enabled flag, and finalizes (journal + one-time downgrade-receipt tombstone) through A. A busy lease is the typed codex-lifecycle-busy refusal with zero mutation. Focused test covers: non-genuine permit refusal, upgrade/install/disabled activation, stale-fingerprint zero-mutation refusal, lease busy + release, mid-transaction fencing, planned/command-started/target-current crash recovery and idempotent retry, one-time downgrade receipt consumption, store-only mutation spies, the H3 timeout/cap/schema/stderr/node/env-poison cases, and a real spawned two-process executor race proving exactly one winner.
Bring the A+B codex-activation branch (tip ac26491) up to current origin/dev (270 commits). All A+B modules are new source — zero shared-source conflicts. Conflict resolution (WISH.md, add/add — union, no history dropped): - Status header: fresh reconciled line — IN_PROGRESS, A+B execution-SHIP on this branch now rebased onto current dev, C-E remain, merge-gate language kept, dev's B1/B2 (3b4faa3/6f423869) + plan-gate digest preserved. - 2026-07-21 criterion-classification block and the A+B execution/review ledger: both live only on origin/dev (branch HEAD carried neither — that ledger was in the uncommitted WIP now preserved on wip/codex-handoff-c-draft-20260712); kept dev's content verbatim. DESIGN.md / DRAFT.md merged cleanly (no conflict). Validation: typecheck clean; A+B suites 118 pass/0 fail (unchanged from ac26491); full check 2268 pass/1 fail — the single failure is dev-introduced ui-bridge.test.ts shelling out to Linux-only `ss` (absent on macOS), a darwin-only environment gap that also fails on a clean origin/dev checkout here, not a merge regression. node-pty native binary required a manual darwin-arm64 prebuild fetch (bun install skips its lifecycle script).
…te core (Group C) - update-capabilities.ts: hidden `update --print-update-capabilities --json` probe (self-hash, one JSON object, empty stderr, exit 0), digest-bound backup capability sidecar, and enforceRollbackCapabilityFloor (no-follow/fstat/bounded read, no-shell sterile 5s/64KiB probe, sidecar/probe/rehash agreement, protocol floor >=1, extant-intent-schema coverage, TOCTOU revalidation of both identities before exchange). Wired via genie.ts + updateCommand pre-mode handler. - codex-delivery.ts: the Group C delivery gate. Observes/classifies via B's facade, publishes attested delivery + explicit-downgrade-receipt facts through A's publishDelivery under a caller-held lifecycle lease, converges non-plugin agents only, defers cache-advancing activation to setup, and emits the A-owned exit-2 result trailer + N/T pending output. Provably never begins activation, consumes/tombstones a receipt, advances a journal, retains a delivery root, or runs a plugin/cache mutator. - Tests: 24 (update-capabilities) + 5 (codex-delivery spy), all green; A+B 118 green untouched; typecheck/lint/complexity/knip clean. Deliverables 6, 7 complete; 1/4/5 core + 8 (rollback cases) complete. Parent/child wiring, exit-2 propagation, install.sh, integration test, races, and sync-only strip remain (see task timeline).
…roup C, D2) Legacy --sync-only is now a pure agent-sync compatibility path (wish decision 3, Felipe-ratified). Removed legacySyncOnlyPluginAdvisory, legacySyncRuntimeDrift, inspectSyncOnlyCodexHealth, and their option types from runLegacySyncOnlyConvergence: it branches before every Codex activation observer/classifier/authorization/plugin query/mutation, never lists/probes/inspects/enables/installs/swaps the plugin, and a genuine agent-sync failure is its ONLY nonzero result. Dropped the now-dead codex plugin-probe/health/parse imports. Closes AC: 'Sync-only makes zero activation observer/classifier/authorization/plugin query/mutation calls and exits 0 unless agent sync itself fails.' Tests: rewrote the sync-only suites to prove zero-plugin-query + agent-sync-only-nonzero + structural absence of any query seam; update.test.ts 164 pass; broad slice 496 pass; lint/complexity/knip clean.
… cache (Group C, item 1a) The post-delivery Codex convergence now classifies the installed generation before touching the plugin (update-integrations.ts convergeCodexForUpdateDelivery): - installed N ≠ delivered T → DEFER. Zero plugin add/remove/marketplace; converge only the non-plugin role agents; return an action-required exit-2 signal (deliveryComplete:true, actionRequired:true) whose detail names N and the 'retire tasks → genie setup --codex → /hooks → new task' recovery. The live N generation is left physically present-unverified. This closes the 2026-07-11 incident class at the exact site it occurred. - installed T (or absent) → the existing convergeCodexPluginOnly runs unchanged (it provably never plugin-adds here); absent stays absent with no redundant command. - indeterminate query (timeout/overflow/nonzero/malformed) → fail closed, action-required, never cache-advance. The installed-vs-expected compare mirrors convergeCodexPlugin's own 'before.version === expectedVersion' short-circuit, so the gate reproduces its 'would it cache-advance?' decision without duplicating mutation logic. Added additive IntegrationResult.actionRequired/deliveryComplete for exit-2 propagation (no behavior change to existing consumers; A+B suites green). Re-spec'd the 4 operator-driven codex tests from the old recache/one-add behavior to the new deferral (zero plugin add, old cache byte-identical, action-required). update.test.ts 164 pass; broad slice 340 pass; typecheck/lint/complexity/knip clean.
…(Group C) Wire the child-gate's action-required signal through every in-process and parent/child update boundary (deliverable 3): - applyConvergenceExitSignal: failed integration -> exit 1; else action-required (delivered, activation deferred) -> exit 2 + the one A-owned ANSI-free JSON result trailer (code=activation-pending, deliveryComplete:true, retry:false, nextAction=retire recovery); else exit 0. - runTrackedManualUpdateConvergence (already-current in-process) and runPostDeliveryConvergenceMode (the --post-delivery-converge child) both emit exit 2 + trailer with no all-green footer. - runFreshBinaryPostDeliveryConvergence now returns converged | action-required: a child exit 2 (execFileSync status:2) is delivered-but-action-required, NOT a failure; the parent mirrors exit 2 without re-emitting the trailer (child already printed it over inherited stdio), so parent/child exit semantics agree. - The indeterminate (unclassifiable) query is a plain exit-1 failure, distinct from exit-2 delivered-action-required. Tests: added D3 parent/child exit-2 boundary tests; update.test.ts 166 pass.
… (Group C, item 4/D10) install.sh handoff_to_subcommand now captures the `genie install` finisher exit code: exit 2 is delivered-but-action-required (installed N ≠ delivered T) — the signed binary is installed but the Codex generation was NOT activated — so main skips the all-green 'genie v<version> installed' footer and exits 2, never dying 1. The finisher's single machine-readable result trailer (deliveryComplete:true) disambiguates this exit 2 from an unsupported-platform exit 2, per the lifecycle exit-matrix contract. A genuine non-2 finisher failure still dies 1. Adds tests/integration/install-exit2-propagation.test.ts (deliverable 10): it actually EXECUTES install.sh's real handoff_to_subcommand + main exit-2 branch (sourced with GENIE_INSTALL_SOURCE_ONLY=1, network/verify surface stubbed, a genuine stub genie whose install yields exit 2 + the trailer) and asserts exit 2, the relayed trailer, no all-green footer, lock released on every terminal path, idempotent rerun, and that a real exit-1 failure still dies 1. bash -n stays a pre-check, not the proof.
… (Group C, item 1 Resolution Y) Unify the delivery gate (team-lead refinement to Resolution Y): codex-delivery.ts now exposes ONE classifier — classifyCodexDelivery(N, T) keyed purely on observed reality — plus buildDeliveryPublication / publishCodexDelivery over A's publishDelivery. Both the parent (publish) and the --post-delivery-converge child (converge) route through it, so there is no divergent gate and no test-only dead publish branch. The child gate (convergeCodexForUpdateDelivery) now delegates its absent/indeterminate/pending decision to the shared classifier, keeping an exact-string tightening only for the safe convergence delegation. Parent wiring in runDelivery (deliverables 4/5, Resolution Y): acquire the Codex lifecycle lease (kind update-delivery, aliased to avoid the agent-sync lease name collision) after signed-download verification and before the first binary swap; hold it through the swap + aux-sync; then publish attested delivery facts through A using OBSERVED reality — installed N from a live codex plugin list and delivered T + digest from a physical scan of the delivered tree (observeCodexActivation, never manifest-trusted); publish the digest-bound rollback sidecar only when the prior binary probes protocol-1+; release the lease on every terminal path. A busy lease refuses before any swap (zero mutation). Guardrails: (1) explicit D8/D9 test that a PRE-CONTRACT backup (unknown probe flag => nonzero) gets NO sidecar and enforceRollbackCapabilityFloor refuses it, while a protocol-1+ backup gets a sidecar and the floor passes end-to-end; (2) applyConvergenceExitSignal exported + tested to prove exit 2 + trailer appear ONLY on codex delivery-pending — a non-codex (claude) failure exits 1 with no trailer, an all-ok convergence stays exit 0, a failure wins over action-required. Tests: codex-delivery rewritten to the shared gate (spy store proves publish-only); update-capabilities +2; update.test.ts +6 (guardrails). 747-slice green; typecheck/lint/complexity/knip clean; A/B 118 untouched.
…sy (Group C) A delivery that finds the Codex lifecycle lease held by another command now refuses before any binary swap with the ratified loser semantics (deliverable 9): exit 2, machine code codex-lifecycle-busy, deliveryComplete:false, retry:true, and zero mutation (raised before promoteStagedInstall). Adds the shared CodexLifecycleBusyError + CODEX_LIFECYCLE_BUSY_TRAILER to codex-delivery.ts (reused by the forthcoming install gate) and maps it in updateCommand's delivery catch to exit 2 + the busy trailer instead of a generic exit-1 failure. Tests: busy trailer shape (deliveryComplete:false/retry:true) + error holder-kind. 185-file slice green; lint/knip clean.
…es (Group C, item 5) Spawns real OS processes contending for the ONE Codex lifecycle lease under a single fixture GENIE_HOME and proves cross-command exclusion (deliverable 9): an update+install and an update+rollback race each produce exactly one winner, and the loser gets the typed codex-lifecycle-busy refusal naming a valid held kind — never a corruption. Complements A's single-kind O_EXCL primitive race by proving update-delivery / install-converge / rollback all serialize on the same lease, so no two lifecycle commands mutate concurrently. All lease state stays under the fixture root. 3 pass.
…ge (Group C, item 5) Re-enable `genie update --rollback` (dev disabled it) behind the digest-bound capability floor (wish decision 7). codex-rollback.ts: discover the .previous/genie-<ver> backup carrying a capability sidecar; enforceRollbackCapabilityFloor confirms it; acquire the rollback lifecycle lease AFTER floor confirmation and BEFORE any staging (busy => exit 2 codex-lifecycle-busy, zero mutation); re-confirm the floor UNDER the lease and require the identical retained digest (the confirmation->exchange TOCTOU window); then the atomic exchange with the floor's identity discipline — open no-follow, fstat regular, copy backup->same-dir staging while hashing, verify the staged digest BEFORE committing, fsync file then parent dir, rename(2) over the live binary, then immediately re-fstat + re-hash the new live binary against the retained identity. On ANY revalidation miss before the rename the live binary is left untouched and the exact miss is reported. runRollback now maps rolled-back/no-backup/refused/busy/aborted to messages + exit codes (busy=>exit 2 + busy trailer; refused/no-backup/aborted=>exit 1). Removed the dead rollbackBinary wrapper. Executed tests (real self-hashing backup + sidecar + probe): successful fixed→fixed rollback swaps + revalidates + releases the lease; EVERY refusal path (no-backup, pre-contract floor refusal, busy lease, TOCTOU digest change, staged-digest mismatch) leaves live/backup/sidecar byte-identical (digests asserted before/after). 10 rollback tests; 520-slice green; typecheck/lint/complexity/knip clean.
…y (Group C) Item 2 — install.ts gate closes AC1's install arm (curl|bash reinstall vector). genie install runs on the freshly linked binary (T = VERSION); a pending Codex generation (installed N != T, from a live codex plugin list via the shared classifyCodexDelivery) is now DEFERRED: converge role agents only, exclude Codex from the plugin convergence (claude/hermes scope), and exit 2 with the one A-owned result trailer (deliveryComplete:true) naming N + the retire recovery — install never advances the cache. A fresh/absent or same-version plugin converges normally (classifier null). The classifier is an injected seam defaulting to the real probe, so existing install.test.ts stays transparent (no codex CLI => null); new tests inject the pending case and prove claude-only scope + exit 2 + trailer + no codex plugin convergence. AC6 — codex-rollback.ts header now documents the security boundary: a SINGLE corrupted/replayed/tampered artifact fails closed, while a same-uid adversary who can rewrite EVERY Genie state file plus the live binary is EXPLICITLY OUTSIDE the boundary (they already own the process). install.test.ts 49 pass; 646-slice green; typecheck/lint/complexity/knip clean.
…update (AC8) `genie install` only held the agent-sync lease (.agent-sync.lock), never the Codex lifecycle lease (.codex-lifecycle.lock) that `genie update`/`rollback` serialise on. A real update+install pair therefore did not contend on the Codex cache, and an install loser received agent-sync's generic exit-1 rather than the required exit-2 codex-lifecycle-busy refusal. installCommand now acquires the codex lifecycle lease (kind 'install-converge') when Codex is in scope, after the read-only classifier probe and before any plugin convergence — mirroring update.ts, including agent-sync-first acquisition order (no lock-ordering hazard) and its busy-error translation. On a busy lease it refuses with exit 2 + CODEX_LIFECYCLE_BUSY_TRAILER (deliveryComplete:false) and zero mutation; the lease releases in finally on every terminal path. The two leases coexist and guard different things. Extracted acquireCodexLeaseOrRefuse + buildInstallResults keep installCommand at the complexity ceiling. Tests: command-level busy-loser projection, no-lease for claude/none, held-lease acquire+release, and a real `genie install` command-path arm in the lifecycle race so the race is proven at command level, not just the primitive.
… (A+B+C done, D-E remain)
…ed observer (Group D, D3/D4)
… observation (Group D, D5)
… (Group D, D1/D2/D8/D9)
…note (Group D, D9)
…tall pairs (Group D, D8)
…ries resolveSelfBinaryPath returned process.argv[1], which on a bun --compile release binary is the virtual /$bunfs/root/genie entrypoint (absent on the real filesystem). hashRegularFileNoFollow then ENOENT-threw, so the capability probe never emitted and enforceRollbackCapabilityFloor would refuse every rollback on shipped binaries. Prefer process.execPath (the real on-disk executable) when argv[1] is a virtual bunfs entry or is absent on disk; keep argv[1] in interpreted modes (bun src/genie.ts, shebang'd dist/genie.js) where the script is the hash payload the sidecar binds.
…ier into build (Group E)
…lease-docs gate (Group E)
… awaiting final gate
|
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 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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dddc356096
ℹ️ 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".
| const result = (deps.executeCodexActivation ?? executeCodexActivation)({ | ||
| permit: authorization.permit, | ||
| store, |
There was a problem hiding this comment.
Route quarantine permits to the quarantine path
When the snapshot is intent-invalid or intent-mismatch, authorization grants a journal-quarantine permit rather than an activation permit, but this code sends every granted permit into executeCodexActivation. The executor's beginActivation path rejects that capability as "permit lacks activation capability", so a corrupt or mismatched refresh intent is never moved through store.quarantineIntent and genie setup --codex remains stuck in the same recovery state after the operator consents.
Useful? React with 👍 / 👎.
| @@ -1700,7 +1784,17 @@ export async function doctorCommand(options?: { json?: boolean; fix?: boolean }, | |||
| } | |||
| out(''); | |||
| out(failed.length === 0 ? '\x1b[32mAll checks passed.\x1b[0m' : `\x1b[31m${failed.length} check(s) failed.\x1b[0m`); | |||
There was a problem hiding this comment.
Suppress the all-green footer when Codex is broken
If the regular doctor checks pass but the new Codex activation summary is broken (for example query-failed, cache-missing, or payload-mismatch), this still prints All checks passed. before emitting the Codex diagnostic to stderr and exiting nonzero. That makes the human output contradict the actual doctor result for exactly the broken-Codex cases this summary is meant to surface.
Useful? React with 👍 / 👎.
| if [[ "$finisher_status" -eq 2 ]]; then | ||
| DELIVERY_ACTION_REQUIRED=1 | ||
| return 0 |
There was a problem hiding this comment.
Distinguish busy finisher exit 2 from deferred activation
genie install now also exits 2 when the Codex lifecycle lease is busy (codex-lifecycle-busy, deliveryComplete:false), but this branch treats every finisher status 2 as delivered/action-required. In that busy case the post-install finisher returned before integration convergence, yet the installer returns success from the handoff and later reports a delivered activation deferral instead of preserving the busy/no-convergence failure semantics.
Useful? React with 👍 / 👎.
| wishPath, | ||
| `# ${H3_INJECTION}\n\n| **Status** | IN_PROGRESS |\n\n### Group A: heading\n- [ ] pending\n`, | ||
| ); | ||
| const proc = spawnSync('node', [sessionContext], { |
There was a problem hiding this comment.
Bound the release H3 fixture subprocess
This verifier is part of the tarball release gate, but the H3 fixture spawn has no timeout or maxBuffer; if the shipped session-context.cjs regresses by hanging or producing unbounded output, the release verification can stall or exhaust memory instead of failing with a bounded error. Add explicit limits here as the production H3 smoke does, so every supported tarball can be reliably verified before promotion.
AGENTS.md reference: AGENTS.md:L43-L45
Useful? React with 👍 / 👎.
…ng INDEX entries Full wish inventory (2026-07-26) found the INDEX.md ledger current but six WISH.md Status headers stale, two wishes entirely absent from INDEX, and one INDEX line (stable-release-security-gate) behind its own WISH header. Headers reconciled (each keeps its history, gains a dated note): - plugin-resource-shipping DRAFT -> EXECUTED (PR automagik-dev#2540 merged 07-10) - codex-plugin-update-handoff EXECUTED/'merge gate OPEN' -> SHIPPED (PR automagik-dev#2617 merged 07-22) - genie-ui IN_PROGRESS -> SUPERSEDED (direction moved to genie-ui-dash 07-21) - agent-sync-hardening BLOCKED -> SUPERSEDED (gates closed with pr-2545 07-24, F02 excepted) - agent-sync EXECUTED -> DONE (merged automagik-dev#2541, superseded by current safety contract) - skills-fable5-revamp EXECUTED -> DONE (merged automagik-dev#2518, 3 LOW remain) INDEX additions: v4-home-residue-doctor (DRAFT, now the natural vehicle for issue automagik-dev#2450 v4-residue findings) and codex-plugin-dogfood-remediation (IN_PROGRESS 21/24 — the most active wish had no INDEX entry, invisible to the jar<->INDEX drift lint). INDEX line for stable-release-security-gate updated APPROVED -> IN_PROGRESS 4/6 to match its WISH header.
Wish: codex-plugin-update-handoff
Plan gate: SHIP 2026-07-12 (digest-stamped). All five groups execution-SHIP by independent reviewers; wish-level final gate SHIP 2026-07-22 (Fable, independent full-suite rerun + exhaustive cache-advance sweep). Full ledger:
.genie/wishes/codex-plugin-update-handoff/WISH.md.Prevents
genie update/installfrom invalidating open or resumable Codex tasks that retain paths into the active versioned plugin generation (the 2026-07-11 incident class). Separates delivery (always safe, exit-2 action-required) from activation (explicit, real-TTY-attested, permit-gated), with protocol-safe rollback.Groups
setup-activationlease, fingerprint re-observe, parity + H3 proof, receipt tombstone).classifyCodexDeliverygate; parent publishes attested facts underupdate-deliverylease; child/install defer N≠T with exit 2 + trailer (zero cache advance); sync-only pure; capability probe + digest-bound rollback floor + atomic exchange; real two-process races.integrationSummary(add-only, pending exits 2 with ok:true); init gated on verified-current; uninstall warning + lease + isolation.Verification
sstest; lease-primitive race flake that degrades holder-name diagnostics only — both root-caused by the final gate).Post-merge (by design, per wish text)
Homolog candidate → Felipe's live dogfood ritual (structural evidence validated by
scripts/validate-live-dogfood-evidence.ts) → WISH→SHIPPED + stable promotion. The ritual includes the ratified veto point on setup's activation-only UX.