fix(deploy): stop Start-Process from shredding the Linux Kit build command - #487
Merged
Merged
Conversation
…mmand The bash launch path passed the whole build command as one -c string with embedded quotes. Start-Process joins its ArgumentList into a single Arguments string and re-tokenizes it, so bash actually received only the repo.sh path as the command: repo.sh printed its usage with no arguments and exited 0, the build argument and the log redirect were silently dropped, and deploy.ps1 took the fake exit 0 as a successful build until the artifact recheck failed with a far less diagnosable message. This was latent since the Linux migration (#467) — every earlier rebuild found the deployment checkout unchanged and skipped the build phase — and first fired on the post-#484 fixpoint rebuild, which reset the checkout and cleaned _build. Write the launch command into a wrapper script instead, so the command line carries exactly one plain path argument that no platform's argument re-quoting can damage. Also fail closed when the build process exits 0 without ever creating its log file: that combination means the launch line was shredded and nothing ran. Verified on the canonical Linux host (isolated probe): repo.sh received exactly 'build', the redirect created the log, exit 0. The full canonical rebuild through this path lands with the ledger fixpoint after merge. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015QTVFY89rS2xRwRB2TpFP6
Contributor
📝 WalkthroughWalkthroughLinux Kit builds now run through a generated Bash wrapper that preserves the ChangesKit launch validation
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant InvokeKitRepoBuild
participant Bash
participant repo.sh
participant BuildLog
InvokeKitRepoBuild->>BuildLog: remove stale log
InvokeKitRepoBuild->>Bash: launch generated wrapper
Bash->>repo.sh: pass build argument
repo.sh->>BuildLog: redirect build output
InvokeKitRepoBuild->>BuildLog: verify new log exists
InvokeKitRepoBuild-->>InvokeKitRepoBuild: return exit code or failure
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
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. Comment |
monkey1sai
enabled auto-merge (squash)
August 11, 2026 06:44
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/lib/host-native-launcher.ps1`:
- Around line 401-408: Remove any existing file at $LogPath before invoking
$StartProcessFn, while safely handling the file’s absence. Add a test covering a
pre-existing log where the simulated launch exits 0 without creating a new log,
and verify the launcher reports failure rather than accepting the stale file.
In `@scripts/tests/test-host-native-launcher.ps1`:
- Around line 205-216: Update the generated repo.sh setup in the test around
fakeRepoSh and WriteAllText to grant the file executable permission on Unix
hosts before Invoke-KitRepoBuild runs. Preserve the existing script contents and
test assertions, and use the platform-appropriate permission API.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 78790a33-b6a9-449e-a037-3f866f250578
📒 Files selected for processing (2)
scripts/lib/host-native-launcher.ps1scripts/tests/test-host-native-launcher.ps1
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: af54c4a0f0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a latent regression in the canonical Linux Kit build launch inside Invoke-KitRepoBuild. Previously the build was started with Start-Process -FilePath 'bash' -ArgumentList @('-c', $bashCommand), but Start-Process re-joins and re-tokenizes ArgumentList, shredding the quoted -c string so repo.sh ran with no arguments (printed usage, exited 0, and the log redirect never happened). The build phase then silently "succeeded" until the later artifacts check failed with a hard-to-diagnose error. The fix writes the full launch command into a wrapper script and passes only that single plain path to bash, plus adds a fail-closed guard that treats exit 0 with no log file as a failure.
Changes:
- Launch the Linux
repo.shbuild through a generatedkit-repo-build-launch.shwrapper (one plain path argument) instead of a re-tokenizablebash -ccommand string. - Fail closed in
Invoke-KitRepoBuildwhen the process exits 0 but never created its log file. - Add regression tests: Test 13 now requires the success fake to create its log; Test 15b pins exit-0-without-log to fail closed; Test 15c drives the real bash launch path.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
scripts/lib/host-native-launcher.ps1 |
Replaces the shredded bash -c launch with a wrapper-script indirection and adds the exit-0-without-log fail-closed guard. |
scripts/tests/test-host-native-launcher.ps1 |
Updates Test 13 and adds Tests 15b/15c to cover the fail-closed guard and the real bash launch path; Test 15c creates a fake repo.sh without an execute bit, which fails the exec on POSIX hosts. |
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
…is launch Review round: feed the wrapper script to bash on stdin so the command line carries no argument at all — a deploy_root with spaces has nothing left to shred. Remove any stale kit-repo-build.log before launching so the exit-0-must-have-a-log guard proves this build created it, not an earlier one. Mark the test fixture repo.sh executable on POSIX hosts where exec would otherwise fail with EACCES, and run the dynamic bash test inside a directory with spaces. Verified again on the canonical Linux host: exit 0, args seen=[build], log created, from a 'deploy root with spaces' directory. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015QTVFY89rS2xRwRB2TpFP6
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7925fe7792
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The registry accepts deploy_root values containing $ and backtick; embedding those raw inside the wrapper's double-quoted sh strings lets the shell perform parameter/command substitution on the path, so exec or the log redirect targets a different location. Escape the four double-quote-special characters when composing the wrapper, and run Test 15c from a directory carrying spaces, $, and a backtick (fails with exit 127 without the escaping). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018Gn3PJQ96Krb3adAErpXGu
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/lib/host-native-launcher.ps1`:
- Around line 403-405: The stale-log cleanup before invoking $StartProcessFn
must fail closed: update Remove-Item for $LogPath to stop on errors, then
explicitly verify the path is absent before proceeding. Ensure any cleanup
failure prevents launch and add a regression test covering an undeletable or
still-present $LogPath.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 8c7667ae-80fc-42ee-b698-a022cc8e6b68
📒 Files selected for processing (2)
scripts/lib/host-native-launcher.ps1scripts/tests/test-host-native-launcher.ps1
🚧 Files skipped from review as they are similar to previous changes (1)
- scripts/tests/test-host-native-launcher.ps1
monkey1sai-blip
approved these changes
Aug 11, 2026
monkey1sai-blip
left a comment
monkey1sai-blip
left a comment
Collaborator
There was a problem hiding this comment.
Scripted approval carrying the operator's authority for the documented one-time governance exception on PR #487. This is not independent human review or human sign-off. Exact head fa0423c; required branch-protection contexts completed as success/skipped. The non-required governance diagnostics and unresolved bootstrap thread remain disclosed and are accepted only for this PR because issue #494 proves the current repair-lane deadlock. AI-assisted changes received Luna, Terra, and Sol/max adversarial verification.
monkey1sai
added a commit
that referenced
this pull request
Aug 12, 2026
…with its rebuild-backed fixpoint (#499) * fix(governance): close the linux-test-deploy-verifier-hardening debt with its rebuild-backed fixpoint Rerun the entry's ordered 14-command verification contract after #487 merged: local suites 1-11 all exit 0, canonical Linux rebuild exit 0 with the repaired stdin-fed build launch proven on the canonical host (deploy tag deploy-20260811-639220482065640754-003), the CAD hardener idempotently exit 0, and the remote Deployment-profile verify all green. Two group-writable directory drifts the #484 trust-root ancestry validation correctly refused are recorded in the summary with their in-run chmod remediation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018Gn3PJQ96Krb3adAErpXGu * docs(evidence): attest the strict contract-order fixpoint sweep Rerun the full 14-command contract in the opening contract's exact order (1-11 local at the deployed source commit c88dca6, then harden 12, rebuild 13 with deploy tag deploy-20260811-639220494716638402-004, verify 14) after review flagged the first sweep's 13-before-12 chronology; every command exit 0 in a single pass. The first sweep and the in-run permission findings remain recorded as context. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018Gn3PJQ96Krb3adAErpXGu * docs(evidence): declare an allowed document nature and add run timestamps working note replaces the non-vocabulary 'evidence' nature per docs/AGENTS.md, and the attested-run section now carries UTC time anchors proving the 12-before-13 execution order. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018Gn3PJQ96Krb3adAErpXGu * docs(evidence): rerun command 14 with the pinned -InventoryPath invocation The immutable command map pins canonical-linux-deployment-verify as verify-all.ps1 -Profile Deployment -InventoryPath <owner-private-inventory>; the sweep had substituted the environment-variable inventory form. Rerun the pinned invocation against the same unchanged -004 deployment (exit 0, all six checks Passed, 2026-08-12T02:00:16Z) and make it the attested record. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018Gn3PJQ96Krb3adAErpXGu * fix(governance): attest the fixpoint with a pinned-form 12-14 remediation rerun Review P1 on this PR proved command 13's recorded invocation carried an extra -IdentityFile beyond the immutable command map's pinned form. The owner moved the deploy key into default ssh resolution (batch-mode preflight DEFAULT_IDENTITY_OK), then commands 12-14 were rerun in contract order, all pinned form, single pass: - 12 harden-cad on the remote deploy_root: exact schema line, exit 0 - 13 rebuild from fresh origin/main (970dc34, isolated worktree), NO -IdentityFile / -TargetId: deploy exit 0, tag deploy-20260812-639221007059362180-001 pushed - 14 verify-all -Profile Deployment -InventoryPath on the NEW deployment: six checks Passed, none Failed, exit 0 summary.md keeps the 2026-08-11 invocation as a historical record and marks the 2026-08-12 rerun as the attested one; ledger fixpoint reverified_at rebound to 2026-08-12T03:07:00Z. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
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
Start-Processre-tokenizes its joined ArgumentList, so the quoted-cbuild command reached bash shredded —repo.shran with no arguments, printed usage, exited 0, and the build log redirect never happeneddeploy_rootwith spaces has nothing left to shred_buildChange Classification
Deploy Path Verification
scripts/lib/host-native-launcher.ps1(Invoke-KitRepoBuildbash launch)scripts/dev/rebuild-test-deploy.ps1 -Build -InventoryPath <owner-private-inventory>+ remoteverify-all -Profile Deploymentunder the open ledger entryAI Coding Governance
fa0423cis awaiting reviewoverall_status=unhealthy, exact checkout trust unknown, canonical checkout missing;gitnexus detect-changes --scope compare --base-ref mainfailed on duplicate repo registrations; exact worktree-rfailed as not indexed. Sol/max reviewer accepted this residual for commit/push only, not merge.Windows On-Demand Verification
fa0423c4052e6c279e267427130eb4180817d52a; PS7 and Windows PowerShell 5.1 launcher suites PASS; root contracts 483 passed / 9 skipped using an isolated pytest temp root because the host shared temp ACL denied access; rebuild/test-deploy contracts, agent governance, PowerShell static, secret scan, and security exceptions PASS;git diff --checkclean; exact-head CI run31473397223pending.Self-Referential Bootstrap
Verification
scripts/tests/test-host-native-launcher.ps1: ALL PASSED under PS7 and Windows PowerShell 5.1 for exact headfa0423c— includes real Git Bash launch, special-character paths, exit-0-without-log, stale-log removal, locked stale-log cleanup failure, non-file log-path rejection, and metadata-error fail-closed guardsrepo.sh, run from adeploy root with spacesdirectory): exit 0,args seen=[build], log created with the script's stdoutpython -m pytest tests -q -p no:cacheprovider: 483 passed, 9 skipped;test-rebuild-test-deploy,test-agent-governance-check,invoke-powershell-static, secret scan, security-exception policy: PASS;git diff --checkclean[fix] running bim-streaming-server Kit buildthen[fail] Kit build completed but runtime artifacts are still missing1.2s later; reproduction showed repo tool usage on the console (redirect lost) andExitCode=0; with the wrapper the same host passes🤖 Generated with Claude Code
https://claude.ai/code/session_015QTVFY89rS2xRwRB2TpFP6
Summary by CodeRabbit