diff --git a/bin/install.js b/bin/install.js index faf23d6e..30bf7742 100755 --- a/bin/install.js +++ b/bin/install.js @@ -6786,6 +6786,26 @@ function installSdkIfNeeded() { emitSdkFatal('Failed to `npm install -g .` from sdk/.', { globalBin: null, exitCode: 1 }); } + // 3a. Explicitly chmod dist/cli.js to 0o755 in the global install location. + // `tsc` emits files at process umask (typically 0o644 — non-executable), and + // `npm install -g` from a local directory does NOT chmod bin-script targets the + // way tarball extraction does. Without this, the `gsd-sdk` bin symlink points at + // a non-executable file and `command -v gsd-sdk` fails on every first install + // (root cause of #2453). Mirrors the pattern used for hook files in this installer. + try { + const prefixRes = spawnSync(npmCmd, ['config', 'get', 'prefix'], { encoding: 'utf-8' }); + if (prefixRes.status === 0) { + const npmPrefix = (prefixRes.stdout || '').trim(); + const sdkPkg = JSON.parse(fs.readFileSync(path.join(sdkDir, 'package.json'), 'utf-8')); + const sdkName = sdkPkg.name; // '@gsd-build/sdk' + const globalModulesDir = process.platform === 'win32' + ? path.join(npmPrefix, 'node_modules') + : path.join(npmPrefix, 'lib', 'node_modules'); + const cliPath = path.join(globalModulesDir, sdkName, 'dist', 'cli.js'); + try { fs.chmodSync(cliPath, 0o755); } catch (e) { /* Windows / path not found */ } + } + } catch (e) { /* Non-fatal: PATH verification in step 4 will catch any real failure */ } + // 4. Verify gsd-sdk is actually resolvable on PATH. npm's global bin dir is // not always on the current shell's PATH (Homebrew prefixes, nvm setups, // unconfigured npm prefix), so a zero exit status from `npm install -g` diff --git a/hooks/gsd-statusline.js b/hooks/gsd-statusline.js index 9bfbdb1f..11687b97 100755 --- a/hooks/gsd-statusline.js +++ b/hooks/gsd-statusline.js @@ -147,10 +147,15 @@ function runStatusline() { if (sessionSafe) { try { const bridgePath = path.join(os.tmpdir(), `claude-ctx-${session}.json`); + // used_pct written to the bridge must match CC's native /context reporting: + // raw used = 100 - remaining_percentage (no buffer normalization applied). + // The normalized `used` value is correct for the statusline progress bar but + // inflates the context monitor warning messages by ~13 points (#2451). + const rawUsedPct = Math.round(100 - remaining); const bridgeData = JSON.stringify({ session_id: session, remaining_percentage: remaining, - used_pct: used, + used_pct: rawUsedPct, timestamp: Math.floor(Date.now() / 1000) }); fs.writeFileSync(bridgePath, bridgeData); diff --git a/tests/bug-2451-context-monitor-over-report.test.cjs b/tests/bug-2451-context-monitor-over-report.test.cjs new file mode 100644 index 00000000..6b666890 --- /dev/null +++ b/tests/bug-2451-context-monitor-over-report.test.cjs @@ -0,0 +1,185 @@ +/** + * Regression test for bug #2451 + * + * The GSD context monitor hook over-reports usage by ~13 percentage points + * compared to Claude Code's native /context command. The root cause: + * + * gsd-statusline.js writes two values to the bridge file: + * - remaining_percentage: raw remaining from CC (e.g. 35%) + * - used_pct: normalized "usable" percentage (e.g. 78%) — accounts for + * the 16.5% autocompact buffer by scaling: (100 - remaining - buffer) / + * (100 - buffer) * 100 + * + * gsd-context-monitor.js displays used_pct (78%) in warning messages. + * But CC's native /context shows raw used = 100 - remaining = 65%. + * The 13-point gap is exactly the buffer normalization overhead. + * + * Fix: the bridge must write used_pct as the raw value (Math.round(100 - + * remaining)), not the buffer-normalized value. The statusline progress bar + * continues to use the normalized value for its own display; only the bridge + * value that feeds the context monitor needs to be raw/CC-consistent. + */ + +'use strict'; + +const { test, describe } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const os = require('node:os'); +const path = require('node:path'); +const { execFileSync } = require('node:child_process'); + +const HOOK_PATH = path.join(__dirname, '..', 'hooks', 'gsd-statusline.js'); +const MONITOR_PATH = path.join(__dirname, '..', 'hooks', 'gsd-context-monitor.js'); + +/** + * Run the statusline hook with a synthetic payload and return the full + * bridge JSON object written to /tmp/claude-ctx-{sessionId}.json. + */ +function runStatuslineHook(remainingPct, totalTokens = 1_000_000, acwEnv = null) { + const sessionId = `test-2451-${Date.now()}-${Math.random().toString(36).slice(2)}`; + const payload = JSON.stringify({ + model: { display_name: 'Claude' }, + workspace: { current_dir: os.tmpdir() }, + session_id: sessionId, + context_window: { + remaining_percentage: remainingPct, + total_tokens: totalTokens, + }, + }); + + const env = { ...process.env }; + if (acwEnv != null) { + env.CLAUDE_CODE_AUTO_COMPACT_WINDOW = String(acwEnv); + } else { + delete env.CLAUDE_CODE_AUTO_COMPACT_WINDOW; + } + + try { + execFileSync(process.execPath, [HOOK_PATH], { + input: payload, + env, + timeout: 4000, + }); + } catch { /* non-zero exit is fine; we only need the bridge file */ } + + const bridgePath = path.join(os.tmpdir(), `claude-ctx-${sessionId}.json`); + const bridge = JSON.parse(fs.readFileSync(bridgePath, 'utf-8')); + fs.unlinkSync(bridgePath); + return bridge; +} + +/** + * Run the context monitor hook with a pre-written bridge file and return + * the parsed additionalContext string from its stdout. + */ +function runMonitorHook(remainingPct, usedPct) { + const sessionId = `test-2451-mon-${Date.now()}-${Math.random().toString(36).slice(2)}`; + const bridgePath = path.join(os.tmpdir(), `claude-ctx-${sessionId}.json`); + fs.writeFileSync(bridgePath, JSON.stringify({ + session_id: sessionId, + remaining_percentage: remainingPct, + used_pct: usedPct, + timestamp: Math.floor(Date.now() / 1000), + })); + + const input = JSON.stringify({ session_id: sessionId, cwd: os.tmpdir() }); + let stdout = ''; + try { + stdout = execFileSync(process.execPath, [MONITOR_PATH], { + input, + encoding: 'utf-8', + timeout: 5000, + }); + } catch (e) { + stdout = e.stdout || ''; + } finally { + try { fs.unlinkSync(bridgePath); } catch { /* noop */ } + try { fs.unlinkSync(path.join(os.tmpdir(), `claude-ctx-${sessionId}-warned.json`)); } catch { /* noop */ } + } + + if (!stdout) return null; + const out = JSON.parse(stdout); + return out?.hookSpecificOutput?.additionalContext || null; +} + +// ─── Bridge file used_pct accuracy ────────────────────────────────────────── + +describe('bug #2451: bridge used_pct matches CC native reporting', () => { + test('used_pct is raw (100 - remaining), not buffer-normalized', () => { + // CC reports remaining_percentage=35 → CC native "used" = 100-35 = 65% + // Buffer-normalized would give: (100 - (35-16.5)/(100-16.5)*100) ≈ 78% + // The bridge used_pct must be 65 (raw), not 78 (normalized). + const bridge = runStatuslineHook(35); + assert.strictEqual( + bridge.used_pct, + 65, + `used_pct should be 65 (raw: 100 - 35) but got ${bridge.used_pct}. ` + + 'Buffer normalization must NOT be applied to the bridge used_pct, ' + + 'otherwise context monitor messages over-report usage by ~13 points ' + + 'compared to CC native /context (root cause of #2451).' + ); + }); + + test('used_pct is raw for high remaining (low usage scenario)', () => { + // remaining=80 → raw used = 20 + const bridge = runStatuslineHook(80); + assert.strictEqual(bridge.used_pct, 20, + `used_pct should be 20 (raw: 100-80) but got ${bridge.used_pct}`); + }); + + test('used_pct is raw for near-critical remaining', () => { + // remaining=20 → raw used = 80 + const bridge = runStatuslineHook(20); + assert.strictEqual(bridge.used_pct, 80, + `used_pct should be 80 (raw: 100-20) but got ${bridge.used_pct}`); + }); + + test('remaining_percentage in bridge matches raw CC value', () => { + // The bridge remaining_percentage should be the exact raw value from CC + const bridge = runStatuslineHook(42); + assert.strictEqual(bridge.remaining_percentage, 42, + 'bridge remaining_percentage must be the raw CC value (no normalization)'); + }); +}); + +// ─── Context monitor message accuracy ─────────────────────────────────────── + +describe('bug #2451: context monitor warning messages show CC-consistent percentages', () => { + test('WARNING message shows raw used_pct consistent with CC reporting', () => { + // remaining=30 → raw used=70; bridge stores used_pct=70 + // Monitor message must say "Usage at 70%", not a buffer-inflated value + const msg = runMonitorHook(30, 70); + assert.ok(msg, 'hook should emit a warning when remaining=30 (below WARNING_THRESHOLD=35)'); + assert.match( + msg, + /Usage at 70%/, + `Warning message should say "Usage at 70%" (raw), got: ${msg}` + ); + }); + + test('CRITICAL message shows raw used_pct consistent with CC reporting', () => { + // remaining=20 → raw used=80 + const msg = runMonitorHook(20, 80); + assert.ok(msg, 'hook should emit a critical warning when remaining=20 (below CRITICAL_THRESHOLD=25)'); + assert.match( + msg, + /Usage at 80%/, + `Critical message should say "Usage at 80%" (raw), got: ${msg}` + ); + }); + + test('gap between hook used_pct and raw CC value is at most 1 (rounding)', () => { + // With the fix, the only acceptable deviation is ±1 due to Math.round + const rawRemaining = 35; + const bridge = runStatuslineHook(rawRemaining); + const ccNativeUsed = 100 - rawRemaining; // 65 + const gap = Math.abs(bridge.used_pct - ccNativeUsed); + assert.ok( + gap <= 1, + `Gap between hook used_pct (${bridge.used_pct}) and CC native used (${ccNativeUsed}) ` + + `is ${gap} points — must be ≤1 (rounding). Larger gaps indicate buffer normalization ` + + 'is still being applied to bridge used_pct (root cause of #2451).' + ); + }); +}); diff --git a/tests/bug-2453-sdk-cli-chmod.test.cjs b/tests/bug-2453-sdk-cli-chmod.test.cjs new file mode 100644 index 00000000..446342d1 --- /dev/null +++ b/tests/bug-2453-sdk-cli-chmod.test.cjs @@ -0,0 +1,88 @@ +/** + * Regression test for bug #2453 + * + * installSdkIfNeeded() builds sdk/dist/cli.js via `tsc` then runs + * `npm install -g .`. TypeScript emits files at process umask (0o644) and + * npm install from a local directory does NOT chmod bin-script targets the + * way tarball extraction does. The result: the globally-installed + * dist/cli.js lands with mode 644 (non-executable), the `gsd-sdk` symlink + * points at a non-executable file, and `command -v gsd-sdk` fails on every + * new install. + * + * Fix: after `npm install -g .`, the installer must explicitly + * `chmodSync(cliPath, 0o755)` on the installed dist/cli.js. This mirrors + * the pattern already used four times in install.js for hook files. + */ + +'use strict'; + +const { test, describe } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); + +const INSTALL_SRC = path.join(__dirname, '..', 'bin', 'install.js'); + +describe('bug #2453: installSdkIfNeeded chmods sdk dist/cli.js to 0o755', () => { + let installSrc; + + test('install.js source exists', () => { + assert.ok(fs.existsSync(INSTALL_SRC), 'bin/install.js must exist'); + installSrc = fs.readFileSync(INSTALL_SRC, 'utf-8'); + }); + + test('installSdkIfNeeded contains a chmodSync call for dist/cli.js', () => { + installSrc = installSrc || fs.readFileSync(INSTALL_SRC, 'utf-8'); + + // Locate the installSdkIfNeeded function body + const fnStart = installSrc.indexOf('function installSdkIfNeeded()'); + assert.ok(fnStart !== -1, 'installSdkIfNeeded function must exist in install.js'); + + // Find the end of the function (next top-level function declaration) + const fnEnd = installSrc.indexOf('\nfunction ', fnStart + 1); + const fnBody = fnEnd !== -1 ? installSrc.slice(fnStart, fnEnd) : installSrc.slice(fnStart); + + // Must chmod dist/cli.js to make it executable after npm install -g . + const hasChmod = fnBody.includes('chmodSync') && fnBody.includes('dist/cli.js'); + assert.ok( + hasChmod, + 'installSdkIfNeeded must call chmodSync on dist/cli.js after npm install -g . ' + + '(tsc emits 644; npm does not chmod bin targets from local dir installs — ' + + 'root cause of #2453: gsd-sdk symlink target is non-executable on first install)' + ); + }); + + test('chmodSync for dist/cli.js uses mode 0o755', () => { + installSrc = installSrc || fs.readFileSync(INSTALL_SRC, 'utf-8'); + + const fnStart = installSrc.indexOf('function installSdkIfNeeded()'); + const fnEnd = installSrc.indexOf('\nfunction ', fnStart + 1); + const fnBody = fnEnd !== -1 ? installSrc.slice(fnStart, fnEnd) : installSrc.slice(fnStart); + + // The chmod call must use 0o755 (executable), not 0o644 + const has755 = fnBody.includes('0o755') && fnBody.includes('dist/cli.js'); + assert.ok( + has755, + 'chmodSync for dist/cli.js must use mode 0o755 to make the binary executable' + ); + }); + + test('chmodSync appears after npm install -g . step', () => { + installSrc = installSrc || fs.readFileSync(INSTALL_SRC, 'utf-8'); + + const fnStart = installSrc.indexOf('function installSdkIfNeeded()'); + const fnEnd = installSrc.indexOf('\nfunction ', fnStart + 1); + const fnBody = fnEnd !== -1 ? installSrc.slice(fnStart, fnEnd) : installSrc.slice(fnStart); + + const npmGlobalIdx = fnBody.indexOf("'install', '-g', '.'"); + const chmodIdx = fnBody.indexOf('chmodSync'); + + assert.ok(npmGlobalIdx !== -1, "npm install -g . step must be present in installSdkIfNeeded"); + assert.ok(chmodIdx !== -1, 'chmodSync must be present in installSdkIfNeeded'); + assert.ok( + chmodIdx > npmGlobalIdx, + 'chmodSync must appear AFTER the npm install -g . step ' + + '(the file to chmod does not exist until npm installs it globally)' + ); + }); +}); diff --git a/tests/gsd-statusline.test.cjs b/tests/gsd-statusline.test.cjs index 93483445..070e2713 100644 --- a/tests/gsd-statusline.test.cjs +++ b/tests/gsd-statusline.test.cjs @@ -255,8 +255,12 @@ describe('context meter respects CLAUDE_CODE_AUTO_COMPACT_WINDOW (#2219)', () => const hookPath = path.join(__dirname, '..', 'hooks', 'gsd-statusline.js'); /** - * Run the statusline hook with a synthetic context_window payload and - * return the used_pct written to the bridge file. + * Run the statusline hook with a synthetic context_window payload. + * Returns { normalizedUsed, rawUsedPct } where: + * - normalizedUsed: the buffer-adjusted % shown in the statusline bar + * (parsed from the hook's stdout ANSI output, e.g. "60%") + * - rawUsedPct: the raw value written to the bridge file (100 - remaining, + * CC-consistent per #2451 fix) */ function runHook(remainingPct, totalTokens, acwEnv) { const sessionId = `test-2219-${Date.now()}-${Math.random().toString(36).slice(2)}`; @@ -277,48 +281,70 @@ describe('context meter respects CLAUDE_CODE_AUTO_COMPACT_WINDOW (#2219)', () => delete env.CLAUDE_CODE_AUTO_COMPACT_WINDOW; } + let stdout = ''; try { - execFileSync(process.execPath, [hookPath], { + stdout = execFileSync(process.execPath, [hookPath], { input: payload, env, + encoding: 'utf8', timeout: 4000, }); } catch (e) { - // Non-zero exit is fine — hook exits 0 on success, but we're reading - // the bridge file, not the exit code. Ignore exit failures here. + stdout = e.stdout || ''; } + // Parse normalized used% from the statusline bar output (e.g. "60%") + // Strip ANSI escape codes then extract the percentage digit(s) before "%" + const clean = stdout.replace(/\x1b\[[0-9;]*m/g, ''); + const match = clean.match(/(\d+)%/); + const normalizedUsed = match ? parseInt(match[1], 10) : null; + + // Read raw used_pct from the bridge file (#2451: bridge stores raw CC value) const bridgePath = path.join(os.tmpdir(), `claude-ctx-${sessionId}.json`); - const bridge = JSON.parse(fs.readFileSync(bridgePath, 'utf8')); - fs.unlinkSync(bridgePath); - return bridge.used_pct; + let rawUsedPct = null; + try { + const bridge = JSON.parse(fs.readFileSync(bridgePath, 'utf8')); + rawUsedPct = bridge.used_pct; + fs.unlinkSync(bridgePath); + } catch { /* bridge may not exist if hook exited early */ } + + return { normalizedUsed, rawUsedPct }; } - test('default buffer (no env var): 50% remaining → ~60% used', () => { + test('default buffer (no env var): 50% remaining → ~60% normalized bar display', () => { // Default 16.5% buffer: usableRemaining = (50 - 16.5) / (100 - 16.5) * 100 ≈ 40.12% - // used ≈ 100 - 40.12 = 59.88 → rounded 60 - const used = runHook(50, 1_000_000, null); - assert.strictEqual(used, 60); + // normalized used ≈ 100 - 40.12 = 59.88 → rounded 60 (shown in statusline bar) + const { normalizedUsed } = runHook(50, 1_000_000, null); + assert.strictEqual(normalizedUsed, 60); }); - test('CLAUDE_CODE_AUTO_COMPACT_WINDOW=400000: 50% remaining → ~83% used', () => { + test('CLAUDE_CODE_AUTO_COMPACT_WINDOW=400000: 50% remaining → ~83% normalized bar display', () => { // With 1M total, 400k window → buffer = 40%. usableRemaining = (50 - 40) / (100 - 40) * 100 ≈ 16.67% - // used ≈ 100 - 16.67 = 83.33 → rounded 83 - const used = runHook(50, 1_000_000, 400_000); - assert.strictEqual(used, 83); + // normalized used ≈ 100 - 16.67 = 83.33 → rounded 83 (shown in statusline bar) + const { normalizedUsed } = runHook(50, 1_000_000, 400_000); + assert.strictEqual(normalizedUsed, 83); }); test('CLAUDE_CODE_AUTO_COMPACT_WINDOW=0 falls back to default buffer', () => { // Explicit "0" means unset — should behave like no env var (16.5% buffer) - const used = runHook(50, 1_000_000, 0); - assert.strictEqual(used, 60); + const { normalizedUsed } = runHook(50, 1_000_000, 0); + assert.strictEqual(normalizedUsed, 60); }); test('buffer capped at 100% when ACW exceeds total context', () => { // Pathological: ACW > totalCtx → buffer = 100%. With no usable range left, // usableRemaining = max(0, (50-100)/(100-100)*100) = max(0, -Inf) = 0, - // so used = 100 (context reported as completely full). - const used = runHook(50, 1_000_000, 2_000_000); - assert.strictEqual(used, 100); + // so normalized used = 100 (context reported as completely full in bar). + const { normalizedUsed } = runHook(50, 1_000_000, 2_000_000); + assert.strictEqual(normalizedUsed, 100); + }); + + test('bridge used_pct is raw (CC-consistent) regardless of ACW setting (#2451)', () => { + // Fix for #2451: bridge used_pct must be raw (100 - remaining), not normalized. + // This ensures gsd-context-monitor warning messages match CC native /context. + // The ACW normalization only affects the statusline bar display, not the bridge. + const { rawUsedPct } = runHook(50, 1_000_000, 400_000); + assert.strictEqual(rawUsedPct, 50, + 'bridge used_pct must be raw (100-50=50) regardless of CLAUDE_CODE_AUTO_COMPACT_WINDOW'); }); });