From bc2e4395accc89c8b2285e728e29f62fe2876596 Mon Sep 17 00:00:00 2001 From: Joe Slitzker Date: Fri, 26 Jun 2026 16:43:31 -0500 Subject: [PATCH] fix(#1477): warn on marker-write failure; writer fault-injection test Address re-review minors on #1487: - bin/install.js: replace the silent .gsd-source marker catch with a console.warn so a failed write is diagnosable (walk-up also fails on the Claude-global layout, so a swallowed error still breaks /gsd-surface). - tests/runtime-artifact-layout.test.cjs: add a writer fault-injection case (mock fs.writeFileSync to throw for the marker) proving the catch branch is reachable, install stays non-fatal, and the warning is emitted. --- bin/install.js | 7 +++- tests/runtime-artifact-layout.test.cjs | 47 +++++++++++++++++++++++++- 2 files changed, 52 insertions(+), 2 deletions(-) diff --git a/bin/install.js b/bin/install.js index 5658452cd..c951729b2 100755 --- a/bin/install.js +++ b/bin/install.js @@ -9912,7 +9912,12 @@ function install(isGlobal, runtime = 'claude', options = {}) { if (fs.existsSync(gsdSourceCommands)) { try { fs.writeFileSync(path.join(targetDir, '.gsd-source'), gsdSourceCommands + '\n', 'utf8'); - } catch (_) { /* non-fatal: surface degrades to walk-up resolution */ } + } catch (err) { + // Non-fatal: install proceeds. But on the Claude-global layout walk-up + // also fails (no commands/gsd source tree), so a silent write failure + // still leaves /gsd-surface broken at runtime — warn so it's diagnosable. + console.warn(` ${yellow}!${reset} Could not write .gsd-source marker (${err.message}); /gsd-surface list/status may fail`); + } } } diff --git a/tests/runtime-artifact-layout.test.cjs b/tests/runtime-artifact-layout.test.cjs index 250b14bfa..ecb35b783 100644 --- a/tests/runtime-artifact-layout.test.cjs +++ b/tests/runtime-artifact-layout.test.cjs @@ -17,7 +17,7 @@ * runtime-artifact-layout-install-profiles.test.cjs — install-profiles seam */ -const { test, describe, beforeEach, afterEach } = require('node:test'); +const { test, describe, beforeEach, afterEach, mock } = require('node:test'); const assert = require('node:assert/strict'); const fs = require('fs'); const path = require('path'); @@ -776,6 +776,51 @@ describe('#1477 .gsd-source marker provisioning', () => { ); }); + // ── Writer fault-injection: marker write failure is non-fatal + warns ──────── + // CONTRIBUTING.md filesystem-write QA matrix: prove the marker-writer catch + // branch (bin/install.js) is reachable. A failed write (e.g. read-only target) + // must not abort the install, and — because walk-up also fails on this layout — + // must surface a diagnostic so the broken /gsd-surface is traceable. + test('marker write failure does not abort install and warns (locks the writer catch)', () => { + const claudeDir = path.join(tmpRoot, '.claude'); + fs.mkdirSync(claudeDir, { recursive: true }); + + const markerPath = path.join(claudeDir, '.gsd-source'); + const realWriteFileSync = fs.writeFileSync; + // Fault-inject ONLY the marker write; every other install write proceeds. + mock.method(fs, 'writeFileSync', (file, ...rest) => { + if (path.resolve(String(file)) === path.resolve(markerPath)) { + throw new Error('EACCES: read-only target (injected)'); + } + return realWriteFileSync(file, ...rest); + }); + + // Capture console.warn around the install (runInstall's silenceConsole would + // otherwise swallow it); still guard process.exit. + const warnings = []; + const origExit = process.exit; + const origWarn = console.warn; + const origLog = console.log; + process.exit = (code) => { throw new Error(`process.exit(${code}) during install`); }; + console.warn = (msg) => { warnings.push(String(msg)); }; + console.log = () => {}; + try { + assert.doesNotThrow(() => install(true /* isGlobal */, 'claude'), + 'a marker write failure must be non-fatal — install proceeds via the catch'); + } finally { + process.exit = origExit; + console.warn = origWarn; + console.log = origLog; + mock.restoreAll(); + } + + assert.ok(!fs.existsSync(markerPath), 'the injected fault must leave no marker on disk'); + assert.ok( + warnings.some((w) => /\.gsd-source marker/.test(w)), + `the writer catch must warn for diagnosability; got: ${JSON.stringify(warnings)}`, + ); + }); + // ── Failure 1 end-to-end: resolution succeeds FROM the deployed tree ───────── // The deployed module's __dirname is /gsd-core/bin/lib, which has no // commands/gsd ancestor (global skills layout). Only the marker rescues it.