Repository navigation
feat(tooling): simplify agent workflows and validation - #3705
Conversation
|
Size Report
Startup median (7 runs, lower is better):
|
There was a problem hiding this comment.
All reported issues were addressed across 32 files
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
|
The code looks good at e93d80a, and all 24 checks pass, including the vitest lanes that use the changed reporters. There are no conflicts. Before merge, please take the open doc threads that still hold: restore the reboot exception in device-verification.md:36, restore the LANE_SOURCES/LANE_DEPENDENCIES split in gates.md:15, restore the origin/main...HEAD range in pull-requests.md:40, make the mktemp path in test-experiments.md:13 portable, and set maxBuffer on the two spawnSync calls at checkout-state.ts:14 and :22. Not blocking, and fine to take or leave: On the open threads: the P1 on host-process.ts:98-105 (win32 orphan probe) still applies, #3705 (comment), and 11 lower-priority Cubic threads still apply. These do not apply, so please resolve them: command-execution.test.ts:54 (two threads) builds test-only source from temp paths, #3705 (comment) and #3705 (comment); run-cli.test.ts:43 and execution.fixtures.ts:42 override HOME but no Windows lane runs these suites, #3705 (comment) and #3705 (comment); testing.md:39 is resolved by the selector plan on line 33, #3705 (comment); testing.md:28 already says the checks do not sandbox the checkout, #3705 (comment); command-execution.test.ts:97 is POSIX-only with no Windows lane, #3705 (comment). I did not run the check-affected tests or the budget test, so the claim that they failed before the fixes is unverified. I read the orphan-group behavior after a SIGKILL in the code and did not observe it. |
|
Addressed the review in d8d1080 and be7a8ae. The final affected gate passed with exactCommit=true at be7a8ae: 36 local checks, 15,549 Vitest passes, one skipped test, zero unhandled errors, and 79 selector/runner tests. The receipt also records 25 CI/manual skips and two deduplications; these are not passing CI evidence. The test audit combined two identical fail-open selector runs while retaining their independent assertions. It strengthened the existing real-Vitest test to verify receipt collection and the activation test to exercise output backpressure. The cancellation and orphan tests remain because they detect distinct faults. The repeated Android gate failure came from a capture-count assumption that awaited TCP work cannot guarantee. Its test now proves the final observed value and keeps the deadline check. A valid 50 ms capture delay reproduces the old failure; the repair passes, while interval and stale-result mutants still fail. No Android runtime behavior changed. Ten planted faults were detected across the isolated experiments; their worktrees were removed. Also addressed the optional dead executor, lock-handle path ownership, and CONTRIBUTING guidance. Receipts remain available until explicit cleanup so review/base-comparison evidence is preserved; automatic pruning is deferred. Native Windows execution remains unverified; its cancellation and uncertainty policy has simulated coverage. The unchanged-base comparison at cee2da1 failed five Apple-runner timeouts, distinct from the repaired Android assertion. Those results remain attributed separately in the PR body. |
There was a problem hiding this comment.
All reported issues were addressed across 24 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
|
Addressed all four new threads in 799b14c. Per the agreed scope, serialized validation now supports macOS/Linux only. Native Windows fails before lock access or check execution; plan generation, direct focused commands, and production Windows runtime behavior remain unchanged. The unsafe Windows PID/tree-termination paths and their mocks were removed, reducing this round by 54 lines overall. The long-path fixture enables core.longpaths. Testing guidance distinguishes failOpenReasons in plan JSON from plan.failOpenReasons in a receipt. Agent procedures remain below their unchanged 40 KB total cap. Exact-commit affected validation passed at 799b14c: 36 local checks; 15,549 Vitest passes, one skipped, zero unhandled errors; 79 runner/selector tests. The host rejection test passes with the guard and fails when it is removed (three passes → one failure/two passes). The disposable experiment checkout was removed. CI/manual skips remain recorded separately; new CI is pending. |
There was a problem hiding this comment.
1 issue found across 10 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="scripts/check-affected/host-lock.test.ts">
<violation number="1" location="scripts/check-affected/host-lock.test.ts:10">
P2: These tests still run on unsupported platforms other than Windows, where `acquireValidationLock` rejects before exercising lock behavior. Skip both lock tests on every platform except `darwin` and `linux`.</violation>
</file>
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
|
|
||
| test( | ||
| 'host lock excludes a waiter, supports cancellation, then admits it after release', | ||
| { skip: process.platform === 'win32' }, |
There was a problem hiding this comment.
P2: These tests still run on unsupported platforms other than Windows, where acquireValidationLock rejects before exercising lock behavior. Skip both lock tests on every platform except darwin and linux.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At scripts/check-affected/host-lock.test.ts, line 10:
<comment>These tests still run on unsupported platforms other than Windows, where `acquireValidationLock` rejects before exercising lock behavior. Skip both lock tests on every platform except `darwin` and `linux`.</comment>
<file context>
@@ -1,121 +1,74 @@
- });
+test(
+ 'host lock excludes a waiter, supports cancellation, then admits it after release',
+ { skip: process.platform === 'win32' },
+ async () => {
+ const directory = fs.mkdtempSync(path.join(os.tmpdir(), 'validation-lock-'));
</file context>
|
The code is clean at 799b14c. The earlier findings from e93d80a are fixed: host-lock.ts now refuses Windows before it takes the lock, the Windows PID kill path is gone, checkout-state.test.ts sets core.longpaths, and docs/agents/testing.md names the right Not blocking: the win32 skip rewrite also put On the open threads: the Cubic thread on host-lock.ts (#3705 (comment)) and the three others at #3705 (comment), #3705 (comment) and #3705 (comment) are fixed at this head, so you can resolve them. The Cubic thread on host-lock.test.ts:10 and :43 (#3705 (comment)) does not need a change, because no CI job or supported dev host runs these suites outside darwin or linux. A one-line I did not run check:affected:test or the exec.test suite. I read the lock release on signal and on a SIGKILLed runner in the code, and I did not see the orphan-group block live after a runner crash. 12 CI jobs were still queued or in progress at review time, with no failure logs. The change to host-kit exec.ts (runCmdStreaming) feeds the unit, integration, smoke and coverage lanes. The docs and AGENTS edits feed Ownership and context budgets, Prose/links and command-docs, and scripts/check-affected feeds Repo Guards, so a failure in any of these could be related. Before merge, wait for those lanes to go green at 799b14c. |
Summary
check:affected --runandpnpm checkone host lock, cancellation, compact progress, disk logs, and checkout-aware receipts.--run --jsonreturns execution results; Vitest uses dots and structured failure reports.Serialized validation supports macOS/Linux. Native Windows fails before lock access or check execution; plan generation, direct focused commands, and production runtime support remain unchanged. This removes speculative Windows tree-killing code.
36 files; contributor tooling and guidance. One Android test oracle now checks the final sample without an invalid capture quota; runtime behavior is unchanged.
Validation
799b14c5f2260b221698f494374cc52280f4a4af: exact-commit affected gate passed 36 local checks, 15,549 Vitest tests (one skipped), and 79 selector/runner tests. 25 CI/manual skips, two deduplications; CI pending.cee2da11a4comparison had five Apple-runner timeouts; these remain distinct from the repaired Android assertion.