[architecture] Define unified MCP operator presentation for #180

[Joncallim]

Source Issue

Refs #180
Refs #172

Status

Draft implementation PR. This branch now includes the exact current #198/S4 base and the bounded repairs requested by the eight open review findings. Exact-head Contract and Web gates are green; the PR remains draft for review.

What changed

• Merged architecture/issue-179-context-packet-evidence at b1c99d6cd5cc3906dc453fd7f6e1df8d58036534 as a semantic union. The S4 role inventory and Web workflow retain every S3/S4 security, recovery, queue, PostgreSQL, and Redis proof.
• All seven S5 readers and the action endpoint use the same owner-gated server loader. Cross-user and missing-task responses are indistinguishable.
• Protected local evidence and terminal runtime audits are read in one PostgreSQL statement through the fixed forge_local_evidence_reader principal. There is no ordinary-application fallback. Only explicit safe columns are granted or projected.
• The six released recovery dispositions call the exact S4 recovery routines. The project grant action keeps its existing locked mutation path. Every action checks the state fingerprint and exact persisted identity, and continuation is enqueued only after commit.
• The database-authoritative ingress reader normalizes PostgreSQL raw bigint and timestamp values before applying its existing strict decision. Invalid, unsafe, or non-canonical values still fail closed; a valid active row now reaches the recovery routine and its expected stale-state response.
• The task page preserves filesystem pointer compare-and-set, explicit 409 reconfirmation, plan-review submission bound to the current approval gate/source artifact, and retained-history/no-hard-delete behavior.
• Submitted packet delivery accepts exactly { state, submittedAt }; other delivery arms remain exact one-key objects.
• Removed the obsolete unreleased-transition and old-head documentation.
• Removed raw Playwright report/test-result uploads from Web CI.

Production proof

Web CI now creates a pristine S5 database from the provisioned S4 database and runs one mandatory real PostgreSQL/Redis-backed test:

• two users, two projects, and real session lookup;
• a complete active ingress identity, including its opening authorization and
  controller identity, so the production action route crosses the same
  fail-closed gate as a deployment;
• all seven S5 GET routes plus the action endpoint;
• owner success, non-owner/missing 404 equivalence, and recursive exclusion of claimToken, grantNonce, effectiveGrant, and their stored values;
• one fixed-principal snapshot containing exact local-evidence/audit joins and a valid submitted delivery;
• exact 1/1, zero skips, and one S5_POSTGRES_HTTP_AUTHORIZATION_OK marker.

Local PostgreSQL 16.14 and Redis are reachable, but the shared local PostgreSQL instance does not contain the CI-only forge_e2e, forge_app_test, or forge_local_evidence_reader roles. The mandatory service proof is therefore intentionally left to the hosted role-provisioned job rather than replaced with mocks or local shared-role mutation.

Local validation

• Focused S5/action/plan/CAS/retention tests: 12 files, 179 passed.
• Full ordinary suite: 125 files passed, 1,875 tests passed.
• Sixteen service-backed tests were skipped locally: 7 release-recorder PostgreSQL, 6 two-runtime PostgreSQL/Redis adoption, and 3 real-Redis occurrence tests. Hosted Web supplies their required services and fails on a skip.
• npm run lint -- --max-warnings=0
• npx tsc --noEmit
• npm run build
• Web workflow YAML parse
• git diff --check

Hosted validation

• Contract: run 30570186582, job 90964670623 — passed.
• Web: run 30570158070, job 90964547212 — passed end to end.
• Complete zero-skip unit suite: 127 files, 1,891 tests passed.
• S4 PostgreSQL: 16/16 passed.
• S5 PostgreSQL HTTP authorization: exact 1/1 passed with S5_POSTGRES_HTTP_AUTHORIZATION_OK once.
• S4 Redis scrub: 3/3 passed with all three required markers.
• S4 Redis access-control-list proof: 3/3 passed with all three required markers.
• Cross-sink production sentinel: exact 1/1 passed with S4_CROSS_SINK_PRODUCTION_SENTINEL_OK once.
• Build, S3 PostgreSQL concurrency, disabled-ingress recovery, and the final browser bridge passed.
• GitGuardian completed neutral/skipped with no reported failure.

Exact integration

• Base branch: architecture/issue-179-context-packet-evidence
• Base SHA: b1c99d6cd5cc3906dc453fd7f6e1df8d58036534
• Head SHA: 3a46904b291ddfa823f9793a7ee653832323646c
• Tree: 3bf208949f8b0ccf9a351e545ba4307ca3770759
• Merge commit parents: prior PR head c7d2dcdad8bce973e349ba5da7989632697520a8 and current S4 base above.

Remaining evidence boundary

The existing real-browser D1-to-D2 pointer and stale-reconfirmation cases run in the bridge. The plan-review PostgreSQL concurrency cases remain classified by the inherited bridge as requiring signed activation; this PR does not relabel or bypass that security gate.

[Joncallim]

Architecture review update

Completed two orthogonal architecture passes for #180.

Round 1 findings addressed

• Replaced the implied single overloaded presenter with one shared module containing strongly typed admission, project-health, and catalog presenters.
• Separated historical admission decisions, current grant controls, and issued packet evidence.
• Distinguished retry actions from setup/remediation links and revise-plan/approve-context actions.
• Added truthful rendering for packetAssembled:false without invented zero counts.
• Centralized ACP non-sandbox boundary wording.
• Required fail-closed handling for unknown future/malformed persisted enum values.

Architecture updates are in docs/architecture/issue-180-review-amendments.md.

Round 2 result

No further architecture findings identified in the reviewed scope. S5 remains reader/presentation-only and must not persist or infer admission policy.

This is an architecture-only PR and does not implement #180.

[Joncallim]

Integrated architecture review — round 3 findings (before correction)

Verdict: Needs architecture changes. Implementation must not proceed from this revision.

High — the valid tuple table is contradictory

• Architecture: docs/architecture/issue-180-mcp-operator-copy.md:69-80 makes every bounded_context_approved state positive, then makes blocked + install_or_fix_mcp dangerous without precedence.
• Current contract: web/lib/mcps/admission.ts:813-820 can produce approved grant coverage plus warning|blocked health/remediation.
• Failure: an unhealthy bounded-context packet may render green “Context approved.”
• Invariant/downstream: operator presentation must reflect admission status/recovery before grant phase; [FEATURE] S6 — End-to-end MCP admission regression (tiny task-tracker flow) and preview==handoff invariant #181 visible-state proof.
• Smallest correction: define an exhaustive valid/invalid tuple table. Positive is only coherent status:'allowed' plus current unconsumed approval. Blocking/warning health and recovery override grant-phase color/copy.
• Proof: generated matrix including approved+unhealthy/missing/config/auth and invalid persisted tuples.

High — root evidence leaks a host path across S4/S5

The packet section at lines 144-157 displays an “approved root identifier,” while #179 proposes root:string. Replace it with opaque rootRef; legacy root/path values are ignored and never rendered. Test absolute/relative path sentinels at reader and component boundaries.

Medium — “current state remains compatible” is not an executable retry predicate

The amendment lines 31-35 is underspecified. Retry is visible/actionable only when the task is eligible, package is still blocked, versioned broker marker and current policy/block fingerprint match, retryable:true, primary recovery is install_or_fix_mcp, and no execution/issuance lease is active. The retry route rechecks this under the canonical lock order. Remediation, revise-plan, and approve-context remain distinct actions.

Medium — three truth sources and three presenter inputs must be explicit in the primary document

The primary document still defines one overloaded AdmissionPresentationInput, while the amendment correctly separates admission history, project health, and catalog facts. Fold the three typed presenters into the primary document. Render historical decision, current grant/action state, and issued packet evidence as distinct labelled sections; stale history never drives a current action.

Medium — untrusted copy needs sink and bound rules

Revocation, health, and error strings must be schema-bounded, control-character/secret/path sanitized, and rendered as React text nodes only—no HTML/Markdown interpretation. Unknown enum/schema values fail to legacy/recompute with no positive or retry action. Add hostile Unicode, HTML/script, oversized, secret-like, and path fixtures to all three presenters.

Coverage gap — packet failure and rollout states

Preassembly failures show stage/code without fake counts; assembled failures show the stored counts and a separate terminal outcome. S5 dual-reads old/new S2/S3/S4 shapes and fails closed until producers are deployed; S4 precedes S5.

Inspected scope: architecture/ADR copy contracts, canonical admission tuples, current metadata reader/task/project/catalog pages, retry route, and packet schema proposal. Confidence: high. Rendered accessibility and browser behavior remain unchecked until implementation; this is not proof of correctness.

[Joncallim]

Integrated architecture review — round 7 findings before correction

The fresh integrated pass found two downstream presentation blockers:

• High / blocking: S4 terminal evidence needs a closed terminal outcome in addition to assembly/delivery. S5 must render only valid terminal-status × assembly × delivery × failure-code combinations and fail closed on every known-but-incoherent cross-product. The live union must also include assembled/not_exposed failed-finalizing states.
• Medium / blocking: PacketIssuanceRecoveryMarkerV2 is currently a bag of independent fields. Encode or validate the exact grant-mode × delivery × disposition × acknowledgement × failure compatibility matrix before the presenter. Unknown and known-invalid tuples remain neutral/actionless; no raw or “sanitized” exception detail becomes copy.

The sibling-wait state also needs the S4-owned convergence transition specified on #198; S5 must remain actionless until that durable transition makes the task exactly approved.

[Joncallim]

Integrated architecture review — Round 8 findings (before correction)

Verdict for this round: blocked by two S5 presentation contradictions inherited from the S4 persistence model.

1. Medium / blocking — failed_finalizing is not derivable from durable evidence. Before the atomic terminal commit, a preparing claim and a pre-submission failure have the same stored tuple; the same is true for assembled/pre-submission and accepted/provider-invalid pairs. The web process cannot truthfully reconstruct worker-memory failure intent after a crash. S5 must show only the last durable staged phase until terminal evidence commits, or S4 must add a fenced persisted discriminant.
2. High / blocking — submitted post-response failures and invariant-success holds have no bounded operator state. A valid submitted response can be followed by sandbox, validation, repository apply/evidence, or completion-materialization failure, including partial host writes. That state requires static copy warning about possible prior submission and possible partial repository changes, no raw error detail, and no automatic resubmission. A terminal-success/live-package invariant split must remain neutral and non-actionable unless exact success reconstruction is proven; it must never render a packet retry from a manufactured failure marker.

Required correction: remove the non-durable failed-finalizing variants; retain only live phases derivable from persisted assembly/delivery; import S4's new closed post-submission stage; add bounded recovery/integrity copy; and keep all unknown or inconsistent tuples neutral and actionless.

Inspected stack head: 6598f7926c90ed50e2a2b01f0ff5cfeacda87c4c. This is a scoped architecture/current-call-path review, not proof of correctness.

[Joncallim]

Round 8 addendum — complete-tuple validation

One additional S5 consistency requirement was found before correction: a packet recovery marker does not contain enough assembly detail to validate the normative terminal tuple by itself. The current-state reader must bind the marker to the exact prior audit/artifact, prove their typed terminal tuples are equal, and validate assembly + delivery + terminal status + failure code/stage together. Missing, mismatched, or terminal-success-plus-failure-marker evidence is neutral, non-retryable, and actionless.

[Joncallim]

Integrated architecture review — Round 9 downstream findings (before correction)

S5 needs two downstream states from the fresh S4 review:

• While a submitted run has an active/unresolved host-effect intent and recovery has not proven the shared host fence quiescent, render actionless “Waiting for worker changes to stop.” Do not offer acknowledgement, retry, or reapproval.
• When recovery proves quiescence but the host-apply ledger has unknown outcomes, bounded copy must say the external submission and local repository may contain prior/partial work. The acknowledgement must explicitly record that the operator reviewed/resolved the working tree against the exact ledger fingerprint before retry can appear.

Typed packet-integrity alerts also need neutral static copy, an owner/runbook reference, no web repair CTA, and no raw path/error detail. The last closed stage is completion_preparation; atomic finalizer rollback is not rendered as that failure.

Posted before correction.

[Joncallim]

Round 9 addendum — review-pending sibling presentation

When any sibling package is awaiting_review, S4 keeps the task running and suppresses packet recovery actions. S5 must distinguish this from an active execution lease with bounded actionless copy such as “Waiting for required review.” It must not offer retry, acknowledgement, reapproval, or privileged integrity repair through normal web controls.

Posted before correction.

[Joncallim]

Integrated architecture review — Round 10 presentation dependency

Severity: Medium
Disposition: Blocking until S4 defines the terminal mismatch lifecycle and this copy consumes it

The current integrity UI language says an audit/artifact mismatch needs operator repair. A true mismatch may not be repairable without rewriting immutable evidence, which the architecture correctly forbids. S4 is being corrected to provide an authorized append-only quarantined_abandoned adjudication that closes the affected task/package without making the packet retryable.

S5 must present this as permanent evidence quarantine with an explicit close/abandon action for authorized Release/DevOps operators. It must not promise repair, automatic recovery, retry, or successful packet reconstruction. The live state remains evidence-derived from the integrity alert/resolution records.

No other S5 presentation blocker was found in this pass.

[Joncallim]

Integrated architecture review — Round 12 S5 findings (before correction)

Medium — The authoritative current-state join omits repository evidence

S5 joins the packet audit/artifact and host ledger, then binds the current marker to host-review fingerprints. S4 also requires exact equality for the pre-submission repository baseline, post-quiescence comparison, repository-review state, and their fingerprint. Without that join, stale or mismatched marker evidence could present a retry/reapproval action after a different repository review was completed.

Required correction: the authoritative presenter joins and compares the complete repository baseline/change/review evidence and marker fingerprint in addition to host ledger/review evidence. Any missing, stale, or mismatched repository component renders the neutral integrity hold with no action.

Medium — abandoned is not one closed cross-slice state

S4's type mentions normal repository review abandoned, its normative table and S5 consume only not_applicable|review_required|reviewed, and integrity quarantine separately has a reviewed|abandoned repository disposition.

Required correction: S5 treats abandonment only as an integrity-quarantine resolution disposition. It must not parse or render normal packet review as abandoned; quarantine copy remains evidence/history with no retry promise.

Low — Acknowledgement needs one explicit barrier exception

S5 must expose only the exact fingerprint-bound acknowledgement that completes review_required; every other retry, reapproval, root-management, or unrelated action remains blocked until that acknowledgement commits.

These are architecture-contract findings, not implementation findings.

[Joncallim]

Integrated architecture review — Round 12 recovery-action finding (before correction)

High — Definitively rejected delivery can still require local-change review, but S5 exposes no review CTA

submission_failed normally presents reapproval/retry directly. A post-call baseline can nevertheless be changed or unverifiable, and S4 requires exact review before that action. S5 currently reserves acknowledgement copy for uncertain/submitted delivery, so the valid definitive-rejection marker cannot progress.

Required correction: presentation is review-first whenever either exact host or repository review is required, independent of delivery. Expose only the typed fingerprint-bound local-change review action; after it commits, present the grant-mode/delivery action. Keep “the provider rejected the request” distinct from “local files require review,” and never imply the provider changed them.

This is an architecture-contract finding, not an implementation finding.

[Joncallim]

Integrated architecture review — Round 12 tombstone presentation finding (before correction)

High — A removed project must not retain an executable action surface

S5 correctly says “Project removed — evidence retained,” but S4 currently retains nonterminal work without an all-mode archived-project claim barrier. The presenter could therefore combine retained-history copy with a live retry/reapproval/progression action.

Required correction: an archived/tombstoned project is history/evidence-only. S5 renders no task execution, retry, reapproval, review-gate, or root-management CTA for cancelled packages; only explicitly authorized evidence inspection or privileged quarantine history remains. This consumes S4's bounded project-removed cancellation state and never infers cancellation from missing paths.

This is an architecture-contract finding, not an implementation finding.

[Joncallim]

Integrated architecture review — Round 12 corrections

Corrected in a98923ce:

• S5's authoritative reader now joins and fingerprint-validates the repository baseline/change/review, host ledger/review, marker, artifact, and task local-change barrier;
• local-change review is a distinct CTA that takes precedence for every delivery state, including definitive provider rejection;
• possible-submission acknowledgement remains a separate later action and never changes delivery;
• normal audit/marker abandonment is rejected; only a joined privileged quarantine resolution can render abandonment;
• successful presentation consumes the two exact S4 success branches and unchanged repository evidence;
• archived-project history is evidence-only and exposes no execution, retry, reapproval, review-gate, or root-management CTA;
• containment copy now describes the protected per-run group and avoids attributing repository changes to the provider.

Validation: documentation-only diff; git diff --check passed; branch stack rebased and pushed. Round 13 will re-run all 12 passes from the integrated S6 head before readiness is declared.

[Joncallim]

Integrated architecture review — Round 13 presentation finding (before correction)

S5's evidence join and exact retry/review predicates must include the versioned Git-control baseline/comparison fingerprint as well as working-tree evidence. Missing, changed, unverifiable, stale, or mismatched Git-control evidence must render the same review/blocking state as repository-local changes and must expose no path, hook, config, ref, or submodule detail. Success and retry controls remain hidden unless both working-tree and Git-control comparisons are exactly unchanged.

[Joncallim]

Integrated architecture review — Round 13 advisory finding (before correction)

Low — “Both packet actions” contradicts the closed three-action mutation union

The document says both packet actions carry immutable request identity, but the typed union has three packet mutations: retry, possible-submission acknowledgement, and local-change review. This stale count can lead an implementation or test to omit stale-request protection for the third action.

Required correction: say “Every packet mutation action carries…” and name all three handlers. Each handler must reject a stale {priorRuntimeAuditId, markerFingerprint} with no mutation.

[Joncallim]

Integrated architecture review — Round 13 advisory finding (before correction)

Low — submission_failed copy names a provider decision that evidence does not prove

submission_failed means a definitive failure before acceptance. It can come from a local adapter, connection, Transport Layer Security (TLS), or transport refusal; it is not necessarily a provider rejection. “The provider rejected the request” overstates the persisted fact.

Required correction: use neutral copy such as “The request was not accepted” or “Submission failed before acceptance,” then keep the separate local-change sentence. The same copy must cover a provider HTTP rejection and a locally definitive pre-send/transport refusal.

[Joncallim]

Integrated architecture review — Round 13 presentation finding (before correction)

Packet-free and handoff-only local-root runs need the same crash-safe local-effect/repository review lifecycle as packet runs, but must not manufacture a packet audit, packet artifact, delivery state, or packet-retry action. S5 must read the packet-independent run-effect record and render exact local-change review/quarantine state for those runs. Packet evidence stays absent; submission acknowledgement and packet retry/reapproval CTAs stay hidden. The local-change action carries the exact run-effect record ID plus evidence fingerprint and rejects stale identity without mutation.

[Joncallim]

Integrated architecture review — Round 13 correction

Corrected in 40e5e115.

• Packet retry and possible-submission acknowledgement retain packet identity; local-change review now uses exact generic local-run evidence identity for packet and no-packet runs.
• Added packet-independent quiescence/local-review presentation with no fabricated packet audit, artifact, counts, delivery, or packet CTA.
• Joined working-tree, Git-control, host-ledger, generic local evidence, and verified task projection before rendering success or any action.
• Changed submission_failed copy to the evidence-safe “The request was not accepted,” with local changes described separately.
• Replaced the stale “both actions” prose with explicit immutable identity and stale-request tests for all three mutation handlers.

Validation: docs-only; git diff --check passes; branch clean; stacked on #198. Round 14 is running. PR remains draft and unmerged.

[Joncallim]

Round 14 finding — Medium / blocking — contradictory local-review action ownership (PR #198 primary; #199/#200 downstream)

The corrected schema makes review_local_changes exclusively a work_package_local_recovery_actions action keyed by (local_run_evidence_id, action, evidence_fingerprint) and removes it from the packet issuance action enum. The packet route still accepts review_local_changes using only {priorRuntimeAuditId, markerFingerprint}, locks only the packet-oriented action row, and later requires every local review to write filesystem_mcp_issuance_recovery_actions, whose declared enum and key cannot represent the action.

Evidence:

• docs/architecture/issue-179-context-packet-evidence.md:238-248
• docs/architecture/issue-179-context-packet-evidence.md:1488-1506
• docs/architecture/issue-179-context-packet-evidence.md:1557-1565

Failure/cross-PR impact: implementations cannot choose a single idempotency source. A packet review can write the wrong table, while packet-free and handoff-only review cannot supply a runtime-audit ID. S5 and S6 may then disagree about replay and current-state truth.

Required correction: define one generic local-effect recovery endpoint and request keyed by {localRunEvidenceId, evidenceFingerprint} for review_local_changes. Packet UI may compose that action with packet presentation, but the mutation writes/replays only work_package_local_recovery_actions. Packet acknowledgement, retry, and one-time resolution remain separate packet actions. Remove local review from the issuance route/ledger contract unless a deliberately coordinated two-ledger transaction is specified and justified.

Required proof: packet, packet-free, and handoff-only local reviews; duplicate/concurrent/stale replay; review followed by packet acknowledgement or retry; exactly one local action row and zero issuance action rows for review.

This finding was published before correction. Architecture only; no implementation or merge is authorized.

[Joncallim]

Round 14 finding — Medium / blocking — S5 test contract still assigns packet identity to generic local review (PR #199 primary; #200 downstream)

S5’s normative types and presenter correctly split packet identity {priorRuntimeAuditId, markerFingerprint} from generic local identity {localRunEvidenceId, evidenceFingerprint}. Required test 28 still says local review, retry, and acknowledgement all carry the version-2 prior-audit/marker identity.

Evidence:

• docs/architecture/issue-180-mcp-operator-copy.md:74-84,189-215,565-589
• docs/architecture/issue-180-mcp-operator-copy.md:776-778

Cross-PR impact: S6 can implement a manifest-bound test that contradicts S4’s packet-independent action model, and packet-free/handoff-only local review cannot satisfy the test.

Required correction: split the test contract. Retry and submission acknowledgement use packet v2 identity; local review uses generic local v1 identity. Explicitly require cross-type substitution to fail closed.

Required proof: component and route fixtures for packet, packet-free, and handoff-only review plus stale and cross-route identities.

This finding was published before correction. Architecture only; no implementation or merge is authorized.

[Joncallim]

Round 14 finding — High / blocking — sole generic review does not converge the dependent packet disposition (PR #198/#199/#200)

The schema and S5 UI establish exactly one packet-independent local-review action and ledger. The S4 packet marker can nevertheless store disposition:'review_local_changes' plus a nextDisposition, while the generic review contract only clears its local marker. Implementing only that UI-exposed generic action can therefore leave the packet marker permanently stranded on a review disposition; implementing the old packet review route creates a second review authority and idempotency fence.

Evidence:

• docs/architecture/issue-179-context-packet-evidence.md:238-248,1446-1451,1468-1476,1488-1497,1557-1564
• docs/architecture/issue-180-mcp-operator-copy.md:210-215,565-589

Invariant: host/repository review is packet-independent, uses one generic request identity/action/ledger, and any exact dependent packet disposition converges from that committed fact without acknowledging delivery or clearing packet-owned evidence.

Required correction: make the generic local-review transaction the sole review mutation and replay ledger. Under the full lock order, it must also compare-and-set any exact dependent packet marker to its stored nextDisposition without clearing the packet marker or acknowledging delivery; alternatively make packet disposition derived from committed generic review. Remove review_local_changes from the packet route and issuance ledger. Recompute the task projection and clear only the exact local marker atomically.

Required proof: packet+local dual markers for every grant/delivery mode; generic exact replay; stale generic-versus-packet fingerprints; local review racing packet acknowledgement/retry/reapproval; no second review action; packet-free parity.

This finding was published before correction. Architecture only; no implementation or merge is authorized.

[Joncallim]

Round 14 finding — High / blocking — S5 suppresses recovery forever after exact review of detected repository changes (PR #199 primary; #198/#200 contract impact)

S5 says exact review advances a changed or unverifiable working-tree/Git-control run to its stored packet disposition, but its general action gate then requires both immutable comparison results to be unchanged. Review changes the review state/fingerprint, not the historical changed|unverifiable result. S4 and ADR 0009 instead allow exact reviewed fingerprints plus a verified zero task projection.

Evidence:

• docs/architecture/issue-180-mcp-operator-copy.md:494-515
• docs/architecture/issue-179-context-packet-evidence.md:1468-1476
• docs/adr/0009-mcp-admission-contract.md:1826-1830

Cross-PR impact: every always-allow recovery with detected and reviewed ACP changes dead-ends in S5 despite a valid S4 disposition. One-time copy can promise reapproval while the presenter suppresses it.

Required correction: action eligibility must accept either unchanged + not_applicable or changed|unverifiable + exact reviewed fingerprint, with exact host review and a current zero task projection. Keep unchanged as a terminal-success condition, not a post-review recovery prerequisite.

Required proof: both grant modes × all delivery states × working-tree-only, Git-control-only, and both changed/unverifiable. After exact review, pre-intent always-allow exposes retry; post-intent exposes acknowledgement then retry; one-time exposes the proper reapproval sequence. Stale/different fingerprints remain blocked.

This finding was published before correction. Architecture only; no implementation or merge is authorized.

[Joncallim]

Round 14 finding — High / blocking — Git evidence still omits object storage and other history-authority files (PR #198 primary; #199/#200 downstream)

The bounded Git-control snapshot covers configuration, hooks, HEAD/resolved refs, index, worktree administration, and submodule state. It does not cover the object database or several files that control object/ref resolution and history integrity: loose objects, packs/indexes, multi-pack index, commit graph, alternates, replace/grafts, shallow state, packed-ref storage as a file-level source, reflogs, and maintenance metadata.

Evidence:

• docs/architecture/issue-179-context-packet-evidence.md:653-668
• docs/architecture/issue-181-e2e-admission-regression.md:275-284

Failure/cross-PR impact: an unconfined ACP process can truncate or replace a pack/loose object, add an alternate/replace object, run repack or garbage collection, or inject unreachable objects while the named working-tree and Git-control evidence appears unchanged. Generic local evidence can clear, S5 can show success, and root reuse can proceed against corrupted or redirected history.

Required correction: add a bounded Git-storage snapshot or broaden Git-control to every file that affects object resolution/integrity. Fingerprint opaque metadata and content under the common-directory fence; if an equivalent bounded filesystem snapshot/journal cannot prove the state, fail closed as unverifiable and disable execution/root reuse.

Required proof: truncate a pack and loose object; swap a pack index; write alternates/replace/grafts/shallow state; run repack/garbage collection; add unreachable objects; repeat with linked external common directories. Every case becomes changed or unverifiable and blocks success, new work, and root management without leaking paths.

This finding was published before correction. Architecture only; no implementation or merge is authorized.

[Joncallim]

Round 14 finding — High / blocking — packet-free stale recovery has no terminal disposition when quiescence proves no local change (PR #198 primary; #199/#200 downstream)

S4 says packet-free and handoff-only crashes create local_effect_recovery only when working-tree/Git evidence is changed or unverifiable. The stale reconciler nevertheless says it always fails the run and moves the package to a structured local-effect recovery block. S5 exposes only local-change review or “Waiting for worker changes to stop,” and S6’s matrix similarly offers only review/quarantine. Once W2 proves quiescence and both comparisons are unchanged/not-applicable, there is no review action, no packet action, and no specified writer that removes the block or reaches an ordinary failed/retryable/terminal state.

Evidence:

• docs/architecture/issue-179-context-packet-evidence.md:786-792,1314-1331,1354-1363
• docs/architecture/issue-180-mcp-operator-copy.md:579-589
• docs/architecture/issue-181-e2e-admission-regression.md:445-447,636-639

Failure/cross-PR impact: an unchanged packet-free crash can leave a package and root permanently held with no normal action, while different implementers may silently choose incompatible retry/failure semantics.

Required correction: define a discriminated generic stale-run terminal matrix. Before quiescence, remain actionless and keep the resource fence. After exact W2/service receipt and comparisons: changed/unverifiable creates the local-review marker; unchanged/not-applicable creates no local marker and atomically reaches one explicit ordinary run/package/task disposition under the project’s existing retry policy, without inventing packet evidence or automatically rerunning. Specify task and wake behavior and exact replay/repair semantics.

Required proof: packet-free and handoff-only crashes at every boundary with unchanged, changed, and unverifiable comparisons; W2/quiescence wait; ordinary terminal/retry policy for unchanged; exact local review for changed; no permanent actionless post-quiescence block; no packet audit/artifact/action in any no-packet case.

This finding was published before correction. Architecture only; no implementation or merge is authorized.

[Joncallim]

Round 14 finding — High / blocking — no protocol-v2 membership or replacement path exists after cutover (PR #198 primary; #199/#200 operational impact)

Every process incarnation is never reused and independently authenticated. At epoch 2, claims/root writes accept only an active exact instance/principal, but the only promotion protocol is the one-time epoch-1 activation. An ordinary restart registers a new candidate that cannot claim. Once the activated set exits, no fresh W2 can recover orphan work and no restarted root writer can operate.

Evidence: docs/architecture/issue-179-context-packet-evidence.md:173-187,295-350,2002-2036.

Required correction: specify a privileged epoch-2 membership-change command/runbook: disable relevant ingress; provision the dedicated candidate; validate host, key, generation, protocol and capability; lock epoch/instances; drain, revoke and terminate the old principal; promote a bounded audited replacement; define capacity, rollback and all-active-gone recovery. W1 death must not prevent a separately authorized fresh W2 from joining safely.

Proof: rolling worker/root-writer restart; abrupt W1 death; all active workers gone; at-capacity replacement; claim/promotion/recovery races; replay by the revoked principal; bounded audit and fail-closed rollback.

Published before correction. Architecture only; no merge authorized.

[Joncallim]

Round 14 finding — Medium / blocking — mandated integrity CLI cannot address packet-free/handoff alerts (PR #198 primary; #199/#200 impact)

Integrity alerts require generic local evidence and make packet audit optional, but the only inspect/resolve command is keyed by --audit <id>. Packet-free and handoff-only recovery intentionally has no packet audit, so Release/DevOps can receive an alert that the required command cannot identify.

Evidence: docs/architecture/issue-179-context-packet-evidence.md:249-259,1355-1386; docs/architecture/issue-180-mcp-operator-copy.md:159-171.

Required correction: key inspect/resolve by mandatory --alert <id> (or exact local-evidence identity plus fingerprint), joining an optional packet audit only when present; generalize the runbook/command names. Proof covers packet and no-packet alerts, stale/wrong/cross-kind IDs, automatic W2 closure, and privileged manual resolution where allowed.

Published before correction. Architecture only; no merge authorized.

[Joncallim]

Round 14 finding — Medium / blocking — S5 has no typed packet-independent quiescence or integrity state (PR #199 primary; #198/#200 contract impact)

LocalRunRecoveryPresentationInput represents only a valid local_effect_recovery marker. S5 nevertheless promises packet-free “Waiting for worker changes to stop” and a neutral integrity hold for missing/mismatched generic evidence. The only quiescence discriminant sits in the packet presenter; its integrity branch is packet-specific. A typed implementation must fabricate packet state, fabricate a valid local marker, throw, or bypass the closed presenter.

Evidence: docs/architecture/issue-180-mcp-operator-copy.md:86-187,579-589.

Required correction: make the local presenter a closed union with generic local_effect_recovery, quiescence_wait, and local_effect_integrity_hold branches. Use bounded closed reasons, optional alert/support correlation, no required packet audit, no packet nouns/actions, and defined route/error semantics.

Proof: packetless/packet local markers; active/orphaned quiescence; absent/wrong run/root/fingerprint; stale task projection; transition to exact marker/hold; exhaustive unknown state; zero packet controls.

Published before correction. Architecture only; no merge authorized.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2cd6e6d600

ℹ️ 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".

[chatgpt-codex-connector]

Route normal migrations through the S5 owner handoff

When migration 0028 is pending, every normal install, upgrade, repair, and documented npm run db:migrate path reaches this call without permission to execute it, because migration 0027's finalizer revoked that temporary grant. Only the new CI-specific wrapper restores the grant, while scripts/install.sh, scripts/setup.sh, and scripts/repair.sh still invoke the raw migrator, so supported deployments fail before 0028 can be applied. Make the supported migration path perform the failure-safe bootstrap and cleanup, including when 0027 and 0028 are both pending.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Make the Enable MCP action actually enable the MCP

When an installed MCP is disabled by config.overrides[mcpId].enabled === false, this branch only adds it to requiredMcps and calls the selection saver, which preserves that same disabled override at lines 568-570. Because the MCP is already installed, the install endpoint is skipped, the refreshed overview remains disabled, and clicking “Enable MCP” repeatedly shows the same button. Update the matching override or installation authority to set enabled: true rather than routing enable through the unchanged selection flow.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Warn when recovery continuation remains pending

When Redis enqueueing fails after a local or packet recovery mutation commits, the action route returns HTTP 202 with continuationStatus: 'pending' but no error. This condition only surfaces 202 responses that contain payload.error, so the UI reloads, removes the consumed recovery control, and gives the operator no indication that execution still needs a continuation wake-up. Handle the pending continuation status explicitly or include an operator-facing warning in those route responses.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Hide recovery actions until sibling work is quiescent

When a recovery marker exists while the task remains running because another package is running, awaiting review, or holds a live lease, this predicate still emits the marker's actions. The protected recovery routines require an approved task and quiescent siblings, so every displayed control in that state deterministically returns 409 until reconciliation finishes. Include task status and sibling lease/review barriers in the action predicate and render the waiting state without controls.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Validate all retained evidence before reporting terminal state

When a terminal runtime audit and local-evidence row are coherent but the run-linked completion artifact, host ledger, or repository reviews are missing or mismatched, this predicate still reports trusted terminal success or failure. The loader never reads those records, so incomplete or conflicting retained evidence is presented as terminal instead of unavailable. Join and validate the complete retained tuple before setting state: 'terminal'.

Useful? React with 👍 / 👎.