diff --git a/CHANGELOG.md b/CHANGELOG.md index 13d052972..12d41f534 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -57,6 +57,7 @@ The first two rows are the ones to care about: a review that read nothing, or fo - **Required team mode also blocks Copilot CLI's skill tool.** Re-run `gstack-team-init required` to upgrade the project hook. - **`gstack-qa-evidence capture` prints a `revalidate` list after your inputs change**, naming the probes to rerun on current inputs before `materialize`; `materialize` says its verdict is final for that report root, so rerun the listed probes first. - **/gstack-upgrade asks you to paste the absolute path Step 2 prints**; every later step checks it is inside gstack's checkout before running git. +- **Two setups writing settings.json at once both land.** A writer whose lock attempt raced a holder releasing it used to skip its hook change (exit 5); it now retries once and only reports `cannot create lock` when the lock directory truly cannot be created. - **/plan-ceo-review prints its mode handoff** by running `gstack-ceo-mode-handoff` right after the mode is chosen (by you or by a saved preference), so the selected mode and approved decisions always appear before the review continues. - **Browser setup names the platform when Aside is absent:** the probe prints `NEEDS_ASIDE: ` (was `NEEDS_ASIDE`), and the macOS-only download pitch trusts that line instead of re-checking the OS. `GSTACK_PLATFORM` overrides it for tests and unusual hosts; set it in your shell, never a project `.env`. diff --git a/bin/gstack-settings-hook b/bin/gstack-settings-hook index d97efe6e8..620a2c8c8 100755 --- a/bin/gstack-settings-hook +++ b/bin/gstack-settings-hook @@ -337,7 +337,7 @@ _release_lock() { _acquire_lock() { # GSTACK_SETTINGS_LOCK_TIMEOUT_MS: test-only override for the give-up # (a contention test should not stall the suite for 10 real seconds). - local waited_ms=0 token stale mtime now + local waited_ms=0 token stale mtime now vanished=0 local give_up_ms="${GSTACK_SETTINGS_LOCK_TIMEOUT_MS:-10000}" local stale_after_s=30 # lock older than this belongs to a crashed holder local poll_ms=50 # retry cadence; sleep below derives from this @@ -351,15 +351,17 @@ _acquire_lock() { trap 'exit 143' TERM return 0 fi - # mkdir failed but no lock dir exists: NOT contention (unwritable parent, - # read-only fs, missing directory) -- waiting cannot help, so give up - # loudly now instead of spinning out the full timeout. (Tiny race: a - # contender could acquire+release between our mkdir and this check; that - # transient reads as an environment failure and the next run converges.) + # mkdir failed but no lock dir exists: either a holder released between + # our mkdir and this check, or NOT contention (unwritable parent, read-only + # fs, missing directory). Retry mkdir once; a second failure with still no + # lock dir is the environment, and waiting cannot help, so give up loudly + # now instead of spinning out the full timeout. if [ ! -e "$_LOCK_DIR" ]; then + if [ "$vanished" -eq 0 ]; then vanished=1; continue; fi echo "gstack-settings-hook: cannot create lock $_LOCK_DIR (unwritable parent?) -- skipping this mutation, exit 5" >&2 return 1 fi + vanished=0 # Stale takeover: atomic rename means exactly one contender wins; the # loser loops and re-contends against the winner's fresh mkdir. # GNU stat (-c %Y) first: on Linux, BSD-style `stat -f %m` prints a diff --git a/test/gstack-settings-hook-symlink.test.ts b/test/gstack-settings-hook-symlink.test.ts index cff8e07a6..07f229b35 100644 --- a/test/gstack-settings-hook-symlink.test.ts +++ b/test/gstack-settings-hook-symlink.test.ts @@ -102,6 +102,38 @@ describe('settings hook preserves the resolved settings target', () => { expect(fs.existsSync(settings + '.lock')).toBe(false); }); + // Ubicloud run 21: a contender's lock released between this writer's failed + // mkdir and its existence check, and the writer dropped its mutation (exit 5). + test.each([ + ['a lock released between mkdir and the check is retried', 'once', 0], + ['a lock that can never be created still fails fast', 'always', 5], + ] as const)('%s', (_name, failures, status) => { + const bin = path.join(root, 'bin-vanish'); + fs.mkdirSync(bin); + const realMkdir = spawnSync('which', ['mkdir'], { encoding: 'utf8', timeout: 5_000 }).stdout.trim(); + expect(realMkdir).not.toBe(''); + fs.writeFileSync(path.join(bin, 'mkdir'), [ + '#!/bin/bash', + 'if [ "$1" = "$LOCK_EXPECTED" ]; then', + ' if [ "$LOCK_FAILS" = always ] || [ ! -e "$LOCK_MARK" ]; then : > "$LOCK_MARK"; exit 1; fi', + 'fi', + 'exec "$REAL_MKDIR" "$@"', + ].join('\n') + '\n', { mode: 0o755 }); + const result = spawnSync('bash', [hook, ...addArgs()], { + env: { ...env(), PATH: `${bin}:${process.env.PATH}`, LOCK_EXPECTED: target + '.lock', LOCK_FAILS: failures, + LOCK_MARK: path.join(root, 'lock-failed-once'), REAL_MKDIR: realMkdir, GSTACK_SETTINGS_LOCK_TIMEOUT_MS: '2000' }, + encoding: 'utf8', timeout: 10_000, + }); + expect(result.status, result.stderr).toBe(status); + const stored = fs.readFileSync(target, 'utf8'); + if (status === 0) expect(JSON.parse(stored).hooks.PostToolUse[0]._gstack_source).toBe('link-test'); + else { + expect(result.stderr).toContain('cannot create lock'); + expect(stored).not.toContain('link-test'); + } + expect(fs.existsSync(target + '.lock')).toBe(false); + }); + test('dangling and cyclic links are rejected without replacing them', () => { fs.unlinkSync(target); const dangling = run(addArgs());