Skip to content

Implement bounded context issuance and evidence for #179#198

Open
Joncallim wants to merge 93 commits into
architecture/issue-178-filesystem-grant-recoveryfrom
architecture/issue-179-context-packet-evidence
Open

Implement bounded context issuance and evidence for #179#198
Joncallim wants to merge 93 commits into
architecture/issue-178-filesystem-grant-recoveryfrom
architecture/issue-179-context-packet-evidence

Conversation

@Joncallim

@Joncallim Joncallim commented Jul 14, 2026

Copy link
Copy Markdown
Owner

Status

Implementation and review remediation are pushed at ba24cc2d96614cb6ca3c7990d84109bdb83ddf64.

Hold: this PR is not merge-ready. The latest orthogonal Review, Security, and QA pass is complete. Further review passes are paused at the operator's request until a new agent resumes the work.

Implemented scope

  • Protected Architect plan and MCP review history with dedicated PostgreSQL principals.
  • Bounded package registrations, capability binding, packet lifecycle evidence, recovery, archive, and strict migration proofs.
  • Closed Redis/SSE projections and post-commit session cache writes.
  • Content-free generic clarification projections with text resolved through audited Architect history.
  • Direct host repository application removed; default execution is sandbox-only and explicit host-write enablement fails closed.
  • Fresh/upgrade PostgreSQL role, ACL, recovery, archive, root-reference, and session reconciliation evidence.

Passing evidence

  • Unit suite: 1,620 passed, 15 skipped without mandatory PostgreSQL URLs.
  • ESLint with zero warnings, TypeScript, and production build passed.
  • Populated PostgreSQL 0026 to 0027 upgrade and strict cutover passed with distinct ordinary migration/admin principals.
  • Isolated mandatory S4 PostgreSQL suite: 8 passed, 0 skipped.
  • Recovery and archive proof includes rejection/zero-mutation/replay and bounded performance evidence.

Blocking follow-up

  1. Sandbox file creation still has a parent-directory swap race; the remaining write surface needs an OS-enforced beneath/no-symlink primitive or must fail closed.
  2. ACP specialist execution remains default-on and unconfined, so it can bypass the sandbox-only claim; default it off and require a constrained worker before enablement.
  3. Clarification plaintext remains duplicated in ordinary task_questions; store only opaque references and resolve text through protected audited history.
  4. Local-effect recovery must recompute its canonical evidence fingerprint and reject successful or packet-linked evidence rather than trusting public metadata.
  5. Mandatory PostgreSQL suites share one database and collide under Vitest file parallelism; give the S4 suite its own freshly migrated database/required command.

Takeover state

  • Head branch: architecture/issue-179-context-packet-evidence
  • Reviewed head: ba24cc2d96614cb6ca3c7990d84109bdb83ddf64
  • Base branch: architecture/issue-178-filesystem-grant-recovery
  • Seven review threads remain intentionally unresolved; do not close them until fixes are pushed and revalidated.
  • This is a stacked branch used by later PRs, so do not delete it when PR Implement bounded context issuance and evidence for #179 #198 eventually merges.

Copy link
Copy Markdown
Owner Author

Architecture review update

Completed two orthogonal architecture passes for #179.

Round 1 findings addressed

  • Added fail-closed compatibility for legacy allow_once approvals without a nonce; no synthetic authority is created.
  • Tied agentRunId creation/reservation atomically to the issuance claim to prevent orphan runnable runs.
  • Separated operator grantDecisionNonce from worker lease claimToken.
  • Required semantic parity between SQL partial indexes, Drizzle schema declarations, and conflict predicates.
  • Bounded lease/heartbeat policy using database time.
  • Clarified that packet metadata is staged before every exposure path, including debug/prompt construction.
  • Aligned lock ordering with [FEATURE] S3 — Deterministic bounded filesystem context grant/denial recovery #178.

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

Round 2 result

No further architecture findings identified in the reviewed scope. Implementation must still prove one-winning-claim, stale-token fencing, truthful recovery artifacts, and no deadlock using real PostgreSQL race tests.

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

@Joncallim

Copy link
Copy Markdown
Owner Author

Integrated architecture review — round 3 findings (before correction)

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

Blocker — packet fencing covers allow_once, not every repository-byte exposure

Blocker — the issuance lease is disconnected from the existing execution lease

  • Architecture: lines 96-167 and amendment lines 19-26 create a second lease/reconciler without a joint state transition.
  • Current implementation: web/worker/work-package-handoff.ts:1986-2060 commits package/run/execution-lease ownership together and has separate heartbeat/recovery paths.
  • Failure: one reconciler can fail an audit while a package/run remains live, or execution recovery can fail a run while issuance continues.
  • Invariant/downstream: no governed read/exposure without both owners; one recoverable terminal state.
  • Smallest correction: execution claim, agent run reservation, issuance audit claim, and one-time consumption share one top-down transaction; one database-time heartbeat renews both; stale recovery fails audit/run, clears the execution lease, writes a distinct issuance marker, moves package to blocked and task to approved, and requires reapproval when a nonce burned.
  • Proof: both lease-expiry orderings, crash points, and concurrent recovery.

High — stale reconciliation reverses the global lock order

Lines 158-167 start with audit rows FOR UPDATE SKIP LOCKED but then need package/run/artifact state. Discover candidate IDs without held locks, then reacquire project → tasks → packages → approval/current decision → agent run → audit and compare-and-set expiry. Test live-owner versus reconciler and grant mutation versus reconciler.

High — audit finalization and artifact uniqueness are not one atomic invariant

Lines 169-190 call both idempotent but allow a crash after terminal audit and before the artifact. Terminal audit transition and partial-index artifact upsert must be one transaction under the token (or a documented durable outbox). Inject the gap and prove exactly one truthful artifact.

High — the packet snapshot persists path-like root

Lines 139-156 contradict the no-path rule at line 156 and ADR 0009. Use an opaque bounded rootRef/project identifier only; legacy absolute roots are never rendered. Bound count maps, stage enums, and sanitized messages. Test a unique absolute-path sentinel across audits, artifacts, task logs, exports, diagnostics, and UI.

Medium — staged and terminal outcomes are conflated

The packetAssembled:false arm includes prompt_submission|finalization, even though assembly already succeeded. Persist an immutable assembly snapshot (assembled or preassembly_failure) before exposure, plus a separate terminal delivery outcome. Never invent zero counts.

Medium — prompt policy and logs need precise boundaries

Only the actual provider system role is immutable; a repeated wrapper after untrusted JSON is a reminder, not a security boundary. Preserve the current prompt-log sanitizer and specify digest/schema/count-only persistence—never prompt, overlay, packet, rejected text, paths, or credentials.

Cross-PR migration requirement

Use additive nullable schema, legacy allow_once without nonce as non-issuable, historical audits as unknown_legacy, dual readers, a worker protocol gate, and a drain before v2 issuance. Rollback leaves additive schema and must not restart a legacy issuer against v2 decisions. Immutable decision attribution must be snapshotted because the approval row is mutable.

Inspected scope: both ADRs, all slice documents, current handoff/executor/lease paths, schema and migration 0020. Confidence: high. This is not proof of correctness; real PostgreSQL and crash-injection proof remain mandatory.

@Joncallim

Copy link
Copy Markdown
Owner Author

Integrated architecture review — round 7 findings before correction

The fresh 12-axis pass reverified the prior corrections, then found these remaining S4 blockers:

  1. High / blocking — epoch activation race. “Stable snapshot” is insufficient. Activation must use an explicit PostgreSQL READ COMMITTED transaction: acquire the epoch row exclusively in one statement, then use a second statement/fresh command snapshot to query running null/v1 claims. If v1 obtained the shared lock first, activation aborts; if activation won first, the later v1 transition is rejected.
  2. High / blocking — packet recovery can be bypassed. The current generic handoff selector considers blocked packages and can promote one when canonical admission is allowed. A valid packet_issuance marker must be an absolute pre-admission/promotion guard. Only S4's recovery route or the S3→S4 one-time resolver may clear it and move blocked → ready.
  3. High / blocking — terminal evidence is incomplete. Assembly and delivery alone cannot distinguish success from assembled/not_exposed, accepted-but-invalid/submitted, or other terminal failures. Audit/artifact metadata needs a closed terminal outcome (succeeded or failed with the exact failure code) plus a normative valid-tuple table. Remove arbitrary sanitized failure detail; raw exceptions are not an acceptable packet evidence input.
  4. High / blocking — terminal-audit/live-package split. A crash after terminal audit+artifact commit but before run/package/lease terminalization currently leaves generic recovery delegating to an S4 CAS that only accepts claiming. Add an idempotent S4 repair branch for terminal packet audit + live run/package, or make the whole terminal transition atomic. A CAS miss is “already handled” only after proving run/package terminal and lease cleared.
  5. High / blocking — sibling convergence is unowned. Preserving task running while a sibling lease is live is correct, but current normal aggregation cannot later make the S4-blocked package actionable. S4 needs an explicit post-terminal/periodic top-down task-state reconciler that changes running → approved when the last sibling lease ends without promoting the packet-blocked package.
  6. Medium / blocking — protocol coverage. The epoch trigger applies to every package transition to running, but protocol 2 is stated only for the integrated packet claim. Every v2 transition path, including packet-free execution and no-op handoff, must use one shared claim primitive that sets protocol 2; only packet-bearing paths create an issuance audit.
  7. Medium / blocking — canonical/source and failure precision. readEffectiveGrantState returns source:'project-level' plus grantMode:'always_allow', not project_always_allow; the latter is only the immutable snapshot vocabulary. Also remove terminalization_interrupted unless a durable preterminal intent makes it distinguishable from worker/lease loss.

Required tests include both epoch lock orderings, direct/sibling handoff bypass, every claim mode after epoch 2, terminal-known-audit repair, full valid/invalid evidence tuples, and path-bearing exception leakage. This is architecture correction only; no production implementation is included.

@Joncallim

Copy link
Copy Markdown
Owner Author

Integrated architecture review — Round 8 findings (before correction)

Verdict for this round: blocked. The earlier findings are materially resolved, but the fresh orthogonal passes found four remaining contract gaps in S4.

  1. High / blocking — no truthful outcome for valid submitted work that fails later. The current executor can accept a valid provider response, then fail during sandbox apply, validation, host apply, repository-evidence persistence, or completion/review-gate materialization. The closed terminal table permits no accurate code for those paths. provider_response_invalid and worker_stopped would both be false. Host apply may also stop after writing some files, so automatic resubmission is unsafe.
  2. High / blocking — invariant repair can turn immutable success into retryable failure. The repair branch currently preserves any terminal audit/artifact, then fails the live run/package and derives a new worker/lease cause. A terminal-success split must not produce a packet-failure marker, and a terminal-failure split must preserve the exact immutable failure code/delivery rather than invent another cause.
  3. High / blocking — a sibling claim can evade task-disposition serialization. Stale recovery locks only siblings already observed as running/claiming, while packet-free and handoff-only ready → running writers are not yet unambiguously required to lock the task. A concurrent claim can establish a live sibling lease while recovery commits task running → approved.
  4. Medium / advisory — protocol-epoch activation has no assigned operator command/runbook. The three-statement READ COMMITTED protocol is correct, but implementation must provide one checked-in maintenance surface with actor identity, dry-run blockers, idempotency, postconditions, and saved activation evidence. Ad hoc SQL is not an acceptable release procedure.

Required correction: add a closed post-submission failure stage contract and no-auto-resubmit semantics; branch invariant repair by exact terminal outcome; make every all-mode claim and every reconciler contend on task/all siblings in global order; and assign the cutover command/runbook plus executable proofs.

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

@Joncallim

Copy link
Copy Markdown
Owner Author

Round 8 addendum — additional findings before correction

The final independent state-machine pass found one additional High / blocking issue and two consistency advisories. No correction has been applied yet.

  1. High / blocking — review-gate decisions violate the finalizer lock/freshness contract. The proposed atomic finalizer includes the package/review-gate transition in the top-down transaction, while the current gate-decision path reads gate/package/artifact/run before its transaction, then updates the gate before the package. That can deadlock with the finalizer or let a stale gate decision overwrite a newer running/awaiting-review transition. Add gate rows to the global order after packet artifact/completion inputs, lock project → task → package → relevant gates in stable order, revalidate source run/artifact/package status and lease inside the transaction, and compare-and-set against those identities. Add both-order PostgreSQL races.
  2. Medium / blocking-by-consistency — marker validation needs the complete terminal tuple. The recovery marker alone omits assembly detail. S5 must join the exact prior audit/artifact, prove audit/artifact tuple equality and marker identity, then validate the full assembly/delivery/terminal/failure-stage tuple. It cannot validate compatibility from the marker in isolation.
  3. Medium / blocking-by-consistency — submission_failed cannot be reclassified as lease expiry. The normative tuple permits only submission_rejected for assembled + definitive submission_failed, but the stale-cause priority currently puts lease loss first. Definitive rejection must persist its bounded cause with delivery and recovery must preserve it; add a crash test between delivery staging and terminalization.

The all-mode claim correction will also lock all sibling packages, recompute eligibility under lock, and prove no sibling is running or leased. This preserves Forge's current sequential specialist model, not just task-status consistency.

@Joncallim

Copy link
Copy Markdown
Owner Author

Round 8 addendum — ADR 0008 canonical vocabulary drift

High / blocking: ADR 0008 still says every packet decision audits root, while S4/ADR 0009 prohibit absolute or relative filesystem paths and require a lifetime-stable opaque rootRef. The same paragraph also lacks the corrected separated assembly, delivery, terminal outcome, closed failure stage, and no-content/no-name evidence boundaries. Because the user identified ADR 0008 as canonical packet vocabulary, leaving it stale would give implementers two conflicting sources of truth.

Required correction: make ADR 0008's evidence summary explicitly defer detailed lifecycle/schema to ADR 0009 and state only the privacy-safe vocabulary: opaque rootRef, bounded counts, separated assembly/delivery/terminal state, closed failure code/stage, and no paths/names/excerpts/contents/raw errors. Keep the beta boundary and no-live-handle decision unchanged.

@Joncallim

Copy link
Copy Markdown
Owner Author

Integrated architecture review — Round 9 finding (before correction)

Medium / blocking-by-consistency: Round 8 correctly made every recovery reader/action join and validate the exact prior packet artifact, but the route-specific locking prose still stops at the prior runtime audit. The S3 one-time resolver prose has the same omission. An implementation following those numbered locks could validate artifact state outside the transaction even though the normative marker contract says artifact equality is authoritative.

Required correction: lock/read the exact (agentRunId, artifactType='mcp_bounded_context_packet_metadata') artifact after the prior audit in the shared global order; validate canonical typed audit/artifact tuple equality under those locks before an acknowledgement, retry, or one-time resolver mutation. A missing/mismatch remains the typed integrity hold/no-action result.

Inspected final-stack head before restack: 472d896cec1aebbd2fa01c28eb6518bb86f66cc0. Finding posted before correction.

@Joncallim

Copy link
Copy Markdown
Owner Author

Integrated architecture review — Round 9 findings (before correction)

Verdict remains blocked on the following fresh post-submission/operability findings.

  1. High / blocking — recovery can race a stale worker's host writes. The contract checks dual ownership before reads, exposure, submission, and finalization, but not after the provider returns or around sandbox apply, validation, per-file host apply, and repository evidence. Recovery could publish an actionable marker while the old process resumes and renames more files. Add a host-process quiescence fence acquired with no database locks, persist post-submission effect intent, keep heartbeats/checks around every stage and per-file atomic replacement, and require recovery to acquire the same fence before terminalizing. If quiescence is unproven, state stays actionless and no new run may start.
  2. Medium / blocking — partial-host evidence has no durable producer. Current code keeps completed files only in memory. Define a run-scoped host-apply ledger with per-entry planned|applying|applied|unknown state, recording intent before and outcome after each atomic replacement under the fence. Reference existing validated output-entry identities; keep paths/raw errors outside packet evidence. Crash after rename/before outcome becomes unknown, and packet retry requires explicit working-tree review recorded against the ledger fingerprint.
  3. Medium / blocking — completion_materialization conflicts with atomic rollback. Rename it completion_preparation and limit it to fallible work before the terminal transaction. Gate insert/finalizer database failure rolls back and leaves the prior durable state; it never persists that stage.
  4. High / blocking — ADR 0008 overclaims both implementation and confinement. It says the S4–S6 beta is implemented and suggests arbitrary host filesystem access is avoided generally. Scope the consequence to the Forge-issued MCP channel, say the architecture takes effect when implemented, and repeat that ACP is not an operating-system sandbox.
  5. Medium / advisory-to-close — integrity holds lack an owner and repair surface. Assign a durable bounded alert, Release/DevOps ownership, a checked-in inspect/repair runbook and command, authorization, fingerprint compare-and-set, and append-only resolution audit. Repair must never rewrite immutable packet evidence or expose retry while evidence/quiescence remains unproven.

Required tests: expiry before/between/after atomic file replacements; crash between rename and ledger outcome; fence acquisition in both orders; pre-transaction completion-preparation versus finalizer rollback; bounded alert deduplication and unauthorized repair rejection.

Inspected stack head: 45506d4de3043e672752d51ffdd9f1f2495bd3e6. Findings posted before correction.

@Joncallim

Copy link
Copy Markdown
Owner Author

Round 9 addendum — sibling review barrier, complete order, and hold authority

Three further blocking-by-contract findings were found before correction:

  1. Forge's current beta runs specialists sequentially through mandatory review. A sibling in awaiting_review has no live lease, so the current “no running/leased sibling” predicate can start the next package or expose a recovery CTA before QA/Review/Security gates finish. The all-sibling package locks must treat awaiting_review as an arbitration barrier; task disposition stays running until no sibling is running/leased/awaiting review.
  2. The order called complete omits multiple artifact rows and append-only issuance-recovery action rows. Define the tail as agent runs ascending → runtime audits ascending → all artifact rows by stable key → recovery-action rows by unique key → review gates ascending. Exact replay/discovery may happen unlocked, but mutation reacquires this order.
  3. The top-level guard currently groups recovery and integrity markers before saying the normal route/resolver can clear “the exact marker.” Split authority explicitly: normal packet recovery/one-time reapproval may clear only packet_issuance; packet_integrity_hold may be cleared only by the separately authorized, fingerprint-bound privileged repair procedure. Every normal action rejects integrity holds.

Required races: claim/recovery versus an awaiting_review sibling and its gate decision; duplicate/replay/one-time resolution versus artifact/action ordering; every normal web recovery action against both integrity-hold reasons.

@Joncallim

Copy link
Copy Markdown
Owner Author

Integrated architecture review — Round 10 finding (before correction)

Medium / blocking: the canonical order omits rows newly introduced by Round 9: host-apply ledger/entries and integrity alert/resolution rows. Place them explicitly between runtime audits and review gates. Every per-stage/per-file ledger mutation and every alert/repair mutation uses the applicable prefix/tail; unlocked discovery is allowed, but mutation reacquires the order. A quiescence-timeout alert may be inserted without owning the host fence because it changes no execution state, but it must use a short top-down transaction and never wait for the fence while holding database locks.

Add observed-lock races for per-file ledger updates/finalization/recovery and alert/repair/finalizer paths.

@Joncallim

Copy link
Copy Markdown
Owner Author

Integrated architecture review — Round 10 additional blocking finding

Severity: High
Disposition: Blocking for architecture readiness

Host-tree exclusion is not closed over project management routes

The proposed post-submission fence is keyed only by project ID and is shared by worker/recovery. Current project management routes can repoint localPath or recursively delete the repository without acquiring that fence. Because a running worker retains the previously validated root, and because the same canonical host path can later be assigned to another project, project-ID-only exclusion does not prevent overlapping mutation/deletion of the same host tree.

Evidence inspected:

  • docs/architecture/issue-179-context-packet-evidence.md host-fence and stable-rootRef contracts.
  • web/app/api/projects/[id]/route.ts project update and delete filesystem behavior.
  • The restacked Round 10 head and complete lock-order correction.

Required correction before the next review:

  1. Define one canonical host-root resource identity derived from the resolved canonical path, with a project-ID component only as a non-colliding fallback when no host root is configured.
  2. Require worker, recovery, project root repoint, project deletion, and any filesystem-management path to acquire the same resource fence while holding no database locks.
  3. After the fence is acquired, lock/revalidate project/task/package state top-down; reject or wait safely while post-submission effects remain active or quiescence is unproven.
  4. For a root change, fence both old and new canonical root identities in deterministic byte order so path swaps/reuse cannot deadlock or overlap.
  5. Preserve the rule that no code waits for a host fence while holding database locks.

This finding is based on the current implementation's project PUT/DELETE behavior; it is not proof that other filesystem-management paths do not exist.

@Joncallim

Copy link
Copy Markdown
Owner Author

Integrated architecture review — Round 10 consolidated addendum

The independent security, QA, and state-machine passes found five additional blocking contract gaps after the top-level order correction.

1. High — Same-root, cross-host, and descendant quiescence are not fenced

  • Fence a stable opaque host-resource identity derived from the canonical physical root, with alias, symlink, and platform case normalization. Pin that identity to the run; do not expose it in packet evidence.
  • Acquire the shared resource fence before the first repository-context read or packet assembly and retain it through provider submission, all local effects, terminal finalization, and descendant quiescence. A post-submission-only fence cannot prevent a root repoint from turning the old path into another project's repository while the first run still reads or writes it.
  • Project root repoint must fence old and new resource identities in deterministic byte order. Project delete and every host-filesystem management path must acquire the same fence with zero database locks.
  • Root mutation/reuse also increments a persisted project root-binding revision. Claim pins the canonical resource identity and revision; every repository read/effect/finalizer revalidates the pin. Repoint/delete/reuse cannot commit while any pinned claim remains active or quiescence is unproven.
  • Recovery must prove an authoritative currentHostId === effectIntent.hostId before local fence acquisition or terminal mutation. A mismatch or unreachable owning host is alert-only.
  • A dedicated supervisor/process-group protocol must retain the fence through every descendant. On worker/control-channel death it terminates and waits for the process group; the inherited fence remains held until the last descendant exits. Hosts that cannot prove this capability fail closed for protocol-v2 local effects.

2. High — S3/S4 review aggregation is contradictory

The S4 resolver called from S3 must participate in the task-wide awaiting_review barrier. It locks sibling packages in canonical order even though grant evaluation remains package-local. No recovery action or running → approved transition may occur while a sibling review is unresolved.

3. Medium — Individual mutating paths still omit the complete tail

The acknowledgement/retry route, one-time resolver, invariant repair, success repair, integrity resolution, and gate decision must each explicitly acquire applicable rows in the canonical suffix: host ledger/entries → all artifacts → recovery actions → integrity alerts/resolutions → review gates. The route cannot validate host-review or integrity state from unlocked rows.

4. Medium — Activation cannot prove host capability/routing

The planned activation command promises to report every worker/fence/same-host blocker, but no durable capability evidence exists. Keep the initial v2 local-effect rollout explicitly single-active-host. Add a durable worker-host capability registration/heartbeat with stable opaque host ID, supported protocol version, fence-supervisor capability, last-seen time, and drain state. Activation fails unless exactly one fresh active host is registered and every legacy/incompatible worker is drained; its audit snapshots the registrations. Multi-host local effects remain disabled until a later host-affine routing design.

5. Medium — Terminal/effect/ledger and mismatch outcomes are underspecified

  • Add one exhaustive compatibility table for assembly, delivery, terminal result/code/stage, effect intent, host ledger, and host review. Terminal post-effect rows require quiesced; active is nonterminal only; caught local failure requires lastStage === failureStage; success forbids planned|applying|unknown; submitted-before-effect failures use not_started. SQL checks, parsers, finalizers, recovery, S5, and S6 consume the same table.
  • A true immutable audit/artifact mismatch may satisfy neither verified success nor coherent failure. Define an evidence-preserving append-only quarantined_abandoned adjudication that terminally closes the task/package without making the packet retryable or rewriting either record. The operator copy must say permanent evidence quarantine/closure, not repair.

These are architecture gaps, not claims that the future implementation or host behavior has been proven.

@Joncallim

Copy link
Copy Markdown
Owner Author

Integrated architecture review — Round 11 findings

Disposition: Changes requested before architecture readiness

Medium — Wrong-host predicate references a field absent from not_started

Recovery currently requires the authoritative current host ID to equal both the run pin and effectIntent.hostId. The not_started union arm intentionally has no host field, including the documented submitted-before-effect crash. That path cannot implement the stated predicate.

Required correction: always compare current host with the locked work-package/agent-run host pin; compare effectIntent.hostId only for active|quiesced. Add wrong-host tests for both not_started and active effects.

High — A caught live host-write failure can still be outcome-ambiguous

The compatibility table says a caught live local failure cannot contain ledger unknown. But an atomic replacement can succeed and the following applying → applied database write can fail or lose ownership. The still-live worker cannot truthfully infer whether the durable filesystem effect and database outcome agree merely because it caught the error.

Required correction: while retaining the resource fence, any live path that cannot durably prove applied maps the entry to unknown before terminalization, or leaves the run nonterminal for recovery if PostgreSQL is unavailable. A caught host_apply failure may therefore contain unknown, must set fingerprint-bound review_required, and cannot expose a new run until review. Success continues to forbid unknown.

These findings are new Round 11 checks; the broader resource-fence/root-binding correction remains intact.

@Joncallim

Copy link
Copy Markdown
Owner Author

Integrated architecture review — Round 11 consolidated lifecycle findings

Disposition: Changes requested; all findings block architecture readiness

1. High — Hard project deletion destroys immutable evidence

Current foreign keys cascade project deletion through tasks, packages, runs, and artifacts. The proposed root cleanup still ends by deleting the project, contradicting permanent packet evidence, integrity alerts, and quarantine resolution retention.

Required correction: protocol-v2 projects use a soft-delete tombstone. Final deletion clears the path and releases the live root binding but retains project/task/package/run/audit/artifact/action/alert/resolution rows. The unique root constraint applies only to non-deleted projects. Normal queries hide tombstones; hard purge is forbidden until a separate retention/export architecture exists.

2. High — A nonexistent clone destination has no physical identity to fence

GitHub/local creation can clone or create a directory before a project row or physical root exists. Two requests can pass pathExists; a losing clone can then remove the winning destination. A physical device/object-derived hostResourceRef cannot be acquired before creation.

Required correction: add a typed pre-create reservation keyed by authoritative host + canonical existing parent identity + platform-normalized missing suffix. Acquire its namespace fence before mkdir/clone, persist planned|materialized|bound|cleanup_required, and retain it while deriving/acquiring the new physical resource fence and atomically inserting the project binding. Crash/loser cleanup must compare reservation token and object identity and must never delete a later reused path.

3. High — Worker registration is checked only at activation

An epoch-2 claimant can currently supply protocol/host/supervisor settings without proving a fresh active forge_worker_instances row. Add transaction-local instance ID, pin it on package/run, and have the running-transition trigger lock the exact instance row after the epoch row and verify active/fresh/host/protocol/supervisor/binding-key equality. Activation/drain lock epoch then instance rows. Add worker-instance rows to the canonical database order after the epoch.

4. High — Inherited file descriptors do not prove ACP descendant quiescence

A child can close extra descriptors or daemonize, and supervisor-first death can release the lock while the main worker continues. Replace the inherited-descriptor claim with a host fence service plus an OS-enforced containment adapter that includes the Forge worker, ACP transport, validation children, and every descendant. The service owns the resource lock and durable local lease; worker/service/control loss makes it orphaned and actionless. Automated release requires the containment adapter to prove the group empty. Unsupported or unverifiable hosts fail closed for protocol-v2 local-root execution; no process-memory/process-tree guess is accepted.

5. High — Binding-key and old root writers are not fenced by cutover

Every worker/epoch/root binding must carry one stable host-binding-key fingerprint; divergent key material on the same host blocks activation/claim. Define backup and rotation as drain → disable → rebind, never silent key replacement.

Add an expand-phase project-root mutation trigger serialized with the epoch. At epoch 1, legacy create/repoint clears/invalidate nullable bindings and forces explicit rebind; at epoch 2 it rejects missing/malformed host ID, resource ref, revision, maintenance token, or authorized writer settings. Activation's fresh snapshot plus exclusive epoch lock then durably excludes stale web/root-management writers, not only workers.

These corrections must preserve the rule that no database lock is held while waiting for a namespace/resource/containment fence.

@Joncallim

Copy link
Copy Markdown
Owner Author

Integrated architecture review — Round 11 evidence-bypass finding

Severity: High
Disposition: Blocking

Unconfined ACP work can bypass the host-review gate

The architecture correctly says prompt text cannot stop an Agent Communication Protocol (ACP) runtime from making equivalent shell or filesystem changes. However, the normative table treats submission_uncertain or submitted-before-local-stage as effectIntent:not_started, no host ledger, and hostApplyReview:not_applicable. A provider can change the repository during submission, crash before Forge's local stage, and leave no Forge ledger. The resource fence proves later quiescence, but it does not prove that the repository remained unchanged.

Required correction: capture a repository baseline before ACP submission and a post-quiescence change fingerprint under the same resource fence. Detected or unverifiable external-runtime changes require fingerprint-bound working-tree review before acknowledgement, retry, reapproval, root management, or a new run. Apply this rule to provider success as well as failure. If that evidence cannot be produced, protocol-v2 packet guarantees must exclude unconfined ACP execution.

Privileged quarantine can bypass a sibling's mandatory host review

quarantined_abandoned cancels nonterminal siblings and closes the task after effect quiescence, but it does not require every affected sibling's host-apply review to be complete or bind those ledger fingerprints. Root management currently blocks unresolved markers only on nonterminal tasks. A sibling with an unknown host entry can therefore lose its root-management barrier when another sibling is quarantined.

Required correction: permanent quarantine must either bind and acknowledge every affected sibling's exact repository-change and host-ledger fingerprints, or append a separate privileged repository-abandonment acknowledgement. An unresolved review/marker remains a project-root management barrier even after task terminalization until that explicit evidence-bound action exists. Normal quarantine must never turn unknown physical work into permission to repoint, delete, or reuse the root.

These are evidence-state findings. They do not assert that a future containment or fingerprint implementation is correct.

@Joncallim Joncallim changed the title [architecture] Define bounded context issuance and evidence for #179 Implement bounded context issuance and evidence for #179 Jul 22, 2026
@Joncallim

Copy link
Copy Markdown
Owner Author

Takeover handoff — held after orthogonal pass

All local commits are now pushed. Remote head is ba24cc2d96614cb6ca3c7990d84109bdb83ddf64; there are no local-only implementation commits required to resume.

The latest independent pass is blocked on five items:

  1. Sandbox TOCTOU: path validation and later open(..., 'wx') can be separated by a parent-directory symlink/reparent swap, redirecting model output outside .forge/task-runs.
  2. Unconfined ACP default: work-package execution is allowed when its flag is absent and spawns an unconfined host process, bypassing the sandbox-only/direct-writer boundary.
  3. Clarification durable duplication: generic API/SSE/log projections are redacted, but raw question/suggestions/answer remain in ordinary task_questions and are reread for replanning instead of using only protected audited history.
  4. Local recovery authority: the fixed recovery routine trusts a caller/public marker fingerprint instead of recomputing the locked evidence digest and does not reject successful or packet-linked evidence.
  5. PostgreSQL test isolation: release-recorder and S4 PostgreSQL files run in parallel against one database and both claim signer policy generation 1, causing nondeterministic unique-key failures and eight skipped S4 tests. Isolate the S4 suite in its own freshly migrated database and command.

Passing baseline before the hold:

  • 1,620 unit tests passed; 15 environment-gated tests skipped.
  • Lint with --max-warnings=0, TypeScript, and production build passed.
  • Full distinct-principal 0026→0027 upgrade/recovery/archive/session/root cutover proof passed.
  • Isolated S4 PostgreSQL suite passed 8/8 with zero skips.
  • The exact combined CI-style PostgreSQL run reproduced the shared-database collision.

Resume order:

  1. Architect/Security decision for ACP and sandbox confinement.
  2. Backend/DB fixes for protected clarification storage and local-recovery evidence binding.
  3. DevOps/QA isolation of the mandatory PostgreSQL suites.
  4. Run full local gates and distinct-principal PostgreSQL proofs.
  5. Resume fresh Review/Security/QA passes only when the operator lifts the hold.
  6. Reply inline and resolve the seven existing review threads only after their concrete fixes and validation are pushed.

PR #198 remains open and must not be merged yet. The head branch is stacked under later PRs, so preserve it.

@Joncallim

Copy link
Copy Markdown
Owner Author

Live CI addendum after push

The first hosted Web CI run on ba24cc2d failed before reaching the known PostgreSQL-suite collision:

Add this as the first DevOps handoff item: install/provision the Redis CLI in Web CI before running the 0027 proof, then continue to the separately documented shared-database PostgreSQL test isolation fix. This PR remains held and not merge-ready.

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.

2 participants