Skip to content

v1.80.0.0 fix: setup survives a failed Chromium install, hooks share one state root, gstack never clobbers a skill it did not create - #2802

Merged
garrytan merged 25 commits into
mainfrom
ljubljana
Sep 5, 2026
Merged

v1.80.0.0 fix: setup survives a failed Chromium install, hooks share one state root, gstack never clobbers a skill it did not create#2802
garrytan merged 25 commits into
mainfrom
ljubljana

Conversation

@garrytan

@garrytan garrytan commented Sep 4, 2026

Copy link
Copy Markdown
Owner

Summary

Fork-port Wave A from the time-attack/gstack residual evaluation: three defects in upstream's own tree that the fork surfaced, fixed at the source, plus the ownership model the pre-landing review demanded.

  • ./setup never aborts at the Chromium bootstrap. The Playwright install is best-effort and bounded (600s default, GSTACK_PLAYWRIGHT_INSTALL_TIMEOUT, GSTACK_SKIP_PLAYWRIGHT=1), every failure is a reason code in the final summary, and skills always register. Stale-lock reclaim is atomic and pid-validated; a lock with no recorded holder expires; a wedged installer is killed with its child tree even where pgrep is missing.
  • /freeze and /careful resolve the same state root the writers use (gstack_hook_state_root mirrors bin/gstack-paths). Under GSTACK_HOME the freeze boundary denies instead of allowing everything; a helper from an older install, a newline-terminated root, or an unexpected set -e death all fail closed.
  • gstack never deletes or links over a skill it did not create (fix(relink): don't clobber foreign skills that share a gstack name #2119). Ownership proof has two strengths. Strong (the .gstack-owned marker gstack writes for directories it creates, or a directory holding nothing but symlinks) may remove a directory whole. Weak (byte-identity or the two-line generated banner on a real file) covers only that SKILL.md and our asset links, and a customized file is moved to ~/.gstack/backups/skills/<ts>/ before we link over it. Foreign entries are reported by name. Applies to setup's linker, alias installer, both prefix-flip cleanups, and gstack-relink.
  • Setup's one-shot telemetry events use --no-sweep so they never finalize other sessions' in-flight markers.
  • The evaluation evidence lives under docs/designs/fork-port-residual-2026-09/.

Closes #1900, #1901, #1902, #913, #2233, #1459, #1509, #2119.

Design notes

  • Lock contention is a reason code by design. Another setup holding the lock means this run registers skills and re-probes next time; it does not wait. The plan's "lock held by a live pid exits 1" verification bullet predates that CEO-review decision.
  • Provenance rule. Symlink into gstack (install dir, render dir, any path with a gstack segment, or a tree carrying setup + VERSION + bin/) or the marker = strong. Byte-identity or the two-line banner within the first 8 KB = weak. A directory with no SKILL.md is unclaimed: the link pass may add ours, the cleanup pass leaves it alone. Accepted residuals: the gstack path-segment convention (needed for sibling worktrees and moved checkouts), and a gstack fork that renders the same banner (weak proof only, so the worst case is one backed-up file).
  • Windows legacy installs. Pre-marker copies refresh in place via weak proof; on a prefix flip their copied asset directories stay behind in a SKILL.md-less directory (harmless, reported).

Test plan

  • bun run test (strict sharded runner, six shards, under xvfb-run on this VM): 8,772 tests across 528 files, all six shards green, recorded by gstack-evidence (label tests, exit 0). Seven earlier full runs on this VM each lost exactly one Chromium/PTY/daemon-bound test to shard load or, in single-process mode, to cross-file interference (design/test/feedback-roundtrip, browse/test/terminal-agent-integration, browse/test/handoff, make-pdf/test/e2e/format-gate, browse/test/watchdog, design/test/daemon*); every one of those files is untouched by this branch and passes alone.
  • 103 net new free tests across 10 files: static invariants plus bash harnesses for the bootstrap block (probe/installer stubs, timeout, lock, kill tree without pgrep), the ownership gates (setup linker, alias installer, both flip cleanups, relink), backups, markers, the hook state root (parity with gstack-paths, plugin/GSTACK_HOME precedence, stale helper, backstop), and --no-sweep.
  • Anchor-sliced harnesses fail loudly on command not found instead of degrading into "foreign, skipped".
  • Windows lane (CI): windows-free-tests green on the final head. The first push failed there in test/setup-alias-name-uniqueness.test.ts: the harness passed its assembled script as one bash -c argv element, and MSYS2's runtime inside bash.exe silently truncates any glob-character-bearing argument at 8186 characters (a fixed 8192-byte glob buffer), so the 15.7 KB script was cut mid-token. Every setup harness now runs its script from a temp file via test/helpers/bash-script.ts; the Windows-curated file list is byte-identical before and after.

Plan Completion

Plan file: 31 items in Wave A. 24 DONE, 6 CHANGED (implementation differs from the plan text by design: lock contention is a reason code, not an exit; shared gstack_hook_state_root instead of a per-hook chain; ownership proof has two strengths instead of one gate; markers on every platform; banner match by bytes; tree heuristic), 1 UNVERIFIABLE (issue closing happens on merge via the Closes lines above). Waves B through E2 are scheduled follow-ups, not part of this PR.

TODOS

Documentation

Doc diff preview (commit 23fccb98):

  • README.md: new troubleshooting entry for the Not registered (a skill you own already uses the name; left untouched) setup summary: what proves gstack owns a skill entry (symlink into gstack, .gstack-owned marker, byte-identical or generated-banner SKILL.md), how to resolve a name collision (rename/move, or ./setup --prefix / --no-prefix), and where a customized generated SKILL.md is moved (~/.gstack/backups/skills/<timestamp>/<skill>/SKILL.md). Manual-uninstall snippet now removes .gstack-owned-marked directories (the only signal on Windows copy installs).
  • CLAUDE.md: "Ownership gate (fix(relink): don't clobber foreign skills that share a gstack name #2119)" paragraph in Dev symlink awareness: strong vs. weak proof, backups path, the two-copy rule (setup + bin/gstack-relink), pinning tests.
  • CONTRIBUTING.md: two new "Things to know" invariants (never delete/link over an entry setup cannot prove is gstack's, naming the helper functions in both scripts; ./setup never fails on Chromium, with all seven reason codes and GSTACK_PLAYWRIGHT_INSTALL_TIMEOUT / GSTACK_SKIP_PLAYWRIGHT=1). "Switching prefix mode" notes only gstack-created entries are removed.
  • docs/PROJECT_STRUCTURE.md: added careful/ (check-careful.sh + hook-extract.sh with gstack_hook_state_root), freeze/ (check-freeze.sh, fails closed), guard/, unfreeze/, gstack-upgrade/; gstack-relink in bin list; setup-*/relink/hook-scripts test line; setup entry mentions the best-effort Chromium bootstrap and ownership gate; docs/designs/ names fork-port-residual-2026-09/.
  • docs/TESTING_INTERNALS.md: new "Anchor-sliced setup harnesses" paragraph (extractFn/block anchors, function not found / anchor not found failures, the command not found drift check, how relink.test.ts and hook-scripts.test.ts exercise the real scripts).

CHANGELOG.md, VERSION, TODOS.md untouched by the doc sync. The doc sync flagged that setup's own linker backed up customized files silently; the summary now reports them (commit e64bab7a).

Documentation Debt

  • gstack-telemetry-log --no-sweep: reference only (CHANGELOG + script header); no contributor how-to.
  • gstack_hook_state_root (careful/bin/hook-extract.sh): the writer/reader state-root parity rule is explained in code comments and CHANGELOG only.
  • GSTACK_PLAYWRIGHT_INSTALL_TIMEOUT / GSTACK_SKIP_PLAYWRIGHT: reference + how-to in README; no getting-started walkthrough for a no-browser/CI install.

Review coverage

Review army (Testing, Maintainability, Security, Performance, Data-Migration, API-Contract, Claude Red Team) ran three cycles; every CRITICAL/HIGH/MEDIUM finding was fixed with a regression test. Cycle 1 (pre-landing): the linker and prefix-flip cleanup clobbered same-name skills, the trap left an orphaned installer, the npm chain masked failures, GSTACK_SKIP_PLAYWRIGHT read as a failure, TIMEOUT=0 killed on the first poll, the careful hook read a different state root, and the README fence swallowed the troubleshooting note. Cycle 2: relink's flip cleanup rm -rf'd a real-file directory on Linux/macOS on weak proof (HIGH, fixed by the two-strength model), banner-only proof overwrote a customized file (fixed by backups), relink's fast path drifted from setup (removed), the banner check read 40 lines while four skills carry it past line 40 (bytes now), a worktree named without a gstack segment was foreign (tree heuristic), _kill_tree needed pgrep (/proc fallback), a lock with no pid never expired, and freeze had no backstop for set -e deaths. Cycle 3: runtime assets in an unclaimed or weakly-owned directory were still replaced, the flip cleanup deleted a customized file the link pass would have backed up, a failed backup fell through to an overwrite, a foreign directory symlink slipped through as unclaimed, alias installers stamped markers into pre-existing directories, the lock hand-back missed a fresh lock without a pid, and the /proc parser stopped at the first paren. All fixed, each with a regression test. Accepted and documented: the gstack path-segment convention, a fork rendering the same banner (weak proof only), and Windows legacy asset copies left behind in a SKILL.md-less directory after a flip.

Codex outside voice (two passes, codex exec, sandbox danger-full-access because bwrap is unavailable on this VM)

Pass 1, adversarial (5 findings, all fixed):

  1. P1 "a colliding skill can forge the AUTO-GENERATED comment and trigger recursive deletion" — accepted. Superseded by the two-strength model: banner or byte-identity is now WEAK proof that covers one file and never deletes a directory; only the .gstack-owned marker (we created the dir) or an all-symlinks directory authorizes rm -rf.
  2. P1 "any path segment named gstack bypasses provenance; the fast path judges /x/gstack/../foreign before canonicalizing" — accepted. relink's fast path is removed; both files canonicalize first. The segment convention itself stays (sibling worktrees, moved checkouts) and is documented as accepted.
  3. P1 "the synthetic session id does not prevent sweeping other sessions' pending markers" — accepted and verified against bin/gstack-telemetry-log. New --no-sweep flag; both setup one-shot events use it.
  4. P2 "timeout validation accepts 000 and values bash cannot compare" — accepted. Normalized to a positive integer, nine-digit cap, default on anything else.
  5. P2 "check-careful calls the new helper without an existence check" — accepted. Falls back to ${GSTACK_HOME:-$HOME/.gstack}; freeze already denied.

Pass 2, review (2 findings, both fixed):

  1. P2 relink's _gstack-command alias was written without the ownership gate — gated, and alias copies are stamped with the marker.
  2. P3 the failure summary listed browser skills as if exhaustive — reworded as examples.

Codex's "request changes" verdicts were both acted on; every finding closed with a regression test.

🤖 Generated with Claude Code


Open workspace in Conductor

garrytan and others added 23 commits September 3, 2026 01:18
…osed under GSTACK_HOME (#1459, #1509)

check-freeze.sh resolved its state dir as ${CLAUDE_PLUGIN_DATA:-$HOME/.gstack}
while every writer (/freeze, /guard, /unfreeze, /investigate) resolves through
bin/gstack-paths, GSTACK_HOME first. With GSTACK_HOME set, /freeze wrote
freeze-dir.txt under GSTACK_HOME, the hook read $HOME/.gstack, found no file,
and allowed everything — a deny-tier boundary failing open.

One resolver now: gstack_hook_state_root() in careful/bin/hook-extract.sh
(already sourced by both check-freeze.sh and check-careful.sh) implements the
exact gstack-paths chain, including the CLAUDE_PLUGIN_ROOT guard that keeps a
CLAUDE_PLUGIN_DATA leaked from another plugin from redirecting our state.
check-freeze.sh and gstack_hook_log_fire both call it; nothing spawns
gstack-paths from a hook.

Tests: the GSTACK_HOME deny regression, GSTACK_HOME-over-CLAUDE_PLUGIN_DATA
precedence, plugin-root guard both ways, and a byte-parity check against
bin/gstack-paths across six env combinations. Existing freeze tests now pass
CLAUDE_PLUGIN_ROOT like a real plugin install would.

Idea from PR #1509 (@NikhileshNanduri); implemented natively against the shared
resolver rather than a second fallback chain.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…2119)

gstack-relink runs on every ./setup. Its cleanup did `rm -rf` on any same-name
entry whose SKILL.md was a symlink, with no readlink check, and its link step
did `mkdir -p` then `ln -snf` onto any existing SKILL.md — on Linux that
replaces a user's real file with a symlink into gstack (macOS refused by
accident). setup's Windows mode-flip cleanup deleted any real dir whose name
matched a gstack skill. A personal `qa` skill, or a fork installed under
another path, was destroyed by the installer of a tool it never asked for.

Ownership is now proven, never assumed. An entry is ours when it is a symlink
resolving into INSTALL_DIR or RENDER_DIR, a real dir whose SKILL.md is such a
symlink, or a real dir carrying the .gstack-owned marker setup now writes for
Windows copy installs (legacy copies count when byte-identical to the source
or carrying gen-skill-docs' AUTO-GENERATED header). Anything else — including
an entry whose readlink fails — is foreign: left untouched, reported on
stderr, and listed in relink's summary line. The same rule replaces setup's
Windows name-match deletion; setup:1040 and gstack-uninstall:204 already
gated on readlink, so this closes the last unguarded deleter of the class.

Tests: foreign real dir in flat mode, foreign flat entry on a prefix flip,
foreign directory symlink, RENDER_DIR-targeted entry (ours), marker-carrying
copy (ours), marker-less copy (foreign); the Windows cleanup test now proves
provenance three ways and keeps the user's own same-name skill.

Idea and two regression cases from PR #2119 (@smblight); implemented on the
destination entry, not only the symlink target.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ways register (#1900, #1901, #1902, #913, #2233)

setup runs under `set -e`, and the Chromium bootstrap in section 2 sat ahead
of skill registration in section 4 with a bare `bunx playwright install
chromium`, an unbounded download, and an explicit `exit 1` after the
post-install launch probe. On an offline, proxied, or AppArmor-restricted box
the user ended with ZERO skills registered and a re-run that died at the same
line; a wedged download hung setup indefinitely.

Every browser failure now records a reason code in _PW_FAIL_REASON and setup
continues: skipped (GSTACK_SKIP_PLAYWRIGHT=1, #913), chromium-install,
chromium-install-timeout (the download is bounded by the existing
_wait_with_deadline helper, default 600s, env GSTACK_PLAYWRIGHT_INSTALL_TIMEOUT,
process tree killed via _kill_tree), chromium-install-locked (another setup
holds the lock: this one registers skills and re-probes next time instead of
exiting), windows-no-node, windows-node-modules, post-install-launch (with the
GSTACK_CHROMIUM_NO_SANDBOX=1 hint for Ubuntu 24.04's userns policy, #2157).
The daemon font refresh is skipped when Chromium is unavailable. The final
summary names the skills that need the browser (/qa, /qa-only,
/design-review, /browse, make-pdf, /pair-agent) and the fix for the recorded
reason, and logs the reason code (never a path) through gstack-telemetry-log
when telemetry is on.

Tests: static invariants over the anchor-sliced block (no exit, every reason
code, deadline helper, trap chaining, guarded refresh, summary contents) plus
an integration harness that executes the real block with a stubbed probe and
installer: install failure, hang killed at the deadline with the tree kill
recorded, non-numeric knob fallback, live lock (continues, installer not run,
lock preserved), stale lock reclaimed, post-install probe failure, and the
skip flag.

Credit @DavidMiserak (PR #1900) for the best-effort shape; re-implemented on
the current block.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The read-only evaluation of what remains portable from time-attack/gstack
(583 raw candidates, 415 canonical, 287 with a residual, 48 adversarially
refuted, 14 standing) lived only on a throwaway VM. This records the report,
the lite residual index, the absorbed/superseded ledger, the refuter
verdicts, and SHAS.md with the fork tip, upstream HEAD, merge-base, and a
sha256 per file, so every scheduled fix in this wave series traces to its
evidence.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… bootstrap knobs

TODOS.md gains the seven items the CEO and eng reviews of the fork-port
residual plan deliberately deferred (shared ownership helper, config-key
reader tripwire, "pre-existing" vocabulary, opt-in reply_language, .auth.json
writer removal, the fork-derived-change rule for CONTRIBUTING, hook slug
parity audit), each with rationale, and updates the two residual bullets for
PR #2232 and PR #2233 with their dispositions. README's Troubleshooting
section explains the best-effort Chromium bootstrap and its three knobs.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Pre-landing review finding: the ownership gate compared readlink output
textually against INSTALL_DIR and RENDER_DIR, so two shapes of gstack's OWN
entries read as foreign and were left behind on a mode flip — a legacy
relative link (`gstack/qa/SKILL.md`, resolved against $PWD instead of the
link's directory) and an entry linked against the real path of a symlinked
install dir (~/.claude/skills/gstack -> checkout). Both now resolve: relative
targets anchor at the link's directory, the directory part is canonicalized
with pwd -P (the basename stays verbatim so a dangling managed target is not
misread), and both spellings of each root are accepted. Two regression tests.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ding markers

gstack-telemetry-log finalizes every .pending-<session> marker that is not
the caller's own as outcome:unknown and deletes it. setup's onboarding
events (_setup_welcome, _setup_playwright) have no session of their own, so
a Chromium bootstrap failure during a live skill session recorded a false
unknown for that session and removed its marker.

New --no-sweep flag skips the stale-marker pass; both setup call sites use
it (the synthetic --session-id did not prevent the sweep). Surfaced by the
Codex adversarial pass.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… careful

A hook script and its sourced helper can be copied at different times. With
an older careful/bin/hook-extract.sh that lacks gstack_hook_state_root:

- check-freeze.sh now emits a deny ("fail closed, re-run ./setup or
  /unfreeze") instead of dying under set -e with no decision JSON.
- check-careful.sh falls back to ${GSTACK_HOME:-$HOME/.gstack} so project
  rules under the plain chain still load and a decision is always emitted
  (a warn hook must never break on a stale helper).

gstack_hook_state_root prints its root without a trailing newline and both
callers capture it with a printf-x sentinel, so a GSTACK_HOME ending in a
newline round-trips byte-for-byte with the writer's %q form.
gstack_hook_log_fire stays on ${GSTACK_HOME:-$HOME/.gstack}/analytics, the
same two-step chain every other analytics writer and reader uses, so the
usage log remains one file under a plugin install.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ot own (#2119)

The relink gate alone left three destructive sites open:

- link_claude_skill_dirs runs BEFORE relink on every ./setup and used
  `ln -snf` (Linux replaces a user's real SKILL.md with a symlink into
  gstack) or, on Windows, rm -rf + cp followed by a marker that made the
  user's directory "ours" on the next flip. It and _install_alias_skill_md
  now consult _claude_entry_is_ours first and skip loudly.
- cleanup_prefixed_claude_symlinks kept a bare name-match deletion and a
  `*gstack*` substring match. Symlink arms use anchored `gstack/` segment
  patterns; the Windows real-file arm proves provenance (marker,
  byte-identity with our source, or the full two-line gen-skill-docs banner
  within the first 40 lines, never a one-line substring another generator
  could emit). cleanup_old_claude_symlinks uses the same banner rule.
- gstack-relink's fast path judged absolute targets before canonicalizing,
  so `/x/gstack/../foreign/SKILL.md` counted as ours; dot-segment targets
  now canonicalize first. Its banner rule matches setup's.

The `.gstack-owned` marker records the owning payload's realpath. Entries
skipped by setup or relink are listed in the final setup summary.

Chromium bootstrap refinements from the pre-landing review: an INT/TERM
trap kills the installer's process tree; the Windows npm chain no longer
masks an install failure; GSTACK_SKIP_PLAYWRIGHT=1 is reported as a choice
rather than a failure and sends no telemetry; the timeout knob is
normalized (0, 000, non-numeric, or more than nine digits fall back to the
600s default instead of killing on the first poll or never killing).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… stripped; deferrals name the four gate sites

- README: the Chromium troubleshooting paragraph sat inside the CLAUDE.md
  snippet code fence, so copy-paste put it into users' CLAUDE.md. Moved to
  the troubleshooting list.
- docs/designs/fork-port-residual-2026-09/REPORT.md: the scratch-run
  preamble banner is gone; SHAS.md re-hashed.
- TODOS: the ownership-gate deferral names the four sites and the
  marker-path idea for the fork-with-banner residual.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…arantine helper

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…r deletes a directory or discards a differing file

The first #2119 gate treated a byte-identical or banner-bearing real-file
SKILL.md as full ownership, so a prefix flip could rm -rf a user's directory
(their own qa skill started from a gstack SKILL.md, plus my-templates/) and
the link pass could replace their customized file with a symlink. Two
strengths now:

- STRONG: the .gstack-owned marker (we created the directory), or a
  directory holding nothing but symlinks and the marker (deleting it loses
  no data). Only strong proof removes a directory whole.
- WEAK: byte-identity with our source or the two-line gen-skill-docs banner
  on a real file. Weak proof covers that SKILL.md and our runtime-asset
  links only; a differing file is moved to
  ${GSTACK_HOME:-~/.gstack}/backups/skills/<ts>/<skill>/ before we link
  over it, and setup/relink print one summary line naming what moved.

The marker is written on every platform now (path-independent proof for
Windows copies and for checkouts whose path carries no gstack segment), but
only for a directory gstack creates: a directory we merely link into
(unclaimed, or a legacy install) never becomes deletable whole. A directory
with no SKILL.md at all is unclaimed: the link pass may add our file, the
cleanup pass has nothing to remove.

Also from the review passes: the banner check reads 8192 bytes, not 40
lines (investigate, office-hours, plan-ceo-review and design-consultation
carry the banner past line 40 and were left "foreign" on pre-marker
Windows installs); a link into a checkout named without a gstack segment
(git worktree add ../gstack-<branch>) is ours when that tree carries
setup + VERSION + bin/; relink's fast path is gone so both files
canonicalize before judging; relink's root alias (_gstack-command) is
gated and stamped like every other entry; relink reports the bare entry
name with setup's wording and setup dedupes when forwarding
(_run_relink_quiet); the summary names the browser skills as examples.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…; abandoned locks expire; the tree kill walks /proc without pgrep

- A pid file holding "", "-1" or "0" counted as a live holder (kill -0 -1
  signals every process and succeeds), locking Chromium out for good. A pid
  must be a positive integer; anything else is stale.
- Two setups judging the same lock stale raced on rm -rf + mkdir and the
  loser deleted the winner's fresh lock. The stale dir is renamed first
  (atomic), so exactly one reclaims.
- A lock dir with no pid file (killed between mkdir and echo) was never
  reclaimed; it now expires once older than the install bound.
- _kill_tree needed pgrep; debian-slim and git-bash ship none, so the bound
  killed only the wrapper subshell and the installer kept running. Without
  pgrep the children are found by walking /proc/*/stat.
- The timeout knob is normalized in one place with one comment; the trap's
  exit 130 is the only exit the block may contain.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… instead of exiting with no decision

set -e plus a failing pipeline (a tool on PATH exiting non-zero, a deleted
cwd) ended the deny-tier hook with no JSON, which Claude Code treats as
non-blocking: the edit outside the boundary proceeded. The EXIT trap now
prints a deny for any non-zero exit that happens before a decision was
written; every deliberate output sets _FREEZE_DECIDED first so a late
failure never prints a second object.

Tests also pin careful's state-root precedence (GSTACK_HOME over
CLAUDE_PLUGIN_DATA, plugin data when CLAUDE_PLUGIN_ROOT names gstack) and
the specific "out of date" deny for a helper without gstack_hook_state_root.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…reak inside the loop

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…rms inline their own chain

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…s; the marker is the one allowed dotfile

setup-claude-skill-assets and user-render-out-dir-install slice
link_claude_skill_dirs out of setup without the helpers it now calls, so
the extracted function died with "command not found" (or, inside an if,
degraded into "foreign, skipped"). Both harnesses now carry the full helper
set and the globals. The hidden-files census allows .gstack-owned, which
the linker writes for directories it creates rather than copying from the
skill source.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…lips, failed backups, foreign dir links, alias markers

Third review cycle on the ownership model, every item reproduced against a
fixture before the fix:

- Runtime assets (sections/, templates/, checklist.md, ...) were refreshed
  with rm -rf regardless of who owned the directory, so an unclaimed or
  weakly-owned directory lost the user's same-named real files. Real assets
  are now replaced only in a directory gstack created or strongly owns
  (marker, or SKILL.md symlink into gstack), plus the legacy Windows
  real-copy shape; elsewhere they are kept and reported. Symlinks are never
  content and are always refreshed.
- The prefix-flip cleanup deleted a customized banner-bearing SKILL.md that
  the link pass would have backed up. Both cleanups now compare the file
  against the source (raw, or with its name: line rewritten to the entry
  name, which is how alias and prefixed copies legitimately differ) and
  move a differing file to the backup root.
- A failed backup (unwritable root) returned success and the caller linked
  over the file anyway. It now fails, and the entry is left untouched and
  reported.
- A foreign DIRECTORY symlink whose target had no SKILL.md fell through to
  the "unclaimed directory" rule and was replaced by a real directory. A
  symlink that does not resolve into gstack is foreign, full stop.
- The alias installers stamped .gstack-owned into pre-existing directories;
  they now follow the same created-or-already-marked rule.
- A directory counts as "only links" only when every link resolves into
  gstack: a user's own symlink makes it mixed, so their link survives.
- The gstack-tree heuristic requires bin/gstack-relink, not just a VERSION
  file, a setup script and a bin/ directory.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…the bound is stale; /proc walk strips through the last paren

- Reclaim renamed the lock by path after judging it stale, so a second setup
  that had already reclaimed and re-created it lost its fresh lock and two
  installers ran. After the rename the moved directory's pid is re-read: a
  new live holder, or a fresh lock whose pid is not written yet, is moved
  straight back.
- A pid file whose process is alive but whose lock is older than the install
  bound is stale too (the holder is past its own deadline, or the pid was
  recycled to an unrelated long-lived process); it was locked forever.
- The /proc fallback stripped the comm field to the FIRST ") ", so a comm
  containing ") " hid a child from the kill. proc(5) says the last paren.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…efore

If gstack_hook_decision ever failed between the flag and its output the
backstop would have stayed silent; setting the flag after the print keeps
the deny backstop armed until a decision is actually on stdout.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
README troubleshooting + manual uninstall cover the skill ownership gate
(.gstack-owned marker, ~/.gstack/backups/skills/<ts>/, foreign same-name
skills left untouched). CLAUDE.md and CONTRIBUTING carry the ownership and
best-effort Chromium bootstrap invariants for people editing setup and
gstack-relink. PROJECT_STRUCTURE gains careful/, freeze/, guard/, unfreeze/,
gstack-upgrade/, gstack-relink, and the setup/relink/hook test files.
TESTING_INTERNALS documents the anchor-sliced setup harness convention.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… to the backup root

The linker moved a weakly-proven, customized SKILL.md aside before linking
over it but never said so; only relink printed a "Moved N" line, and by the
time relink runs the file is already a symlink. The summary now names each
moved file and where it went, next to the foreign-entry report.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@trunk-io

trunk-io Bot commented Sep 4, 2026

Copy link
Copy Markdown

😎 This pull request was merged.

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown

E2E Evals: ✅ PASS

0/0 tests passed | $0 total cost | reconcile exit: 0

Shard Result Status Cost
Fail-closed reconciliation
[test:paid] report: 6/6 slices, 4 planned shards, tier=gate
  slice 1  passed              0s  test/llm-judge-recommendation.test.ts
  slice 2  passed              0s  test/skill-llm-eval-spec.test.ts
  slice 3  passed              0s  test/skill-llm-eval.test.ts
  slice 4  passed              0s  test/skill-routing-e2e.test.ts
[test:paid] report: ⚠ 3 shard(s) passed with EVERY test skipped — they verified nothing:
  ⚠ test/llm-judge-recommendation.test.ts (1 skipped — external service missing or tier mismatch)
  ⚠ test/skill-llm-eval.test.ts (25 skipped — external service missing or tier mismatch)
  ⚠ test/skill-routing-e2e.test.ts (10 skipped — external service missing or tier mismatch)
[test:paid] report: every planned shard accounted and passed

Sliced lane: diff-selected gate census via scripts/test-paid-shards.ts (planner → 6 executors → fail-closed report)

garrytan and others added 2 commits September 4, 2026 23:05
… -c` argv (Windows MSYS2 8 KB truncation)

windows-free-tests (run 33907177851) failed in
test/setup-alias-name-uniqueness.test.ts with
  bash: -c: line 178: unexpected EOF while looking for matching `'
The harness slices functions out of `setup` and passed the joined script as
one `bash -c` argv element. The ownership gate grew that script from 6.7 KB
to 15.7 KB, and on Windows bash is an MSYS2 program: when its parent is a
non-MSYS process (bun), msys-2.0.dll's build_argv() runs any argument
containing `?*["'(){}` through globify()/glob(), which copies the pattern
into a fixed `Char patbuf[8192]` and silently stops after 8192 - MB_CUR_MAX
(8186 chars under C.UTF-8); GLOB_NOCHECK then returns the truncated text as
the argument. Character 8186 lands inside the single-quoted sed token on
line 178. Rebuilding the exact script with CI path shapes and cutting it at
8186-8190 characters reproduces the identical message locally; cmd.exe's
8191-UTF-16 cap and CreateProcess's 32767 do not fit the evidence.

Fix: test/helpers/bash-script.ts writes the script to a temp file and runs
`bash <path>` — a short glob-free argument that never enters globify. Every
setup harness that assembled a script for `bash -c` (11 files, 22 sites)
uses it; timeouts and env are preserved verbatim, spawn/timeout errors are
appended to stderr, temp cleanup is best-effort. `spawnSync('bash',
[<Windows absolute path>])` already passes on windows-latest in setup-help,
uninstall-windows-copies and the migration tests. The Windows-curated list
is byte-identical before and after.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…le> via test/helpers/bash-script.ts, not bash -c

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@garrytan
garrytan merged commit c241216 into main Sep 5, 2026
21 checks passed
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