Skip to content

refactor(commands): read command inputs and output from caller hosts - #3829

Merged
thymikee merged 1 commit into
chore/process-globals-runtimefrom
chore/process-globals-commands
Oct 19, 2026
Merged

thymikee merged 1 commit into
chore/process-globals-runtimefrom
chore/process-globals-commands

Conversation

@thymikee

@thymikee thymikee commented Oct 19, 2026 •

Copy link
Copy Markdown
Member

Summary

Part of #3753. Part of #3747.

Pass the caller environment and working directory through command projection, replay configuration, diff/color output, CLI call sites, IO, and MCP. Required callers and tests touch 20 files (429 gross lines). Released IO defaults remain until #3759; createLocalArtifactAdapter() keeps its signature.

Fourth layer, based on #3828. #3823 remains the top gate layer.

Validation

Head 96260a8b76fcc1ac26f370f38b4ec07aeee8debc.

pnpm check:affected --run --base chore/process-globals-runtime --head 96260a8b76fcc1ac26f370f38b4ec07aeee8debc passed; receipt run-bAjCg9, exactCommit=true.

Coverage failures were fixture races: 7e2cae20ed replaces a directory-order assumption with explicit files; 288e6cceae waits for bridge evidence contents rather than file creation. Both fixes are inherited by all layers. Upper patches are unchanged by range-diff.

Prior full-check and Android daemon/provider evidence stays attributed to e223d37244 in #3823’s review-response comment. This round changes only test fixtures and ancestry. Full coverage and GitHub-owned lanes remain CI obligations.

View guided diff Turn on auto-fix

@github-actions

github-actions Bot commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

Size Report

Metric Base Current Diff
Installed (including dependencies) 5.44 MB 5.44 MB -2.3 kB
Package (unpacked) 5.44 MB 5.44 MB -2.3 kB
Package (download) 1.64 MB 1.64 MB -597 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 33.0 ms 30.3 ms -2.7 ms
CLI --help 61.3 ms 61.7 ms +0.4 ms

@thymikee
thymikee added this pull request to stack #3830 October 19, 2026 16:29
@thymikee
thymikee force-pushed the chore/process-globals-commands branch from 467108e to fb36192 Compare October 19, 2026 16:55
@thymikee

Copy link
Copy Markdown
Member Author

Coverage follow-up: 7e2cae20ed fixes the daemon progress fixture by passing explicit files; the failing assertion assumed filesystem directory order. Restacked this unchanged commands patch to fb36192358; range-diff is identical. Foreground check:affected passed at fb36192358 (run-j7njcE, exactCommit=true). CI is running on the pushed head.

… host (#3753)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@thymikee
thymikee force-pushed the chore/process-globals-commands branch from fb36192 to 96260a8 Compare October 19, 2026 17:25
@thymikee

Copy link
Copy Markdown
Member Author

Second coverage fix: 288e6cceae polls the detached bridge’s expected cwd contents; the shell creates the evidence file before writing it, and CI read an empty file (run 39289751890). Head 96260a8b76 inherits both this fix and 7e2cae20ed; upper-layer range-diff is unchanged. Foreground check:affected passed (run-bAjCg9, exactCommit=true). Cold self-review found no issue in the test-only diff. CI is running on this head.

@thymikee

Copy link
Copy Markdown
Member Author

The code in 96260a8 is ready to merge. CI is still pending: 17 checks are cancelled or queued, with no failure logs, so there is nothing to attribute yet. The cancelled runs are Smoke, Lint/Typecheck/Package, Repo Guards, Coverage and Integration, and this diff touches the same routes they exercise (src/commands, src/mcp, src/cli output). No conflicts. Before merge, the Coverage, Repo Guards and Smoke lanes need to finish green on 96260a8. Not blocking, and you can take or leave these: captureHostEnvironment(process) in src/io.ts still names the global process in a non-entry module (I could not read the #3823 rule, so please confirm it passes, or take the host as a parameter as #3759 intends); the diff, color and system tests pass process.env and process.cwd() as the host and the MCP and batch paths have no test, so one host-distinct case each would catch a regression to process.cwd(); five CLI sites (progress.ts:133 and :226, reporters/default.ts:146 and :172, proxy.ts:109) still call supportsColor(process.env) instead of using the CLI host, so one output-context owner could decide color once; and formatSnapshotDiffText takes env: EnvMap while formatScreenshotDiffText takes host: HostEnvironment, and PrepareBatchStep in src/commands/batch/projection.ts copies the PrepareDaemonCommandRequest shape and its closure shadows the outer input. I ran no tests, and I did not inspect the MCP executor tests or the reordered host capture in command-tools.ts.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Oct 19, 2026
@thymikee
thymikee merged commit 7faa04e into main Oct 19, 2026
17 of 25 checks passed
@thymikee
thymikee deleted the chore/process-globals-commands branch October 19, 2026 17:43
@thymikee

Copy link
Copy Markdown
Member Author

Re-ran Apple 39294051862 once: smoke:form-input failed on fill with typed MAIN_THREAD_TIMEOUT / runner_main_thread_timeout in XCTest; smoke:webview-remote-content then timed out waiting for its link. This layer changes no Swift, so these are suspected runner/fixture flakes pending the one rerun.

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.

1 participant