Skip to content

fix: preserve literal workspace paths across shell commands - #1739

Open
steipete wants to merge 3 commits into
mainfrom
codex/qa-20260902-literal-workspaces
Open

fix: preserve literal workspace paths across shell commands#1739
steipete wants to merge 3 commits into
mainfrom
codex/qa-20260902-literal-workspaces

Conversation

@steipete

@steipete steipete commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

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, CDPATH lookup, or OLDPWD expansion. 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

  • Captured the pre-fix failures: 27 of 35 portable workspace cases and two of three checkpoint cases failed. All 38 passed after the production repair.
  • All 20 existing invalidation and sync controls passed. The sibling run passed 129 cases and exposed one obsolete generated-command assertion; that assertion was removed and its real Bash execution row strengthened and verified separately.
  • Formatting, go vet, and a fresh native build passed. The production diff is net −16 lines; regression tests are counted separately.
  • Two independent final source reviews completed. Their proposed regressions were checked against actual callers and native old/current behavior; neither established an actionable defect.
  • On Ubuntu 26.04, 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.
  • The repaired-source remote run exited 0 within its original bound, and its test machine was released with cleanup confirmed. Full source inventories and the exact Go 1.26.5 Linux toolchain were verified. Heartbeat, event-upload and remote Git-seed warnings remain recorded; file synchronization succeeded, but remote Git metadata is not claimed as verified.
  • Exact-head CI 33618909223 passed on 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 ./worker path and checks the actual remote workspace for leftover artifact archives at release. That replaces an ambiguous rm substring 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.

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.
@clawsweeper

clawsweeper Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

@steipete
steipete marked this pull request as ready for review September 2, 2026 09:43
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