Skip to content

fix(#5179): preflight install destinations before writes - #5306

Open
bshiggins wants to merge 2 commits into
open-gsd:nextfrom
bshiggins:fix/5179-install-refusal-order
Open

bshiggins wants to merge 2 commits into
open-gsd:nextfrom
bshiggins:fix/5179-install-refusal-order

Conversation

@bshiggins

Copy link
Copy Markdown
Contributor

Fix PR

Linked Issue

Fixes #5179

What was broken

When an artifact destination such as ~/.claude/skills sits behind a symlink the install root does not trust, the installer refused it only inside installRuntimeArtifacts (src/install-engine.cts:1313 on next at 651bd2f). By then copyWithPathReplacement had already removed and re-copied gsd-core/ (the rmSync at bin/install.js:7831, reached from 11356 and 11359). A refused run exited 1 and left a half-replaced install: no VERSION, .gsd-runtime or gsd-file-manifest.json, and a new gsd-local-patches/ backup. The rerun with GSD_ALLOW_SYMLINKED_DEST=1 then reported nearly every file as a local modification.

What this fix does

install() now calls preflightInstallSymlinkDestinations before orphan recovery, local patch preservation, migrations and the gsd-core/ replacement, so a refusal happens before the install's first write. In one pass it runs the same hasExistingSymlinkBetween test, with allowOptInFollow: isSymlinkedDestOptIn(), on every destination the install will write:

  • gsd-core/, against the config dir;
  • each artifact kind in the runtime's artifact layout, against that kind's own install root (kind.home when set, else the config dir);
  • on global installs, the runtime-surface corpus directories the layout requires (gsd-core/commands/gsd, gsd-core/agents);
  • on Codex installs that write agent config, config.toml, agents/ and each selected agent's .toml.

Each refusal throws the message the later check for that destination throws today. Those messages now come from builders in src/runtime-artifact-install-plan.cts that the preflight and the existing checks both call, so the text cannot drift. The preflight also takes each decision from a helper the install path itself calls: the install root and destination of a kind, the combined skill family, legacy flat local installs, standalone agents, the required corpus sources, whether Codex agent config is written, and the Codex agent .toml names. It therefore checks only destinations that install writes for that runtime, scope and mode, and refuses only where a later check would.

Every existing check stays where it is. hasExistingSymlinkBetween and isSymlinkedDestOptIn are unchanged, symlinks stay untrusted by default, and copyWithPathReplacement's confinement check and the settings.json path (#5037) are untouched. Two checks are not moved, on purpose: the user-artifact staging root, because recovery and staging already warn and skip on an untrusted staging root instead of aborting, and the compatibility marker, whose writer is non-fatal.

Root cause

As the triage found: the clean-install rmSync (acd62c0) predates the opt-in refusal added in #2393 (12e4d93), which #2875 (3ab0007) kept inside the per-kind loop after the gsd-core/ copy. The refusal therefore ran after the first destructive write.

Testing

How I verified the fix

Four tests in tests/install-runtime-artifacts.test.cjs, beside the other installer tests:

  • refuses an untrusted symlinked destination: without the opt-in, a symlinked skills/ still exits non-zero with the existing message.
  • leaves the previous install byte-identical: after that refusal, every path and hash under the config dir and the symlink target is unchanged, including VERSION, .gsd-runtime, gsd-file-manifest.json and a sentinel file, and no gsd-local-patches/ appears.
  • installs the same layout with GSD_ALLOW_SYMLINKED_DEST=1: the opted-in run exits 0 and writes through the symlink.
  • checks a kind with an alternate home against that home: a Codex global install puts skills under $HOME/.agents/skills; with that directory behind an untrusted symlink and the install seeded as an older one (VERSION and a sentinel file), the rerun is refused with the same message, and the config dir, $HOME/.agents and the symlink target are unchanged. It passes --no-legacy-cleanup only to skip the optional legacy artifact scan, which runs after the install's writes.

With bin/install.js and src/ reverted to next (651bd2f), the refusal and opt-in tests still pass, and the preservation and alternate-home tests fail. With the fix, all four pass, as do all 410 tests in that file.

Gates, run locally on Node 24 against next at 651bd2f: build:lib, lint:ci, check:phase-id-drift, lint:changeset, lint:docs, the context-index check, prompt-injection-scan.sh --diff upstream/next, and the full suite (28 chunks, 44,415 tests, 44,377 passed, 1 failed). The one failure, #4988: installed local CLI syncs and reads its own Claude agents in tests/effort-local-install.test.cjs, fails the same way on next itself on macOS: it compares the unresolved temp dir (/var/...) with the CLI's resolved path (/private/var/...). It is unrelated to this change.

Regression test added?

  • Yes: added a test that would have caught this bug
  • No

Platforms tested

  • macOS
  • Windows (including backslash path handling)
  • Linux
  • N/A (not platform-specific)

Runtimes tested

  • Claude Code
  • Antigravity
  • OpenCode
  • Other: Codex (the alternate-home test)
  • N/A (not runtime-specific)

Checklist

  • Issue linked above with Fixes #5179
  • Linked issue has the confirmed-bug label
  • Fix is scoped to the reported bug: no unrelated changes included
  • Regression test added
  • All existing tests pass (npm test)
  • .changeset/ fragment added (agile-mice-jump.md, type Fixed)
  • No unnecessary dependencies added

Breaking changes

None

🤖 Generated with Claude Code

A symlinked destination that the install root does not trust was refused
only after copyWithPathReplacement had already removed and re-copied
gsd-core/, so a refused upgrade left no VERSION, no .gsd-runtime and no
file manifest, and the opted-in rerun backed up the first run's files as
local patches.

install() now calls preflightInstallSymlinkDestinations before its first
write. The preflight runs the same hasExistingSymlinkBetween checks, with
the same GSD_ALLOW_SYMLINKED_DEST opt-in and the same refusal messages,
for gsd-core/, every artifact-layout kind against its own install root
(an alternate kind.home included), the Runtime Surface corpus, and the
Codex config.toml, agents/ and per-agent toml paths. The refusal strings
and the install-root, destination and branch predicates move into
runtime-artifact-install-plan so the preflight and the write-time guards
share one copy. The write-time guards all stay in place.

Tests: four regression tests in tests/install-runtime-artifacts.test.cjs.
On upstream/next the preservation and alternate-home tests fail and the
refusal and opt-in tests pass; with the fix all four pass.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

This branch has not been deployed

No deployments
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.

bug(install): a refused symlinked destination aborts after gsd-core/ is already deleted, leaving no VERSION or .gsd-runtime

1 participant