Skip to content

fix(workflows): execute admitted workflow runs - #495

Open
andrei-hasna wants to merge 11 commits into
mainfrom
fix/dbac6d69-workflow-executor
Open

fix(workflows): execute admitted workflow runs#495
andrei-hasna wants to merge 11 commits into
mainfrom
fix/dbac6d69-workflow-executor

Conversation

@andrei-hasna

@andrei-hasna andrei-hasna commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Atomically claim, advance, and admit dependency-ready workflow branches as independent background-agent runs.
  • Add the app-server workflow supervisor that reconciles terminal workers, runs deterministic artifact and command verifiers, and admits successors.
  • Preserve the source thread goal, declared artifact contract, terminal worker evidence, lease fencing, and duplicate-admission idempotency.
  • Remove the disabled verifier-executor state and fail closed on sandbox, path, timeout, output-limit, and verifier mismatches without persisting command output.

Regression and validation

  • Regression-first red: Blacksmith run 31234819100 at b355eb7, rc 101; start returned pending instead of running.
  • Prior implementation suite: Blacksmith run 31235385785 at 6218f87, success.
  • Exact-head remote matrix: Blacksmith run 31236388250 at 53a3beb (in progress when this PR was opened).
  • Local Rust build/test intentionally not run; repository policy requires the remote Testbox lane.
  • cargo fmt --all: rc 0.
  • Staged secret/co-author scan and pre-push scan: rc 0 for every commit and push.

Task

OPE2-00241 / Todos dbac6d69-799a-422f-96b0-5737e15a8876.


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

@andrei-hasna

Copy link
Copy Markdown
Contributor Author

Exact-head focused validation passed on 53a3bebeb5a460a1d7fc625d677a27ca75110659.

Blacksmith run 31236388250, job 93049541290, completed 2026-08-08 03:10:29Z. The app-server runtime suite passed 6/6, and the focused manager/state lifecycle tests all passed: atomic start and successor admission, verifier ready-without-execution state, failing verifier terminal behavior, branch success/failure evidence persistence, and stale-generation/idempotent admission. The build command exited 0.

Regression-first evidence remains run 31234819100 at test-only commit b355eb73253199dffb24f5c961b5a26dff9d08c2: expected running, observed pending, rc 101.

@andrei-hasna

Copy link
Copy Markdown
Contributor Author

[COORDINATOR BASE GATE — OPE2-00241] Rebase fix/dbac6d69-workflow-executor onto current main 17e7dae before review. Current head 53a3beb is still based on d39186d; branch tree 3cbbaef7161a035df3594d5c9f3e3dcebbefaa1f differs from the current landing tree 144d69f08df225552b723ce2f4e9629b12820746. After safe rebase/force-with-lease, run the required secrets scan and remote exact-head matrix, then report the new clean head/base/tree evidence. Exactly one independent reviewer will be fixed only against that rebased candidate. Do not merge, publish, install, activate the preserved iapp-news workflow, or mutate live state.

@andrei-hasna

Copy link
Copy Markdown
Contributor Author

[REVIEW] NO_GO — #495 @ 53a3beb — lens: correctness+security+gates, reviewer Aruns (1 of 1)

Exact candidate and source read:

  • Confirmed HEAD is 53a3bebeb5a460a1d7fc625d677a27ca75110659 and origin/main is 17e7dae867314fb68dd38569112eb35596879c7c.
  • Read git log --oneline origin/main..HEAD (rc 0), git diff origin/main...HEAD --stat (rc 0), the complete 1,813-line diff for all 8 changed files (rc 0), and surrounding workflow-manager, workflow-spec validation, state-orchestrator, verifier-claim/result, background-agent admission, supervisor, execution-snapshot, and status-snapshot source.

Blocking P0/P1 findings:

  • P1 — admitted workflow branches cannot enter the background-agent executor. Both production start callers pass version_fingerprint: None (thread_workflow_processor.rs:326, manager_tool.rs:345), and create_background_branch_run_if_missing_in_tx persists that value into the background-agent run (workflow_orchestrator.rs:1349). Its branch_execution_payload also does not populate the required packageFingerprint. The background-agent runtime rejects this exact state: should_start_background_run requires the installed admission-schema fingerprint, while background_agent_admission_is_ready requires an initial_execution_context whose packageFingerprint matches the installed runtime (background_agent_live.rs:3158; background_agents/runs.rs:1486-1521). Consequently the new start path can return running with an active/queued branch, but that branch is ineligible for supervisor execution and can never reach completion, verifier execution, or successor admission. The added tests manually terminalize queued rows and therefore do not exercise the real worker-admission boundary. Remedy: build workflow branches through the same frozen, compatibility-stamped background-agent admission path used by executable agents (including schema/package fingerprints and a valid execution context), then add an integration regression proving a started workflow's actual worker leaves queued and reaches its verifier.

Declared setup/gates:

  • bun install — rc 0; setup only, not a test gate; 521 packages installed; pass/fail count not applicable.
  • The repository package.json declares no test script: there is no repository test gate.
  • The repository package.json declares no typecheck script.
  • gh pr view 495 --repo hasna/codewith --json statusCheckRollup — rc 0; authoritative forge checks at review time: 5 SUCCESS, 0 FAILURE, 18 IN_PROGRESS, 3 SKIPPED.
    • Bazel test on ubuntu-24.04 for x86_64-unknown-linux-gnu — IN_PROGRESS
    • cargo-deny — IN_PROGRESS
    • build-test — IN_PROGRESS
    • Build Codewith Linux CLI — IN_PROGRESS
    • cla — SUCCESS
    • Check for spelling errors — SUCCESS
    • Blob size policy — SUCCESS
    • Detect changed areas — SUCCESS
    • python-sdk — SUCCESS
    • Bazel test on ubuntu-24.04 for x86_64-unknown-linux-musl — IN_PROGRESS
    • Bazel test on windows-latest for x86_64-pc-windows-gnullvm shard 1/4 — IN_PROGRESS
    • Format / etc — IN_PROGRESS
    • sdks — IN_PROGRESS
    • Bazel test on windows-latest for x86_64-pc-windows-gnullvm shard 2/4 — IN_PROGRESS
    • Bazel test on windows-latest for x86_64-pc-windows-gnullvm shard 3/4 — IN_PROGRESS
    • Bazel test on windows-latest for x86_64-pc-windows-gnullvm shard 4/4 — IN_PROGRESS
    • cargo shear — IN_PROGRESS
    • Bazel test on windows-latest for x86_64-pc-windows-gnullvm (native main) — SKIPPED
    • Bazel clippy on ubuntu-24.04 for x86_64-unknown-linux-gnu — IN_PROGRESS
    • Argument comment lint package — SKIPPED
    • Bazel clippy on windows-latest for x86_64-pc-windows-gnullvm — IN_PROGRESS
    • Verify release build on ubuntu-24.04 for x86_64-unknown-linux-gnu — IN_PROGRESS
    • Argument comment lint - Linux — IN_PROGRESS
    • Verify release build on windows-latest for x86_64-pc-windows-gnullvm — IN_PROGRESS
    • Argument comment lint - Windows — IN_PROGRESS
    • [code]smith — SKIPPED

Non-blocking follow-ups:

  • The absence of declared test and typecheck scripts is recorded as a repository-gate gap; I did not invent a replacement command and did not use an undeclared local Rust command as a blocker.
  • No additional P2/P3 follow-ups.

@andrei-hasna

Copy link
Copy Markdown
Contributor Author

[REVIEW] NO_GO — #495 @ 37cf2de — lens: correctness+security+gates, reviewer Aruns (1 of 1)

Focused re-review of the named blocker:

  • Read the complete 53a3bebeb5a460a1d7fc625d677a27ca75110659..37cf2deb793cc0762129101f31c04904b7ed296d diff (rc 0): one new 74-line test in workflow_verifiers.rs covering stale running-verifier recovery after lease takeover.
  • The new commit does not change either production workflow-start caller, branch admission, compatibility fingerprints, execution context, or background-agent supervisor eligibility.

Blocking P0/P1 findings:

  • P1 remains — admitted workflow branches cannot enter the background-agent executor. thread_workflow_processor.rs:326 and manager_tool.rs:345 still pass version_fingerprint: None; workflow_orchestrator.rs:1349 still persists that incompatible value, and branch_execution_payload still omits the required packageFingerprint. background_agent_live.rs:3158 requires the installed admission-schema fingerprint before a queued run can start, and background_agents/runs.rs:1486-1521 requires a matching packageFingerprint before admission is ready. The new verifier recovery test is downstream of this boundary and cannot make the worker leave queued. Remedy remains: admit workflow branches through the compatibility-stamped executable background-agent path, with a valid frozen execution context, and add an integration regression proving a real started workflow worker leaves queued and reaches its verifier.

Declared setup/gates:

  • bun install — rc 0 on this review worktree; setup only, not a test gate; 521 packages installed; pass/fail count not applicable.
  • package.json still declares no test script and no typecheck script; no local test or typecheck gate exists.
  • gh pr view 495 --repo hasna/codewith --json statusCheckRollup — rc 0 on the current head; 9 SUCCESS, 0 FAILURE, 14 IN_PROGRESS, 3 SKIPPED.
    • Bazel test on ubuntu-24.04 for x86_64-unknown-linux-gnu — IN_PROGRESS
    • Build Codewith Linux CLI — IN_PROGRESS
    • cla — SUCCESS
    • Check for spelling errors — SUCCESS
    • Blob size policy — SUCCESS
    • cargo-deny — SUCCESS
    • build-test — SUCCESS
    • Detect changed areas — SUCCESS
    • python-sdk — SUCCESS
    • Bazel test on ubuntu-24.04 for x86_64-unknown-linux-musl — IN_PROGRESS
    • Bazel test on windows-latest for x86_64-pc-windows-gnullvm shard 1/4 — IN_PROGRESS
    • sdks — IN_PROGRESS
    • Format / etc — SUCCESS
    • Bazel test on windows-latest for x86_64-pc-windows-gnullvm shard 2/4 — IN_PROGRESS
    • Bazel test on windows-latest for x86_64-pc-windows-gnullvm shard 3/4 — IN_PROGRESS
    • Bazel test on windows-latest for x86_64-pc-windows-gnullvm shard 4/4 — IN_PROGRESS
    • Bazel test on windows-latest for x86_64-pc-windows-gnullvm (native main) — SKIPPED
    • cargo shear — SUCCESS
    • Bazel clippy on ubuntu-24.04 for x86_64-unknown-linux-gnu — IN_PROGRESS
    • Argument comment lint package — SKIPPED
    • Bazel clippy on windows-latest for x86_64-pc-windows-gnullvm — IN_PROGRESS
    • Verify release build on ubuntu-24.04 for x86_64-unknown-linux-gnu — IN_PROGRESS
    • Argument comment lint - Linux — IN_PROGRESS
    • Verify release build on windows-latest for x86_64-unknown-linux-gnu — IN_PROGRESS
    • Argument comment lint - Windows — IN_PROGRESS
    • [code]smith — SKIPPED

Non-blocking follow-ups:

  • The absence of declared test and typecheck scripts remains a repository-gate gap; no undeclared command was invented or used as a blocker.
  • No additional P2/P3 follow-ups.

@andrei-hasna
andrei-hasna force-pushed the fix/dbac6d69-workflow-executor branch from 37cf2de to 75c697f Compare August 8, 2026 04:08
@andrei-hasna

Copy link
Copy Markdown
Contributor Author

[REVIEW] NO_GO — #495 @ 75c697f — lens: correctness+security+gates, reviewer Aruns (1 of 1)

Exact candidate and source read

  • Confirmed local HEAD 75c697f and origin/main 17e7dae.
  • Ran git log --oneline origin/main..HEAD (exit 0), git diff origin/main...HEAD --stat (exit 0), git diff --name-status origin/main...HEAD (exit 0), and read the complete origin/main...HEAD diff for all 12 changed files plus the surrounding workflow admission, background-agent configuration/status, verifier execution, lease/reclaim, and app-server start paths.
  • Change size read: 2,083 insertions and 106 deletions across 12 files.

Declared setup and gates

  • bun install — exit 0. Setup only, not a test gate. Output: 521 packages installed.
  • package.json declares no typecheck script.
  • package.json declares no test script, so this repo declares no package-level test gate. That absence is a gate-coverage finding; I did not invent a replacement command.
  • gh pr view 495 --repo hasna/codewith --json statusCheckRollup — exit 0. Read 26 forge checks: 7 SUCCESS, 2 FAILURE, 14 PENDING, 3 SKIPPED.
    • build-test — FAILURE
    • Bazel test on ubuntu-24.04 for x86_64-unknown-linux-gnu — PENDING
    • Build Codewith Linux CLI — PENDING
    • cla — SUCCESS
    • Check for spelling errors — SUCCESS
    • Blob size policy — SUCCESS
    • cargo-deny — SUCCESS
    • Detect changed areas — SUCCESS
    • python-sdk — SUCCESS
    • Bazel test on ubuntu-24.04 for x86_64-unknown-linux-musl — PENDING
    • Format / etc — FAILURE
    • Bazel test on windows-latest for x86_64-pc-windows-gnullvm shard 1/4 — PENDING
    • sdks — PENDING
    • Bazel test on windows-latest for x86_64-pc-windows-gnullvm shard 2/4 — PENDING
    • Bazel test on windows-latest for x86_64-pc-windows-gnullvm shard 3/4 — PENDING
    • Bazel test on windows-latest for x86_64-pc-windows-gnullvm shard 4/4 — PENDING
    • Bazel test on windows-latest for x86_64-pc-windows-gnullvm (native main) — SKIPPED
    • cargo shear — SUCCESS
    • Bazel clippy on ubuntu-24.04 for x86_64-unknown-linux-gnu — PENDING
    • Argument comment lint package — SKIPPED
    • Bazel clippy on windows-latest for x86_64-pc-windows-gnullvm — PENDING
    • Verify release build on ubuntu-24.04 for x86_64-unknown-linux-gnu — PENDING
    • Argument comment lint - Linux — PENDING
    • Verify release build on windows-latest for x86_64-pc-windows-gnullvm — PENDING
    • Argument comment lint - Windows — PENDING
    • [code]smith — SKIPPED
  • gh run view 31238772493 --repo hasna/codewith --job 93056037256 --log — exit 0. The build-test failure is the declared just fmt-check lane: Rust formatting failed in workflow.rs, workflow_orchestrator.rs, and workflow_verifiers.rs, ending with exit code 1.
  • gh run view 31238772482 --repo hasna/codewith --job 93056074622 --log — exit 1 because that workflow run was still in progress and its logs were not yet available.

Blocking P0/P1 findings

  1. P1 — isolated workflow branches are admitted into the app-server process directory instead of an isolated or source-thread workspace.

    • codex-rs/state/src/runtime/workflow_orchestrator.rs::workflow_branch_execution_identity ignores candidate.workspace_json when choosing the execution path and uses std::env::current_dir() for cwd and workspaceRoots.
    • branch_execution_payload persists managedWorktreeId as null even when the step declares workspace.mode: isolated_worktree.
    • This is currently reachable: codex-rs/prompts/templates/workflows/dental_lead_saas.yaml declares multiple parallel workspace-write build steps with mode: isolated_worktree. Those workers will share and mutate the app-server checkout rather than independent worktrees, risking cross-step corruption and changes in the wrong repository.
    • Required remedy: resolve the source thread/repository workspace, provision or attach the requested managed isolated worktree before admission, persist its stable ID and path, and add an integration test proving a source-thread cwd is honored and two isolated branches get distinct workspaces.
  2. P1 — workflow verifier ownership can requeue and duplicate a live workspace-writing verifier.

    • The owner is deterministically workflow-manager:<source_thread_id> in both start surfaces and the supervisor, so separate app-server/supervisor instances for the same thread are indistinguishable.
    • claim_workflow_run permits a same-owner reclaim before lease expiry, increments generation, and requeue_running_workflow_verifiers_in_tx moves every running verifier back to retryable Blocked. The new test explicitly exercises immediate same-owner reclaim as “restart,” but there is no supervisor-instance identity proving a restart occurred.
    • The supervisor executes verifier commands without renewing the 60-second workflow lease, while the shipped workflow template allows run_commands verifiers up to 1,200 seconds. A second process can therefore take over and run the same verifier again while the first command is still executing; stale-result fencing prevents the first result from recording but cannot undo duplicated filesystem/process side effects.
    • Required remedy: use a unique supervisor/executor instance identity, do not treat a live same-owner claim as restart recovery, heartbeat or otherwise hold the lease through command execution, and add concurrency coverage proving one long workspace-write verifier executes at most once across two supervisors/retried starts.
  3. P1 required gate failure — build-test and Format / etc are currently FAILURE. The captured build-test log shows the declared rustfmt gate failing. The formatting diff must be applied and the authoritative forge checks rerun.

Non-blocking follow-ups

  • The root package.json has neither a test nor typecheck script. This is explicitly reported as missing coverage, not used to invent or falsely block on an undeclared local command.
  • No other P0/P1 finding was identified in the exact candidate after the full diff and surrounding-source review. P2/P3 style or refactor observations are intentionally omitted.

Disposition
NO_GO. The isolation and verifier-lease remedies are real execution-safety work, so I am leaving the PR open. Do not merge this SHA.

@andrei-hasna

Copy link
Copy Markdown
Contributor Author

[EXECUTOR HANDOFF — OPE2-00241]

Exact candidate

  • PR head: 59d6ffe41b733ac0c28585d3b48129501a835e20
  • Remote branch: 59d6ffe41b733ac0c28585d3b48129501a835e20
  • Current origin/main, PR base, and merge-base: 17e7dae867314fb68dd38569112eb35596879c7c
  • Worktree is clean and tracks the remote branch.

Secret and provenance gates

Implementation staged scan:

0 /tmp/ope2-00241-gitleaks-staged.out
0 /tmp/ope2-00241-gitleaks-staged.err

Formatter staged scan:

0 /tmp/ope2-00241-format-gitleaks-staged.out
0 /tmp/ope2-00241-format-gitleaks-staged.err

Exact origin/main..HEAD range scan:

0 /tmp/ope2-00241-format-gitleaks-range.out
0 /tmp/ope2-00241-format-gitleaks-range.err

All scan commands exited 0. Every branch commit has canonical author/committer
Andrei Hasna <andrei@hasna.com>, exactly one Agent: vespasian trailer, and
no Co-Authored-By trailer.

Remote validation

  • Focused Blacksmith run: https://github.com/hasna/codewith/actions/runs/31239062609
  • Exact run head: 59d6ffe41b733ac0c28585d3b48129501a835e20
  • Result: FAILURE, exit 101.
  • cargo fmt --all -- --check advanced successfully into the test commands.
  • State admission test: test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 391 filtered out; finished in 0.12s
  • Same-owner verifier-recovery test: test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 391 filtered out; finished in 0.12s
  • Lease-takeover verifier-recovery test: test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 391 filtered out; finished in 0.11s
  • Actual-worker integration failed before admission with
    workflow YAML does not satisfy the workflow spec invariants.
  • Terminal line: test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 715 filtered out; finished in 0.22s
  • Clippy did not run because the && chain stopped at the failed integration.
  • No local Rust or Bazel command was run.

Forge snapshot at 2026-08-08T04:23Z: 15 success, 8 in progress, 3 skipped, 0
failed. build-test and Format / etc are success. The PR is MERGEABLE but
UNSTABLE while checks remain in progress.

Review termination

Fixed reviewer Aruns, 1 of 1, returned the third NO_GO:
#495 (comment)

Open concrete P1 findings:

  1. Isolated workflow branches currently persist the app-server process
    directory with managedWorktreeId: null; parallel workspace-writing branches
    therefore lack source-thread workspace resolution and distinct managed
    worktrees.
  2. The deterministic workflow-manager:<thread> owner, same-owner reclaim, and
    lack of a lease heartbeat through long verifier commands can duplicate live
    workspace-writing verifier side effects across two supervisors.
  3. The new actual-worker integration fixture is rejected by the workflow-spec
    invariants, so the required worker-leaves-queued-to-verifier proof is absent.

The bounded two-remediation-cycle policy terminates this lane on the third
NO_GO. No additional reviewer, code remediation, merge, install, live
activation, or production mutation was started.

Conditional future landing and rollback sequence

The steps below are evidence requirements for a future authorized lane. This
handoff does not authorize any of them.

  1. Keep this PR and iapp-news workflow run
    14c27756-857b-40f7-93d0-426450f3573b unmerged/unactivated while the owning
    coordinator routes the three open P1 items into the next authorized bounded
    remediation.
  2. Merge only a reviewed exact-head candidate after its required remote checks
    pass.
  3. Build and install that merged Codewith runtime on the intended target
    machine, preserving the previous known-good runtime artifact/version.
  4. Run the live acceptance path on a safe disposable workflow: a real worker
    leaves queued, uses the correct source or distinct isolated worktree, and
    its verifier completes exactly once across restart/takeover conditions.
  5. Activate the preserved iapp-news workflow only after that live proof passes.
  6. If install or live proof fails, keep iapp-news pending, reinstall the previous
    known-good Codewith runtime, revert the landing change, and re-run the prior
    runtime health check before any retry.

@andrei-hasna

Copy link
Copy Markdown
Contributor Author

[REVIEW] NO_GO — #495 @ 59d6ffe — lens: correctness+security+gates, reviewer unresolved-account001 (1 of 1)

Candidate and review scope

  • git rev-parse HEAD — exit 0: 59d6ffe41b733ac0c28585d3b48129501a835e20.
  • git rev-parse origin/main — exit 0: 17e7dae867314fb68dd38569112eb35596879c7c.
  • git log --oneline origin/main..HEAD — exit 0; 11 PR commits read.
  • git diff origin/main...HEAD --stat — exit 0; 12 files, 2,086 insertions and 106 deletions.
  • Read the complete diff for all 12 changed files and surrounding workflow admission, background-agent snapshot/config resolution, persisted thread-cwd, verifier claim/result, lease/fencing, app-server startup, and integration-test code.

Declared gates and commands actually run

  • bun install — exit 0; setup only, not a gate: 521 packages installed. The generated untracked bun.lock was removed; the worktree is clean.
  • package.json declares no typecheck script.
  • package.json declares no test script. This repository therefore declares no package test gate; I did not invent one and ran no lane-chosen test command.
  • gh pr view 495 --repo hasna/codewith --json statusCheckRollup — exit 0; authoritative forge rollup: 28 checks total, 20 SUCCESS, 5 FAILURE, 3 SKIPPED.
  • gh run view 31239002524 --repo hasna/codewith --log-failed — exit 0; diagnostic log retrieval only. Actual failing signal: error: aborting due to 32 previous errors; required summary: argument_comment_lint_prebuilt failed.
  • gh run view 31239002512 --repo hasna/codewith --log-failed — exit 0; diagnostic log retrieval only. Actual test signal: test result: FAILED. 40 passed; 1 failed; 0 ignored; 0 measured; 675 filtered out; actual Clippy signal: error: this loop could be written as a while let loop.

Forge checks read, by name and conclusion

  • FAILURE — Bazel test on ubuntu-24.04 for x86_64-unknown-linux-gnu
  • SUCCESS — cla
  • SUCCESS — Check for spelling errors
  • SUCCESS — Blob size policy
  • SUCCESS — cargo-deny
  • SUCCESS — build-test
  • SUCCESS — Build Codewith Linux CLI
  • SUCCESS — Detect changed areas
  • SUCCESS — python-sdk
  • FAILURE — Bazel test on ubuntu-24.04 for x86_64-unknown-linux-musl
  • SUCCESS — Bazel test on windows-latest for x86_64-pc-windows-gnullvm shard 1/4
  • SUCCESS — Format / etc
  • SUCCESS — sdks
  • SUCCESS — Bazel test on windows-latest for x86_64-pc-windows-gnullvm shard 2/4
  • SUCCESS — Bazel test on windows-latest for x86_64-pc-windows-gnullvm shard 3/4
  • SUCCESS — Bazel test on windows-latest for x86_64-pc-windows-gnullvm shard 4/4
  • SKIPPED — Bazel test on windows-latest for x86_64-pc-windows-gnullvm (native main)
  • SUCCESS — cargo shear
  • FAILURE — Bazel clippy on ubuntu-24.04 for x86_64-unknown-linux-gnu
  • SKIPPED — Argument comment lint package
  • SUCCESS — Bazel clippy on windows-latest for x86_64-pc-windows-gnullvm
  • FAILURE — Argument comment lint - Linux
  • SUCCESS — Verify release build on ubuntu-24.04 for x86_64-unknown-linux-gnu
  • SUCCESS — Verify release build on windows-latest for x86_64-pc-windows-gnullvm
  • SUCCESS — Argument comment lint - Windows
  • FAILURE — CI results (required)
  • SUCCESS — Bazel test on windows-latest for x86_64-pc-windows-gnullvm
  • SKIPPED — [code]smith

Blocking P0/P1 findings

  1. P1 correctness / unsafe-writes: workflow branches execute in the app-server process directory, not the source thread's persisted workspace. workflow_branch_execution_identity uses std::env::current_dir() at codex-rs/state/src/runtime/workflow_orchestrator.rs:1515 and persists it as both cwd and workspaceRoots; resolve_background_agent_config consumes that persisted cwd, and the verifier later uses the worker status snapshot's cwd. The app server already supports a thread cwd that differs from the server cwd (for example, thread_schedule_default_prompt and monitor_thread_cwd resolve persisted thread metadata). A workflow started for such a thread can therefore run a workspace-write branch and its verifier against an unrelated directory. Remedy: resolve and persist the source thread's canonical cwd at admission, fail closed if it is unavailable, and add an integration regression where app-server cwd and thread cwd are distinct and only the thread workspace can be touched.
  2. P1 integrity / unsafe duplicate execution: every app-server supervisor uses the same owner string, workflow-manager:{source_thread_id}, at codex-rs/app-server/src/request_processors/thread_workflow_runtime.rs:106. claim_workflow_run accepts owner_id = ? even while the lease is live, increments generation, and requeues every running verifier on each claim. Two app-server processes sharing the durable state DB can therefore treat each other as the same owner, requeue a verifier while the first process is still executing it, and run a workspace-write command verifier twice. The app server already creates a per-process local_active_owner_id, but this supervisor does not carry it. Remedy: fence workflow ownership with a runtime-unique owner identity, propagate the same identity through start and supervisor paths, renew without requeueing a live in-process verifier, and test two distinct owners so a live lease prevents duplicate verifier execution while expired-lease takeover still recovers it.
  3. P1 required-gate failure: five authoritative forge checks are red. The new app-server integration fixture omits the exact per-step model route required by validate_steps, so both Linux Bazel test checks fail before exercising the feature (workflow YAML does not satisfy the workflow spec invariants). Linux Clippy rejects the new drain loop, Linux argument-comment lint reports 32 exact-comment violations, and CI results (required) consequently fails. These are current required build/test/lint gates and must be fixed and rerun on the replacement head.

Non-blocking follow-ups

  • P2 reviewability: this non-mechanical change is 2,192 changed lines, above the repository review-size guidance. If practical, stage state admission/fencing, app-server executor/verifier runtime, and integration coverage as coherent commits or PRs.
  • P2 hardening: artifact_contains reads the entire artifact into memory, and output_limit_bytes for command verifiers is checked only after both streams are fully drained. Bound reads/drains during execution so a valid workflow cannot make the app server consume unbounded artifact/output bytes before failing closed.

Disposition
NO_GO on the exact current head. The workspace identity and runtime-owner fencing remedies are real cross-layer work, so I did not apply a partial lint-only patch. Leave the PR open, fix the named P1 defects and the five red forge checks, then return the replacement head to this same reviewer for focused verification of those fixes and direct regressions only.

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.

1 participant