Repository navigation
test(link): keep the constrained-mode prefix out of the cmd.exe budget - #8517
Merged
Merged
Conversation
The Windows containment probe for the pinned sync command failed on every lane=all run since #8505 grew the packed script to 7740 encoded characters. The test decoded the builder's command, prepended a ConstrainedLanguage line and re-encoded it through powershellArgv, which also repeats its preamble; that scaffolding overhead alone pushed it to 8020 and tripped the 8000 cap, so the test threw before PowerShell ever ran. The production command stays under the cap and the builder still enforces it. The prefix is now encoded beside the builder's own argv, and a cross-platform case pins that each probe runs the builder's command unchanged.
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Contributor
|
✅ Deterministic PR hygiene checks passed. |
github-actions
Bot
added
the
chore
Contributor
|
Caution Review failedThe pull request is closed.
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The full
lane=allrun ondev(5197322e25, run 42941869160) fails onwindows 11/11:#8505 grew the packed pinned sync script from 7672 to 7740 encoded characters, which is still under the 8000 cap. To simulate a Constrained Language Mode session, the test decoded that command, prepended a
LanguageMode='ConstrainedLanguage'line, and re-encoded it throughpowershellArgv, which also adds its$ProgressPreferencepreamble a second time. That overhead alone reached 8020, so the test threw before PowerShell started. It is Windows-only, so the Ubuntu PR lanes could not see it.The prefix is test scaffolding that no command Home sends carries. It is now encoded next to the builder's own argv (
constrainedArgv) and no longer spends the production cmd.exe budget the builder already enforced. The test spawnspowershell.exedirectly, so cmd.exe's limit does not apply. A new cross-platform case pins that each probe runs the builder's command unchanged after the prefix. No source changes.Verification
... budget (8020)forwindowsPinnedSyncArgv("lnk_0123456789abcdef"). The new one encodes to 7916 characters, and the builder's own command stays at 7740.bun test tests/clients/link-ssh-argv-discovery.test.ts: 44 pass, 13 skip (Windows-only), 0 fail.bun run typecheck: no type errors.lane=alldispatch ondevafter merge.Checklist