fix: preserve literal workspace paths across shell commands - #1739
Open
steipete wants to merge 3 commits into
Open
fix: preserve literal workspace paths across shell commands#1739steipete wants to merge 3 commits into
steipete wants to merge 3 commits into
Conversation
Resolve the selected workspace before directory changes and environment loading. Render one bound Actions hydration plan before invalidation, preserving caller arguments, stdin, exit status, and environment values.
Contributor
|
🦞👀 Pull request received. I will update this pull request when review starts. |
steipete
marked this pull request as ready for review
September 2, 2026 09:43
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.
What Problem This Solves
Fixes an issue where users running commands or synchronizing a relative workspace could operate on the wrong directory after a shell directory change,
CDPATHlookup, orOLDPWDexpansion. Local Actions hydration could also invalidate sync state before discovering that its workspace-dependent plan could not run.Why This Change Was Made
Resolve literal workspace paths at the shell owner before changing directories or loading the user environment. Capture workflow/composite sources first, bind the actual remote workspace, and render one immutable hydration plan before any invalidation or sync mutation. Retain caller arguments, stdin, exit status, and user environment values. Remove the obsolete shell wrappers and duplicate sync paths absorbed by those owners.
User Impact
Commands, uploads, downloads, sync metadata, and local Actions hydration use the selected workspace consistently, including literal relative and dash-prefixed paths. User environment values and normal command arguments, stdin, and exit status retain their existing contracts.
Evidence
go vet, and a fresh native build passed. The production diff is net −16 lines; regression tests are counted separately.go test -json -count=1 -timeout=180s ./internal/cli -run '^TestLocalActionsHydrationLiteralPaths$'reproduced three relative/dash-path failures with a passing absolute-path control. The same four cases all passed on the repair. This fixture replaces SSH transport and executes the generated hydration commands with native Linux tools; it also changes workflow/composite inputs at the first remote boundary to verify the captured plan remains stable.a4ded6642cbe9ece16009060dac7280db052ee1a. GitHub's required independent approval remains pending.Landing is also held for a subsequent real AWS Ubuntu 26.04 flow using this exact commit. Provisioning and guest qualification passed, but native Actions hydration exited 7 at fingerprint invalidation before the intended systemd tests ran. The underlying transport or workspace failure is not yet established. The focused Linux fixture passes above do not prove that this real flow is repaired; native cleanup is complete, with the machine released and its local claim and connection removed. Diagnosis continues at the workspace-owner execution boundary, which the earlier fixture omitted.
The first CI run caught an unused private SSH forwarding helper after its sole Actions caller moved into the canonical execution path. A follow-up removes that four-line declaration. Its managed review passed without findings; active call paths and timeout arguments are unchanged.
The following CI run exposed stale generated-path and cleanup-text expectations in two existing tests. The correction retains the literal
./workerpath and checks the actual remote workspace for leftover artifact archives at release. That replaces an ambiguousrmsubstring check and fixes a hook that previously inspected the wrong directory. Required-artifact, download, archive contents, permissions and release-order assertions remain. All three focused cases passed, and independent review found no actionable defects. This follow-up adds nine test lines and removes seven, with no production change. The updated PR's full CI passed, including race tests, coverage, module checks, Windows and macOS; GitHub still requires an independent approval.Scope and risk
The repair keeps one canonical command/hydration path. It changes no configuration or persisted schema and introduces no compatibility fallback. Literal paths, including relative paths and leading dashes, are preserved without lexical normalization. An adopted workspace that differs from the bound plan is rejected before destructive sync work.
Follow-up
An earlier, separate guest-closeout observation timed out after collecting the pre-fix failures. That failed observation remains recorded, and the original machine was subsequently confirmed destroyed. The successful repaired-source run used a fresh machine and separate normal provider cleanup; it does not relabel the earlier timeout as success.
The final managed source review completed all four partitions and raised one proposed input-default regression. Direct old/current source inspection and a three-case native witness on each version disproved its premise: literal defaults and direct hashes work in both; a nested hash expression in a default is rejected by both before the alleged second interpolation. No new recursive-expression capability or production change was added. The diagnostic files were removed after their recorded process closure, and both source inventories were restored.
The independent review’s proposed empty-workspace invalidation regression is unreachable through the private function’s three production callers: each receives a nonempty repository path or an explicitly nonempty adopted workspace. No defensive branch was added for an unproduced state. Both original review reports and their adjudications are retained.