Skip to content

fix(computer-use): require explicit lab root - #4072

Open
orangeCatDeveloper wants to merge 4 commits into
apache:mainfrom
orangeCatDeveloper:fix/cu-explicit-lab-root
Open

fix(computer-use): require explicit lab root#4072
orangeCatDeveloper wants to merge 4 commits into
apache:mainfrom
orangeCatDeveloper:fix/cu-explicit-lab-root

Conversation

@orangeCatDeveloper

@orangeCatDeveloper orangeCatDeveloper commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Summary

computer-use restart-soak fails outside one contributor's machine because its launcher and harness resolve the fixture from a private absolute path.

The restart scripts embedded that path directly, and the real AX harness retained it as a fallback even though the real AX launcher already required MAKA_CU_AX_MODEL_LAB_ROOT.

Require the existing environment variable through one shared parser so every Lab-backed entry point fails early with an actionable error and never depends on a contributor-specific checkout. The parser rejects blank, relative, missing, and incomplete fixture roots, then returns the canonical path of a checkout with an executable test-app/launch.sh.

Document the public fixture checkout, absolute-path export, automatic fixture build, and the shared setup for both canonical real-ax runs and the non-qualifying restart soak.

Run a pure Node contract in CI that verifies every Lab-backed entry point uses the shared parser and does not embed the external checkout name.

Evidence

Before (origin/main):
origin/main:scripts/computer-use/process-restart-harness.mjs:28:const labRoot = '/Users/haoqing/Documents/Learning/codex-computer-use-lab';
origin/main:scripts/computer-use/process-restart-launcher.mjs:30:const labRoot = '/Users/haoqing/Documents/Learning/codex-computer-use-lab';
origin/main:scripts/computer-use/real-ax-harness.mjs:37:  '/Users/haoqing/Documents/Learning/codex-computer-use-lab';

After:
Error: MAKA_CU_AX_MODEL_LAB_ROOT is required: point it at a local checkout of the Codex CUA Lab fixture
✔ rejects missing and invalid Computer Use Lab roots
✔ canonicalizes a valid Computer Use Lab root
✔ Lab-backed entry points require the configured root
ℹ pass 3
ℹ fail 0

Verification

  • node --test scripts/ax-tree-audit.test.mjs scripts/computer-use/lab-root.test.mjs
  • node --test --test-concurrency=1 scripts/ci-test-plan.test.mjs scripts/verify-windows-harness.test.mjs
  • node scripts/asf-license-headers.mjs check
  • biome lint .
  • biome format .
  • git diff --check

The full Computer Use harness was not run because it requires a local Codex CUA Lab fixture and built workspace artifacts. Full build and typecheck were not run for this script-only draft.

AI use

Select exactly one:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: Codex implemented the shared environment validation, tests, operator documentation, CI wiring, and PR description.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

@github-actions github-actions Bot added the effort/S Under 100 readable lines label Aug 28, 2026
@orangeCatDeveloper
orangeCatDeveloper marked this pull request as ready for review August 28, 2026 09:29
@github-actions github-actions Bot added effort/M Under 500 readable lines and removed effort/S Under 100 readable lines labels Aug 28, 2026
@Astro-Han

Copy link
Copy Markdown
Contributor

This is a synthesis of the independent blind review by @Sol-404ARE at exact head 66470239cf7d70bb820cd0f8d9636c7ee8fe36b5 (base a956b1ae04aa7421a749931006a6df8fe564fc60, 9 files +129/-17). I verified the diff and the exact-head CI myself; the file:line findings below are from Sol's sealed review.

What I checked myself:

  • Read gh pr diff 4072 (fix: require explicit lab root, 9 files, 3 commits) and confirmed the shared parser at scripts/computer-use/lab-root.mjs:20-27 only checks truthiness, while docs at computer-use-evidence-classes.md:69-79 claim absolute repository root containing test-app/launch.sh.
  • Checked exact-head CI: test SUCCESS, label SUCCESS, OPEN/MERGEABLE/BLOCKED/REVIEW_REQUIRED.

Findings from Sol's review (file:line anchored):

Standards — NO-GO — 3×P2 (worst P2)

  1. P2 — The documented Lab-root contract is not enforceddocs/computer-use-evidence-classes.md:69-79 requires an absolute repository root containing test-app/launch.sh; scripts/computer-use/lab-root.mjs:20-27 accepts any truthy string, and its test at :26-40 only proves passthrough/missing-variable. Relative or wrong paths therefore pass the supposedly fail-early boundary and later produce cwd-dependent paths or a generic ENOENT. The parser should validate/canonicalize an absolute existing root plus the fixture sentinel (preferably via realpath + regular/executable check) and cover relative/wrong-dir/valid fixture cases.

  2. P2 — Commit 46c17e54c4a12d639e7504bf82178203cd25893e lacks the required Generated-by: Codex trailer — the PR body states Codex implemented the shared validation; missing the trailer violates CONTRIBUTING.md:32-36.

  3. P2 — Commit 66470239cf7d70bb820cd0f8d9636c7ee8fe36b5 likewise lacks the trailer — the PR body states tests were implemented by Codex, so the same trailer is required.

Excluded: docs-only commit a65078e1 is not attributed to Codex in the body, so no trailer is required for it. No Fowler smell was found; the four call sites are required contract propagation and the shared parser reduces duplication once the boundary is hardened.

Spec — NO-GO — 1×P2 (worst P2)

  • P2 — The core promise “existing explicit root / early actionable failure / absolute root containing test-app/launch.sh” is not implemented — the parser only checks truthiness. Direct repro shows ' ', 'relative/lab', '/', and a non-existent absolute path all pass unchanged. Launchers then run build/setup first and only later fail with a generic ENOENT at <bad-root>/test-app/stop.sh; a relative root also varies with the launcher/harness cwd. The fix should check trimmed nonempty + isAbsolute, canonicalize the existing directory, and verify the fixture sentinel (regular/executable) with tests for whitespace/relative/nonexistent/missing-launch/valid-root.

Remaining Spec mapping is green: all four existing Lab-backed launcher/harness call the shared parser, the private fallback is removed, launcher→harness env propagation is preserved, real-ax/restart-soak docs share the public fixture setup, and CI runs the pure-Node contract. Symlink canonicalization to a valid fixture is acceptable, and the manual four-entry inventory is complete.

Verification: git diff --check PASS, focused AX/CU 14/14 PASS, CI planner/Windows harness 88/88 PASS, targeted Biome PASS, ASF headers PASS, worktree clean, head unchanged. test + label green does not override the four P2s.

What I did not judge: a true lab-root.mjsstop.sh ENOENT E2E with a relative root across two different cwd values was not executed beyond the parser repro — verification was by code inspection and the tests noted above.

Gate: Standards 3×P2 and Spec 1×P2 remain; despite test/label green, head 66470239 is not merge-ready as “require explicit lab root” until the parser is hardened and the trailers are amended. Seal: notes/pr-4072-provisional.md.


Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/M Under 500 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants