Skip to content

feat(tooling): simplify agent workflows and validation - #3705

Merged
thymikee merged 8 commits into
mainfrom
t3/simplify-agent-workflows
Oct 16, 2026
Merged

thymikee merged 8 commits into
mainfrom
t3/simplify-agent-workflows

Conversation

@thymikee

@thymikee thymikee commented Oct 16, 2026 •

Copy link
Copy Markdown
Member

Summary

  • Give check:affected --run and pnpm check one host lock, cancellation, compact progress, disk logs, and checkout-aware receipts. --run --json returns execution results; Vitest uses dots and structured failure reports.
  • Reduce default AGENTS context 57% (8,968 → 3,847 bytes). Route specialized rules; enforce 4,000-byte AGENTS, 5,000-byte procedure, 11,000-byte glossary, and 40,000-byte procedure-total limits.
  • Require isolated mutation experiments and explicit blocked-validation ownership.
  • Bound output capture and excerpts, flush diagnostics, fingerprint modes and large file lists, and consolidate duplicate test execution.

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.

pnpm check:affected --run --head <commit>
pnpm --silent check:affected --run --json

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.
  • Host guard: three tests pass; removing it yields one failure/two passes. Long-path fixture: two pass. Guidance contracts: five pass. Experiments used disposable checkouts.
  • Isolated mutants also detect selection, fingerprint, report, output, cadence, and stale-sample regressions. Opus/Haiku/Apex guidance exercises completed.
  • Unchanged cee2da11a4 comparison had five Apple-runner timeouts; these remain distinct from the repaired Android assertion.

@github-actions

github-actions Bot commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-10-16 12:44 UTC

@github-actions

github-actions Bot commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Size Report

Metric Base Current Diff
Installed (including dependencies) 5.29 MB 5.29 MB -211 B
Package (unpacked) 5.29 MB 5.29 MB -211 B
Package (download) 1.59 MB 1.59 MB +11 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 27.0 ms 26.8 ms -0.2 ms
CLI --help 75.8 ms 75.7 ms -0.1 ms

Comment thread scripts/check-affected/command-execution.test.ts
Comment thread scripts/check-affected/command-execution.test.ts

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread scripts/check-affected/host-lock.ts
Comment thread scripts/check-affected/checkout-state.ts
Comment thread scripts/check-affected/checkout-state.ts
Comment thread scripts/check-affected/run-cli.test.ts
Comment thread docs/agents/testing.md Outdated
Comment thread docs/agents/device-foldable.md Outdated
Comment thread scripts/check-affected/command-execution.test.ts Outdated
Comment thread docs/agents/device-verification.md Outdated
Comment thread docs/agents/gates.md Outdated
Comment thread docs/agents/device-verification.md Outdated
@thymikee

Copy link
Copy Markdown
Member Author

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: streamingExecutor and the !options.execute branch in scripts/check-affected/run.ts:191 are unreachable now that --run always passes an executor, so execute could be required and the old runner deleted. In scripts/check-affected/host-lock.ts:24, only acquireValidationLock takes lockDirPath, while recordValidationCommand and finishValidationCommand use the module-load HOST_LOCK_PATH. A custom lock path would then write its orphan record where its own waiter never looks. A handle that owns the path and the record/finish/release calls would remove that gap. CONTRIBUTING.md:216 does not say that pnpm check now takes a host lock, prints a receipt and writes logs under .tmp/validation, and the fragment at CONTRIBUTING.md:242-243 reads incomplete. scripts/check-affected/execution.ts:104 creates a .tmp/validation/run-* directory per run and never prunes old ones.

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.

@thymikee

Copy link
Copy Markdown
Member Author

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.

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread scripts/check-affected/host-lock.ts Outdated
Comment thread scripts/check-affected/checkout-state.test.ts
Comment thread scripts/check-affected/host-lock.ts Outdated
Comment thread docs/agents/testing.md Outdated
@thymikee

Copy link
Copy Markdown
Member Author

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.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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' },

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>

@thymikee

Copy link
Copy Markdown
Member Author

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 failOpenReasons fields.

Not blocking: the win32 skip rewrite also put { skip: process.platform === 'win32' } into the four vitest test(...) calls in the generated fixture string at https://github.com/callstack/agent-device/blob/799b14c/scripts/check-affected/test-report.test.ts#L28, and the outer node:test already skips on win32, so you can take or leave restoring those four lines to plain test('passes', () => ...).

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 !['darwin','linux'].includes(process.platform) skip is optional.

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.

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

Labels

ready-for-human Valid work that needs human implementation, judgment, or maintainer merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants