Skip to content

fix(mount): retain late-bound remote paths - #33

Merged
khaliqgant merged 2 commits into
mainfrom
fix/mount-layout-contract-0823
Aug 23, 2026
Merged

fix(mount): retain late-bound remote paths#33
khaliqgant merged 2 commits into
mainfrom
fix/mount-layout-contract-0823

Conversation

@khaliqgant

@khaliqgant khaliqgant commented Aug 23, 2026

Copy link
Copy Markdown
Member

Summary

Follow-up to #31 after the Cloud twin exposed a late-bound template omission.

The dynamic remote-path sentinel was non-absolute. buildMountArgs routes paths through scopedRemoteRoots, which discarded the sentinel before dynamicMountArgs could replace it. Generated late-bound start and flush commands therefore used exact local directories but omitted --remote-path.

This makes the sentinel absolute so it survives normalization, then replaces it with the late-bound shell variable as intended. Regression assertions cover both start and flush templates.

Validation

  • targeted mount and lifecycle tests: 24 passed
  • final full suite: 766 total, 757 passed, 9 skipped, 0 failed
  • typecheck: passed
  • package build and smoke test: passed
  • npm publish dry-run for 0.1.7: passed
  • git diff --check: passed

An earlier concurrent full-suite run hit the known unrelated Freestyle 25ms timing test; its isolated rerun passed 25/25 and the final full suite passed exactly.

Release

0.1.6 was published from merged #31 before this omission was found. npm packages are immutable, so this PR prepares 0.1.7 in package.json and both package-lock version fields under the explicit release contract from #29.

Registry check: npm latest is 0.1.6; npm 0.1.7 and git tag v0.1.7 are absent. Do not republish or pin 0.1.6 as the complete mount fix. Chief owns merge and publish.

No merge, live publish, deploy, or image rebuild was performed by this lane.

@coderabbitai

coderabbitai Bot commented Aug 23, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 8 minutes.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: a11a307f-3ed8-4023-8a0e-b64c53ac3848

📥 Commits

Reviewing files that changed from the base of the PR and between f216b47 and 963e8c9.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (3)
  • package.json
  • src/mount-script.test.ts
  • src/mount-script.ts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@khaliqgant

Copy link
Copy Markdown
Member Author

@codex review

Please review the exact head f2cfa72. Focus on whether the absolute dynamic sentinel survives scopedRemoteRoots and is replaced in both generated start and flush commands without leaking the sentinel or changing static path normalization.

@khaliqgant

Copy link
Copy Markdown
Member Author

Current-head gate for f2cfa72

CI is green on the exact head: https://github.com/AgentWorkforce/sandbox/actions/runs/32667588316. GraphQL reports 0 review threads.

Automated rollup caveat:

  • CodeRabbit: Review rate limited
  • Devin: Full review skipped because the trial expired and no credits remain
  • cubic: NEUTRAL because the monthly line limit was reached

A fresh Codex review was explicitly requested here: #33 (comment)

The two-file delta was also manually inspected: the absolute sentinel is retained by scopedRemoteRoots and replaced with the dynamic shell variable; regressions assert both start and flush templates contain the resulting remote-path argument. Publish remains blocked pending the review and chief merge gate.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Nice work!

Reviewed commit: f2cfa725dc

ℹ️ 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".

@khaliqgant

Copy link
Copy Markdown
Member Author

Release correction on head 963e8c9

Registry verification shows npm latest is already 0.1.6 and remote tag v0.1.6 exists. That immutable release came from merged #31 before the late-bound remote-path omission was found.

This PR now prepares 0.1.7: package.json, package-lock top-level, and package-lock root package all read 0.1.7. npm 0.1.7 and git tag v0.1.7 are absent. npm publish dry-run builds @agent-relay/sandbox@0.1.7 successfully.

The previous f2cfa72 CI was green, but the version-bump head requires its own CI. Fresh Codex review was requested and must target the new head. No live publish or merge was performed here.

@khaliqgant

Copy link
Copy Markdown
Member Author

@codex review

Please review the current release head 963e8c9. The code delta was reviewed clean at f2cfa72; the only additional commit synchronizes package.json and both package-lock version fields to 0.1.7 after 0.1.6 was published.

@khaliqgant
khaliqgant merged commit a58012a into main Aug 23, 2026
4 checks passed
@khaliqgant
khaliqgant deleted the fix/mount-layout-contract-0823 branch August 23, 2026 21:32
@khaliqgant

Copy link
Copy Markdown
Member Author

Current release-head gate

CI is green on exact head 963e8c9: https://github.com/AgentWorkforce/sandbox/actions/runs/32667729597

Codex reviewed the code head f2cfa72 and found no major issues: #33 (comment). A second review was requested on the exact release head after the mechanical 0.1.7 version synchronization: #33 (comment).

CodeRabbit remains rate-limited, Devin remains credit-skipped, and cubic remains monthly-limit neutral. No merge or publish by this lane.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Swish!

Reviewed commit: a58012a6ce

ℹ️ 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".

@khaliqgant

Copy link
Copy Markdown
Member Author

Release complete, validation-parity follow-up separated

Registry verification now shows npm latest 0.1.7 and remote tag v0.1.7. The critical absolute-sentinel correction from this PR is therefore published.

While comparing the Cloud vendored port, I found Cloud also rejects non-absolute and empty late-bound roots. Static sandbox builders already reject those forms, but the public template preflight did not. Narrow source-parity hardening is now isolated in #34: #34

#34 is not a blocker for current production recovery: normal generated args already pass through buildRelayfileMountPathArgsShell, and 0.1.7 contains the critical retained --remote-path fix. No merge or publish performed by this lane.

@khaliqgant

Copy link
Copy Markdown
Member Author

Published-artifact verification

Fetched the npm 0.1.7 tarball directly from the registry and inspected package/dist/mount-script.js. It contains the absolute DYNAMIC_REMOTE_PATH value /relayfile_dynamic_remote_path and the replacement to "$relayfile_mount_remote_path"; the built argument list retains --remote-path generation. This verifies the critical #33 correction is present in the published artifact, not only in git source.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant