Repository navigation
Conversation
…eckout (garrytan#3084) An older install can keep its gstack checkout where a host's runtime root goes, such as ~/.cursor/skills/gstack, and ./setup --status tells that user to run ./setup --host cursor from there. _activate_runtime_root then moved the checkout aside, swapped the new runtime root in and deleted the old one. The checkout and every host that pointed at it were gone, while the summary said the previous install was left in place. Cursor, Factory, OpenCode, Copilot and agy all activate through it; Copilot and agy even reported "installed" and exited 0. Refuse before anything moves when the root's resolved path is, or contains, the resolved source, as the Codex arm already refuses a runtime directory that contains the source checkout. A symlinked root still loses only the link, so older symlinked installs keep upgrading. The new message gets a troubleshooting entry beside this function's other error, and the real ./setup is tested from a checkout at each of the five hosts' runtime roots. Fixes garrytan#3084 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Merging to
After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here |
garrytan
added a commit
that referenced
this pull request
Oct 11, 2026
… checkout; pin the symlinked-root control (#3128) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
Contributor
|
Superseded by #3140, merged in v1.91.89.0 (e6edff0). Setup refuses to replace a runtime root that is or contains its checkout, on every host that swaps a root, Kiro included. You confirmed #3128's five end-to-end layouts are all refused there. Credited in CHANGELOG ( |
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.
Why (in your own words)
If your gstack checkout lives in a host's skills directory, such as
~/.cursor/skills/gstack(the reporter's checkout, set up on 1.31),./setup --host cursordeletes it. That is the exact command./setup --statussuggests for such an install ("register it: cd ~/.cursor/skills/gstack && ./setup --host cursor")._activate_runtime_rootbuilds the new runtime root beside the old one, moves the old root aside and deletes it withrm -rf. When the old root is the checkout, the checkout goes with it, along with every host that pointed at it (the reporter's~/.claude/skills/gstacksymlink broke too). The install summary still says "its previous install was left in place", and its retry command points at asetupthat no longer exists.It is not Cursor-only. Every arm that swaps its root through
_activate_runtime_rootwith no source check of its own has it: Cursor, Factory, OpenCode, Copilot and agy. (Codex moves such a checkout out first.) The Copilot and agy arms check_sidecar_root_user_ownedfirst, but a checkout's own rootSKILL.mdcarries theAUTO-GENERATED frombanner, so that check passes it through. On those two hosts the run even reportsinstalledand exits 0.Fixes #3084.
Why this shape
This is @andrewkjankowski's first suggested fix: "Refuse in
_activate_runtime_root, or in the Cursor arm, when the resolved runtime root equals or containsSOURCE_GSTACK_DIR, as the Codex arm does." It goes in_activate_runtime_root, not the Cursor arm, because all five arms share the swap.The check mirrors the Codex refusal at
setup:568-576: skip a symlinked root, resolve withpwd -P, thencase "$SOURCE" in "$runtime"/*. It also matches equality, and it resolves the source as well. Like the function's other error paths it returns 1 before anything moves, so the summary's "left in place" is now true.The tests extend the two harnesses that already cover this function and these hosts.
test/setup-skill-copies.test.tsslices_activate_runtime_rootout ofsetup, andtest/host-conformance.test.tsruns the real./setupin a clean HOME.CONTRIBUTING's wave rule: if two PRs fix the same thing, keep the smaller one. This is 4 files on current
main(one function insetup, two test files, one troubleshooting entry), and fine to absorb into a wave.Not followed:
~/.gstack/repos/gstack, the waymigrate_direct_codex_installdoes for Codex. Moving a user's checkout is a product call, and fix(setup): relocate direct Claude install out of the skill-discovery root (#1202, #1694) #1819 was closed in favor of fixing the install footprint "without relocating the install".--statushint and the failure-summary wording), and its note about unprefixed Cursor folders. They are real but separate; see below.What it does
setup,_activate_runtime_root: before the leftover sweep and the staged build, it checks the root. If the root is a real directory (not a symlink) and its resolved path is, or contains, the resolved source, it printsrefusing to replace <root>: it holds the gstack checkout setup is running from (<source>)…and returns 1. The function's header comment says why.test/setup-skill-copies.test.ts, refusal: three layouts are refused with the checkout unchanged. The root is the checkout, contains it, or is the checkout reached through a symlinked parent directory.test/setup-skill-copies.test.ts, control: a root that is a symlink to the checkout is still replaced, as it is today, and the checkout it pointed at survives. Only the link is removed.test/host-conformance.test.ts: one case per installable host, run from a checkout at that host's runtime root. The real./setup --host <host>exits 1 with the refusal, and the checkout keeps every entry and itssetup. Three hosts are excluded:~/.gstack/repos/gstack.docs/troubleshooting.md: an entry for the new message, next to the entry for this function's other error (could not back up an edited SKILL.md copy).What it deliberately does not do
--statushint (bin/gstack-install-registry.sh:272). It still suggests the command, which now gets a refusal that says what to do, instead of losing the checkout.VERSION/CHANGELOG/ README / ETHOS untouched;/shipnot run (the wave stamps the version).AI was used for assistance.
Live evidence
Isolation: each run gets a fresh temp
HOMEwith the revision's tree (git archive) at the host's runtime root. The compiled binaries are stubbed,bun run buildis refused, andGSTACK_SKIP_PLAYWRIGHT/GSTACK_SETUP_SKIP_CSO_BUILD/GSTACK_SKIP_FONTSare set, with--no-plan-tune-hooks, the waytest/helpers/install-fixture.tsruns setup. Theexit=lines are the run's exit status. Paths are shortened to~, and trimmed output is marked....Before (
upstream/mainat5d647455f):After (this branch,
4c08ce486):Scope
setup(+13/−2),test/setup-skill-copies.test.ts(+49),test/host-conformance.test.ts(+19),docs/troubleshooting.md(+11). No VERSION, no CHANGELOG, no generated files.upstream/main(the "red on tip" check), with the branch's test files copied into a clean worktree:setupor slices functions out of it (61 files, each in its own process): 1,430 pass, 1 fail. The failure is the pre-existing C3 ordering case described below; withLC_ALL=Cthat file passes 4 of 4.bun run typecheckis clean, andbun run typecheck:testreports "865 known diagnostics, none new".bun run testis not green on this Linux box, onupstream/maineither. Chromium can't launch here, there is no Xvfb, and the /cso suites needbun run build:csoand Docker.design/test/feedback-roundtrip,gstack-capy-install(exit 127: a tool this box lacks),office-hours-attempt,qa-only-cleanup,test-free-shards-settlement-lifecycle, and the C3 locale case.upstream/mainand on this branch: 122 and 121 failures. No failure occurs only on this branch; the extra one onupstream/mainis a flaky browser-settlement case.bin/gstack-redact --repo-visibility publicon the diff reports one MEDIUMpii.ip_public. It is on an unchanged context line (toBe('1.91.12.0'), a version string); the added lines alone scan clean.upstream/mainand on this branch: 640 findings each, none in the changed files.bun run eval:select --base upstream/mainputs this diff on the full-fallback profile, becausesetupis a shared runtime input. Those paid lanes need secrets a fork PR doesn't get.skipIf(win32), like the rest of their files.Found and left alone:
./setup --host kirolinksbin,lib,browse/distandbrowse/binonto themselves inside it (bin/bin -> ~/.kiro/skills/gstack/bin). It then stops withln: '…/ETHOS.md' and '…/ETHOS.md' are the same file(exit 1). That is a different mechanism (Kiro builds its root in place), so it is not in this PR.test/setup-skill-copies.test.ts. It fails under a non-C collation (en_US.UTF-8here), onupstream/maintoo, because_record_skill_copieslistsgstack-upgrade/SKILL.md SKILL.mdin that order. It passes withLC_ALL=C, and CI runsC.UTF-8.Liveness proof (required for external contributors)
Checklist
GSTACK PRtyped live into a real surface) or PR author is @garrytan (owner exemption)Test plan
bun test test/setup-skill-copies.test.ts test/host-conformance.test.ts -t 3084: red onupstream/main, green heresetup(above)bun run test: the same environment failures asupstream/main, none only on this branchFork PRs do not get eval secrets; the free file above is the gate.
Fixes #3084