test(factory): cover budget recovery and missing checkpoint proof

[Bakar404]

Description

Refs #50. Ready for fresh review: non-closing Factory budget-recovery tooling and evidence.
The original budget-revision feature shipped in #77; auditor-led policy #236 and
broader recovery acceptance remain separate. No issue closure or merge is requested.

• Add owned synthetic recovery, revised-budget hard-stop and missing-checkpoint
  scenarios, retaining usage, source/session/workspace lineage and verifier policy.
• Harden shared QA/CI drivers and Windows process/source ownership, including
  external-adapter lifecycle, source pinning and platform evidence.
• Address the current review: native process identity/handle cleanup instead of
  reserved $pid and CIM/native timestamp mixing; unconditional reserved-port
  rejection; identical preview/create source plus persisted task/run assertions;
  marker cleanup before resume; explicit PostgreSQL17 discovery and omitted CI tests.
• Integrate current main while preserving both budget-recovery and steering-
  contention synthetic-provider modes. No production Rust, web-app, migration or
  root dependency/lockfile changes relative to main.

Base: main aa2ef457d86d727232ae353f53a13c8c9f149bf6.
Final head: 6e69bc543ef16d11ecf4baf5f2488a6a7b478100.
Branch: codex/issue-50-factory-recovery -> main, published history preserved.
Actual full scope:45 files, including shared CI, cross-platform fixture drivers,
process/source helpers, focused tests and historical/new evidence. The old11-file
description was obsolete. The review correction itself is bounded to the approved
fixture/CI/test/report paths; no new execution authority or parallel harness is added.

Harness-first: reuse existing native Codex session/resume, Factory/controller,
budget revision, verifier/review and local_stack.psm1 ownership/handle helpers.
factory_budget_process.mjs is a thin adapter, not another process supervisor.

Type of change

• Bug fix (non-breaking fixture safety and integration corrections)
• Test/CI tooling and documentation update

How Has This Been Tested?

Final integrated-source local gates:

• Migration checker:41 immutable migrations unchanged.
• cargo fmt --check: passed.
• cargo clippy --workspace --all-targets --offline --locked -- -D warnings: passed.
• cargo test --workspace --offline --locked -- --quiet: 554 passed,343 ignored, zero failures. Ignored:1 runner,5 server,337 store; not executed by that generic gate.
• Frontend/office suite: 310 passed; web build/lint passed.
• Five focused review suites: 44 passed, zero skipped. Includes actual Windows
  receipt stop/mismatch preservation, actual marker release/failure behavior, a
  real junction refusal through the driver's source guard, serialized source
  propagation and task/run-source mismatch rejection.
• Related ownership/CI suite with ECORP_OWNED_PROCESS_TEST=1: 12 passed, none
  skipped, including actual startup, two restarts, database-drift refusal and
  idempotent stop. An earlier broader53-case pass had one opt-in skip; that native
  case was subsequently executed, not silently counted as passing.
• Actual extracted CI PostgreSQL-discovery block: supported installed17 accepted;
  missing tools rejected before startup. No toolchain download performed.
• New-change whitespace/syntax checks passed; actual server/runner/CLI binaries rebuilt.

Fresh owned native runtime tests:

1. Bounded Factory recovery: passed.
2. Revised-budget overrun/hard stop: protected.
3. Native missing-checkpoint recovery: passed, with no marker left before resume.
4. Separate Windows external-adapter fixture: source-pinned Claude/OpenCode
   synthetic lifecycles, artifacts, mixed-provider graph, controlled readiness,
   synthetic identity lifecycle and budget breakers passed.

Every owned QA service was stopped with identity verification; no QA listeners
remained. Data, failures and source/worktrees are retained. No retained office,
private DB or credential was borrowed/reset; no real provider inference or remote
GitHub publication occurred in runtime QA.

Fresh review report and exact run/fixture evidence.
It distinguishes pre-commit working-source tests, final UI-only main refresh,
historical reports and current results. Old hosted checks are historical; evaluate
new hosted CI against this head. No new browser/production-auth/real-provider
qualification is claimed. Screenshots do not apply to these CLI/fixture corrections.

Test Configuration: Windows ARM64; native PostgreSQL17; Node24.19.0,
pnpm11.19.0, Cargo1.98.1; locked/offline dependencies; synthetic Codex and
Claude/OpenCode protocol fixtures plus fake GitHub responses.

AI agent integration handoff

• Preserve native ownership through stop: exact executable, native creation time,
  workspace and retained handle; never restore PID-only or uncertain cleanup.
• Preview and create must retain identical immutable source; missing/mismatched
  persisted task/run binding cannot count as acceptance. CI flags are not evidence
  that a manual port or database is disposable.
• PR feat(factory): pin and expose the shared claim authority #255/Require a shared claim authority for contributor factories on the same backlog #161 reuse related QA foundations but retain their own pinned historical
  evidence. Coordinate changes; do not rewrite their old dependency patch or claim
  these fixtures implement the shared-authority feature.
• Keep Make budget-suspended missions recoverable with an authorized budget revision #50 open: already-terminal Factory cancellation, broader lost-response/
  reconnect/browser recovery and other documented scenarios remain separate.
  Add auditor-led budget extensions without cancelling healthy missions #236 does not gain auditor grants or increased allowances from this PR.
• Never rerun a preparation/reset against occupied or unknown resources. Inspect
  retained receipts and authorize exact new-fixture scope first. Preserve failures.

Checklist

• Current contributor/product/security/evaluation guidance followed
• Reviewed all visible and suppressed comments, not only inline threads
• Actual cleanup/source/marker regressions and full native scenarios passed
• All six repository gates passed; ignored, historical and unrun coverage explicit
• Description now matches full45-file CI/fixture/evidence scope
• New head submitted for independent review; human request-changes not dismissed

The earlier draft-only wording is superseded by the operator-approved Ready
handoff. This is review-only publication: no issue closure, merge, auto-merge,
deployment, cloud change, original-office restart or real-backlog dispatch.

[Copilot]

🟡 Changes recommended

Unresolved critical process/source-ownership issues and moderate CI/fixture findings must be addressed before approval.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds non-production Factory budget-recovery and missing-checkpoint fixtures, cross-platform fixture hardening, CI coverage, and dated evidence.

Changes:

• Adds synthetic checkpoint-resume and revised-budget hard-stop scenarios.
• Strengthens fixture ownership, process cleanup, source validation, and checkpoint handling.
• Records validation results, evidence, and remaining limitations.

Review findings: Critical (1 vote each): handle-based cleanup and canonical synthetic-repository validation are required in the recovery fixture and Windows stack. Moderate (1 vote each): CI needs validated PostgreSQL provisioning and focused tests; external adapters need source propagation and reserved-port rejection; checkpoint markers need cleanup or relocation. Nit (1 vote each): reconcile CI scope and hosted-CI evidence claims.

File summaries

File	Reviewed change
tools/task_graph_fixture.test.mjs	Task-graph fixture contract tests.
tools/task_graph_fixture.mjs	Task-graph ownership and source selection.
tools/qa_hold_checkpoint_file.ps1	Checkpoint read-lock injection; marker cleanup finding noted.
tools/owned_test_stack.test.mjs	Owned server lifecycle tests.
tools/owned_test_process_linux.py	Linux pidfd process control.
tools/owned_test_process_linux_test.py	Linux process-control tests.
tools/identity_fixture.test.mjs	Identity fixture tests.
tools/identity_fixture.mjs	Identity lifecycle fixture.
tools/fixtures/owned_test_server.mjs	Inert owned test server.
tools/fake_codex_budget_stream.test.mjs	Synthetic Codex protocol tests.
tools/factory_budget_fixture_config.test.mjs	Budget fixture configuration tests.
tools/factory_budget_fixture_config.mjs	Fixture modes and reference validation.
tools/e2e_task_graph.mjs	Task-graph E2E coverage.
tools/e2e_identity.mjs	Identity lifecycle E2E coverage.
tools/e2e_factory_publication.mjs	Factory publication regression coverage.
tools/e2e_factory_claims.mjs	Factory claims and verification coverage.
tools/e2e_external_adapters.test.mjs	External adapter contract tests.
tools/e2e_external_adapters.mjs	External adapter driver; source and port findings noted.
tools/e2e_external_adapters_http.test.mjs	HTTP and child-process boundary tests.
tools/e2e_budgets.mjs	Budget breaker E2E coverage.
tools/e2e_artifact_staging.mjs	Artifact and readiness coverage.
tools/controlled_runner_fixture.test.mjs	Controlled runner tests.
tools/controlled_runner_fixture.mjs	Source-bound runner readiness helpers.
tools/ci_runner_tests.test.mjs	CI runner contract tests.
tools/ci_owned_test_server.test.mjs	CI server ownership tests.
tools/ci_owned_test_server.mjs	Owned CI server lifecycle.
tools/ci_factory_repository.test.mjs	Factory repository checks.
tools/budget_fixture.test.mjs	Budget fixture readiness tests.
tools/budget_fixture.mjs	Budget fixture configuration and assertions.
scripts/fake-codex-app-server.mjs	Synthetic Codex provider behavior.
docs/evidence/2026-09-12-main-sync-ci-validation.md	Main-sync validation evidence.
docs/evidence/2026-09-11-issue-50-review-handoff.md	Review handoff and scope.
docs/evidence/2026-09-11-issue-50-recovery-baseline.md	Recovery baseline evidence.
docs/evidence/2026-09-11-issue-50-missing-checkpoint-result.md	Missing-checkpoint result.
docs/evidence/2026-09-11-issue-50-missing-checkpoint-preflight.md	Missing-checkpoint preflight.
docs/evidence/2026-09-11-issue-50-factory-budget-recovery.md	Budget recovery evidence.
docs/EVALS.md	Evidence-scope guidance.
.github/workflows/ci.yml	CI fixture and platform lanes; provisioning, test coverage, and scope findings noted.

Review details

Suppressed comments (8)

.github/workflows/ci.yml:286

• These steps depend on $env:PGBIN, but the workflow neither provisions nor validates that runner-image variable before passing it to a script that requires an absolute bin directory. That makes the Windows fixture non-reproducible outside the particular image described in the evidence, and an image change will fail both the dry-run and execution before the fixture starts. Pin/provision PostgreSQL (or add an explicit, validated discovery step) in the job rather than relying on an undocumented image environment variable.

        run: ./tools/ci_external_adapters_windows.ps1 -FixtureRoot (Join-Path $env:RUNNER_TEMP 'ecorp-external-adapters-ci') -PgBin $env:PGBIN -DryRun
      - name: Verify external-adapter lifecycle and cross-provider graph
        shell: pwsh
        run: ./tools/ci_external_adapters_windows.ps1 -FixtureRoot (Join-Path $env:RUNNER_TEMP 'ecorp-external-adapters-ci') -PgBin $env:PGBIN -Execute

.github/workflows/ci.yml:31

• The CI Node test command omits the newly added tools/factory_budget_fixture_config.test.mjs (and the updated tools/fake_codex_budget_stream.test.mjs). Consequently the new fixture-mode/reference-snapshot guards are not exercised by this workflow and regressions can pass CI; add these focused tests to the quality test list.

        run: node --test tools/e2e_external_adapters.test.mjs tools/e2e_external_adapters_http.test.mjs tools/task_graph_fixture.test.mjs tools/controlled_runner_fixture.test.mjs tools/ci_factory_repository.test.mjs tools/owned_test_stack.test.mjs tools/ci_owned_test_server.test.mjs tools/identity_fixture.test.mjs tools/ci_runner_tests.test.mjs tools/fixture_source_identity.test.mjs tools/budget_fixture.test.mjs

.github/workflows/ci.yml:270

• The stated 11-file local test/evidence scope is no longer accurate: this new CI job is part of a broader diff that also modifies existing E2E drivers, process helpers, and repository evidence files. Please reconcile the PR description with the actual scope so reviewers can assess the added CI/runtime-fixture behavior correctly.

  external-adapters-windows:
    runs-on: windows-latest
    timeout-minutes: 20
    steps:
      - uses: actions/checkout@v4

docs/evidence/2026-09-12-ci-external-adapter-platforms.md:11

• This evidence file records hosted CI runs on the published PR head and later hosted follow-ups, but the PR description explicitly says that no hosted-CI result is claimed. Please reconcile the scope or label these as prior/external observations; as written, the evidence and handoff make incompatible claims about what was validated.

Hosted CI run `34676356598`, integration job `103506808050`, failed on published
head `f3bb566df718f21257de038edb92d119d10f015a`. The Claude/OpenCode sample received
HTTP 409: `requires adapter claude-code, but that adapter is unavailable`.
The other five jobs passed; the subsequent integration steps were skipped.

tools/e2e_external_adapters.mjs:144

• On Windows, source is used only for the readiness preview, but the mission creation request sent here omits it. The server only pins the source when the create request contains it (crates/crony-server/src/main.rs:2598-2600), so the real tasks/runs are created without the checked immutable source and can be scheduled against a different checkout. Include the exact source in mission creation and assert the persisted task/run source in the fixture mock/test.

    const mission = await ok(`${prefix}/missions`, missionRequest)

tools/e2e_external_adapters.mjs:28

• Allowing reserved/manual ports whenever GITHUB_ACTIONS and CI are set defeats the owned-fixture boundary: those flags do not prove that ports such as 8791 or 8793 are disposable, while the next request performs a mutating /api/demo/reset. The workflow uses dedicated ports instead, so these ports should be rejected unconditionally and the exception test removed.

  assert.ok(!manualPorts.has(endpoint.port) || (env.GITHUB_ACTIONS === 'true' && env.CI === 'true'),
    'Refusing a manual-stack port outside the disposable GitHub Actions job')

tools/e2e_factory_budget_recovery.mjs:160

• The retained qa\source directory is passed to Git and the runner without first resolving it. An existing junction/symlink can therefore redirect fixture writes outside the owned QA root, defeating the redirect-rejection boundary. Add a realpath(source) === path.resolve(source) check before this call.

  assert.equal((await run('git',['-C',source,'status','--porcelain'])).stdout.trim(), '')

tools/qa_hold_checkpoint_file.ps1:36

• The readiness marker is written inside the runner worktree and this helper never removes it. After the lock is released, .qa-checkpoint-lock-ready remains as an untracked QA file, so the resumed same-workspace run operates on extra state that was not present in the captured checkpoint and the retained worktree is left contaminated. Put the synchronization marker outside the worktree or remove it in the driver immediately after the lock helper exits (including failure cleanup).

• Files reviewed: 42/42 changed files
• Comments generated: 3
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Bakar404]

Coordination reference: #161 is now assigned to @Bakar404 for shared claim authority / multi-system execution. The broader multiplayer workstream #239 already identifies #161 as groundwork to reuse.

This PR is related to #161 only through shared QA tooling, including tools/e2e_factory_claims.mjs, tools/owned_test_stack.mjs, and docs/EVALS.md. The #161 contribution should reuse or coordinate these changes rather than introduce a competing fixture implementation.

This is a non-closing cross-reference: #237 remains the budget-recovery test/evidence contribution related to #50; it does not implement or close #161. Merged #162 and #191 provide earlier shared-authority/contributor guidance, but enforcement and multi-host acceptance remain open under #161.

No change to this PR's implementation scope, review verdict, merge authority, or execution policy is implied.

[shyamsridhar123]

Reviewed commit 843130639c1429bc0e72aa974f4dfea488fbb64b against ECorp's product, architecture, security, and eval contract.

Changes required: Windows cleanup defect.

• [P2] tools/e2e_factory_budget_recovery.mjs:77: stopVerifiedChild assigns to $pid while ErrorActionPreference is Stop. PowerShell names are case-insensitive and $PID is a read-only automatic variable, so every live non-PostgreSQL cleanup attempt throws before Get-Process/Kill, preserves the server/runner and makes the fixture exit nonzero. Reproduced locally: 'Cannot overwrite variable PID because it is read-only or constant.' Use a nonreserved variable or reuse the existing native ownership helper, and exercise the real cleanup path.

Validation: Reviewed shared CI diff, Windows fixture supervisor, native Linux pidfd ownership helper, owned server lifecycle, and budget-recovery orchestration. Seven hosted checks pass. Independently reproduced PowerShell's read-only PID assignment failure without starting or stopping any fixture process. Full recovery scenarios were not rerun.

• PR body still describes 11 files and no shared CI changes, but this head changes 42 files including CI/platform fixtures. Update the description and exact-head validation after fixing cleanup.

• Creation-time comparison also merits checking: the fixture records CIM timestamps and compares native ticks. This secondary concern was not independently reproduced and is not the basis of the request-changes finding.

[Bakar404]

Addressed the review at 6e69bc543ef16d11ecf4baf5f2488a6a7b478100, including the suppressed Copilot findings and Shyam's September17 request-changes.

• Replaced the budget driver's read-only $pid assignment and CIM/native timestamp mismatch with the existing native identity/retained-handle helpers. Wrong executable/time/workspace receipts preserve the child; the correct receipt stops it.
• Retained and tested canonical synthetic-source admission, including an actual junction refusal before Git access.
• Bound Windows preview/create and persisted task/run records to the same immutable source; removed the CI exception for reserved manual ports.
• Cleaned the checkpoint handshake marker on normal/failure paths before resume; preserved unknown pre-existing files.
• Added missing budget/provider/process tests and explicit validated PostgreSQL17 discovery to CI. Updated the full45-file scope, source pins and historical-versus-current evidence in the PR description.

Fresh results:44 focused review tests;12 native ownership/CI tests;310 frontend tests;
554 Rust passed/343 ignored; all six repository gates passed. All three native
Factory recovery scenarios passed/protected, and the separate Windows external-
adapter/source lifecycle fixture passed. Owned services were verified stopped;
no production identity, real provider or original office was used.

Exact evidence and limits.
The human request-changes review is deliberately not dismissed. Please review the
new head; #50 and the documented broader recovery gaps remain open. No merge or
deployment is performed by this follow-up.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[shyamsridhar123]

Reassessed 6e69bc543ef16d11ecf4baf5f2488a6a7b478100 against main 0b1ad59da398e3dbd6a696d0264bcb6ebd620219 and the current product, architecture, security and evaluation contracts.

The earlier reserved-PID/process-time, source-admission, source-pinning, reserved-port and checkpoint-marker findings are addressed in this revision. The process adapter reuses local_stack.psm1; it does not introduce a second ownership mechanism. Independently executed on this exact head: 44 corrected regressions, 12 ownership/CI tests with native process testing enabled, and 87 related fixture tests. All 143 passed, with no failures or skips. The owned child processes are stopped and both review worktrees remain clean. All 12 current-head GitHub checks, including CodeQL, integration and Windows external adapters, are successful; no unresolved review threads or open PR code-scanning alerts were returned.

Remaining merge hold: the Development association still closes #50. At 04:17 UTC on September 18, GraphQL closingIssuesReferences(userLinkedOnly:true) returned the open #50 for this PR. That contradicts the deliberately non-closing, bounded test/evidence scope and the PR body's own metadata hold. Remove that closing association in GitHub's Development control while retaining the textual Refs #50. The available authenticated API operations do not expose that edit, so I have not claimed to remove it or merged around it.

The new report docs/evidence/2026-09-17-pr237-review-followup.md distinguishes its author-run six contributor gates and three synthetic native recovery scenarios from historical evidence and wider #50 acceptance. Those full gates and scenarios were not independently rerun in this follow-up; the 143 tests above are the independent validation performed here. Once the closing association is removed, finish the required current-revision integration validation and fresh approval before merge. The original reserved-PID defect must not be carried forward as a current code finding.

[shyamsridhar123]

Status correction for 6e69bc543ef16d11ecf4baf5f2488a6a7b478100, assessed against main 0b1ad59da398e3dbd6a696d0264bcb6ebd620219: GitHub now returns no closing issue references, both with userLinkedOnly: true and without that filter. The #50 closing-association hold in my previous review is resolved. The textual Refs #50 can remain.

The implementation, review discussions and prior independent validation are unchanged. The 143 passing focused tests remain evidence from the previous pass; they were not rerun for this metadata correction. All 12 observed current-head hosted checks are successful.

The remaining hold is completion evidence: the description still presents a review-only handoff, and the required current-revision integration validation remains outstanding. A completed handoff for this bounded, non-closing scope and that validation are needed for fresh approval. The author-reported six contributor gates and native recovery scenarios remain distinct from the independent tests already recorded.

Neither the old PID/timestamp defects nor the #50 closing association are outstanding findings. This update asserts no new code defect and does not require this contribution to close the broader #50 work.