From 6a94bb6afb8d8da580a77c6a5a829aa9f2cefd3f Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Tue, 26 May 2026 11:57:39 -0400 Subject: [PATCH] refactor(#287): deepen active workstream pointer store seam (#288) --- .../bin/lib/active-workstream-store.cjs | 236 +++++++++++++++++- get-shit-done/bin/lib/planning-workspace.cjs | 224 ++--------------- tests/active-workstream-store.test.cjs | 56 ++++- 3 files changed, 299 insertions(+), 217 deletions(-) diff --git a/get-shit-done/bin/lib/active-workstream-store.cjs b/get-shit-done/bin/lib/active-workstream-store.cjs index 5bfe6192f..6ce0b7628 100644 --- a/get-shit-done/bin/lib/active-workstream-store.cjs +++ b/get-shit-done/bin/lib/active-workstream-store.cjs @@ -1,15 +1,225 @@ /** * Active Workstream Pointer Store Module * - * Owns workstream source precedence and selection: + * Owns active workstream source precedence, session identity, and pointer IO: * CLI --ws > GSD_WORKSTREAM env > stored active workstream pointer. */ -const { getActiveWorkstream } = require('./planning-workspace.cjs'); -const { - validateWorkstreamName, - assertValidActiveWorkstreamName, -} = require('./workstream-name-policy.cjs'); +const fs = require('fs'); +const os = require('os'); +const path = require('path'); +const crypto = require('crypto'); +const { probeTty, platformWriteSync, platformReadSync, platformEnsureDir } = require('./shell-command-projection.cjs'); +const { isValidActiveWorkstreamName } = require('./workstream-name-policy.cjs'); + +const WORKSTREAM_SESSION_ENV_KEYS = [ + 'GSD_SESSION_KEY', + 'CODEX_THREAD_ID', + 'CLAUDE_SESSION_ID', + 'CLAUDE_CODE_SSE_PORT', + 'OPENCODE_SESSION_ID', + 'GEMINI_SESSION_ID', + 'CURSOR_SESSION_ID', + 'WINDSURF_SESSION_ID', + 'TERM_SESSION_ID', + 'WT_SESSION', + 'TMUX_PANE', + 'ZELLIJ_SESSION_NAME', +]; + +let cachedControllingTtyToken = null; +let didProbeControllingTtyToken = false; + +function planningRoot(cwd) { + return path.join(cwd, '.planning'); +} + +function validateWorkstreamName(name) { + return isValidActiveWorkstreamName(name); +} + +function sanitizeWorkstreamSessionToken(value) { + if (value === null || value === undefined) return null; + const token = String(value).trim().replace(/[^a-zA-Z0-9._-]+/g, '_').replace(/^_+|_+$/g, ''); + return token ? token.slice(0, 160) : null; +} + +function probeControllingTtyToken() { + if (didProbeControllingTtyToken) return cachedControllingTtyToken; + didProbeControllingTtyToken = true; + + if (!(process.stdin && process.stdin.isTTY)) { + return cachedControllingTtyToken; + } + + const ttyPath = probeTty(); + if (ttyPath) { + const token = sanitizeWorkstreamSessionToken(ttyPath.replace(/^\/dev\//, '')); + if (token) cachedControllingTtyToken = `tty-${token}`; + } + + return cachedControllingTtyToken; +} + +function getControllingTtyToken() { + for (const envKey of ['TTY', 'SSH_TTY']) { + const token = sanitizeWorkstreamSessionToken(process.env[envKey]); + if (token) return `tty-${token.replace(/^dev_/, '')}`; + } + + return probeControllingTtyToken(); +} + +function getWorkstreamSessionKey() { + for (const envKey of WORKSTREAM_SESSION_ENV_KEYS) { + const raw = process.env[envKey]; + const token = sanitizeWorkstreamSessionToken(raw); + if (token) return `${envKey.toLowerCase().replace(/[^a-z0-9]+/g, '-')}-${token}`; + } + + return getControllingTtyToken(); +} + +function getSessionScopedWorkstreamFile(cwd, fixedSessionKey) { + const sessionKey = fixedSessionKey || getWorkstreamSessionKey(); + if (!sessionKey) return null; + + let planningAbs; + try { + planningAbs = fs.realpathSync.native(planningRoot(cwd)); + } catch { + planningAbs = path.resolve(planningRoot(cwd)); + } + const projectId = crypto + .createHash('sha1') + .update(planningAbs) + .digest('hex') + .slice(0, 16); + + const dirPath = path.join(os.tmpdir(), 'gsd-workstream-sessions', projectId); + return { + sessionKey, + dirPath, + filePath: path.join(dirPath, sessionKey), + }; +} + +function createSharedPointerAdapter(cwd) { + const filePath = path.join(planningRoot(cwd), 'active-workstream'); + return { + read() { + const raw = platformReadSync(filePath); + return raw ? raw.trim() || null : null; + }, + write(name) { + platformWriteSync(filePath, name + '\n'); + }, + clear() { + try { fs.unlinkSync(filePath); } catch {} + }, + }; +} + +function createSessionScopedPointerAdapter(cwd, fixedSessionKey) { + const scoped = getSessionScopedWorkstreamFile(cwd, fixedSessionKey); + if (!scoped) return null; + + return { + read() { + const raw = platformReadSync(scoped.filePath); + return raw ? raw.trim() || null : null; + }, + write(name) { + platformEnsureDir(scoped.dirPath); + platformWriteSync(scoped.filePath, name + '\n'); + }, + clear() { + try { fs.unlinkSync(scoped.filePath); } catch {} + try { + const remaining = fs.readdirSync(scoped.dirPath); + if (remaining.length === 0) { + fs.rmdirSync(scoped.dirPath); + } + } catch {} + }, + }; +} + +function createMemoryPointerAdapter(initialName = null) { + let value = initialName; + return { + read() { + return value; + }, + write(name) { + value = name; + }, + clear() { + value = null; + }, + }; +} + +function pickActiveWorkstreamAdapter(cwd, opts = {}) { + if (opts.activeWorkstreamAdapter) { + return opts.activeWorkstreamAdapter; + } + + const sessionKey = getWorkstreamSessionKey(); + if (sessionKey) { + if (opts.activeWorkstreamAdapters && opts.activeWorkstreamAdapters.session) { + return opts.activeWorkstreamAdapters.session; + } + return createSessionScopedPointerAdapter(cwd, sessionKey); + } + + if (opts.activeWorkstreamAdapters && opts.activeWorkstreamAdapters.shared) { + return opts.activeWorkstreamAdapters.shared; + } + return createSharedPointerAdapter(cwd); +} + +function getActiveWorkstream(cwd, opts = {}) { + const adapter = pickActiveWorkstreamAdapter(cwd, opts); + if (!adapter) return null; + + const name = adapter.read(); + if (!name || !validateWorkstreamName(name)) { + adapter.clear(); + return null; + } + + const wsDir = path.join(planningRoot(cwd), 'workstreams', name); + if (!fs.existsSync(wsDir)) { + adapter.clear(); + return null; + } + + return name; +} + +function setActiveWorkstream(cwd, name, opts = {}) { + const adapter = pickActiveWorkstreamAdapter(cwd, opts); + if (!adapter) return; + + if (!name) { + adapter.clear(); + return; + } + if (!validateWorkstreamName(name)) { + throw new Error('Invalid workstream name: must be alphanumeric, hyphens, underscores, or dots'); + } + + const wsDir = path.join(planningRoot(cwd), 'workstreams', name); + platformEnsureDir(wsDir); + adapter.write(name); +} + +function clearActiveWorkstream(cwd, opts = {}) { + const adapter = pickActiveWorkstreamAdapter(cwd, opts); + if (!adapter) return; + adapter.clear(); +} function parseCliWorkstream(args) { const wsEqArg = args.find(arg => arg.startsWith('--ws=')); @@ -44,7 +254,7 @@ function parseCliWorkstream(args) { function resolveActiveWorkstream(cwd, args, env = process.env, deps = {}) { const parsed = parseCliWorkstream(args); - const getStored = deps.getStored || getActiveWorkstream; + const getStored = deps.getStored || ((dir) => getActiveWorkstream(dir, deps)); let ws = null; let source = 'none'; @@ -60,8 +270,8 @@ function resolveActiveWorkstream(cwd, args, env = process.env, deps = {}) { source = ws ? 'store' : 'none'; } - if (ws) { - assertValidActiveWorkstreamName(ws); + if (ws && !validateWorkstreamName(ws)) { + throw new Error('Invalid workstream name: must be alphanumeric, hyphens, underscores, or dots'); } return { @@ -78,6 +288,14 @@ function applyResolvedWorkstreamEnv(resolution, env = process.env) { module.exports = { validateWorkstreamName, + getWorkstreamSessionKey, + createSharedPointerAdapter, + createSessionScopedPointerAdapter, + createMemoryPointerAdapter, + pickActiveWorkstreamAdapter, + getActiveWorkstream, + setActiveWorkstream, + clearActiveWorkstream, parseCliWorkstream, resolveActiveWorkstream, applyResolvedWorkstreamEnv, diff --git a/get-shit-done/bin/lib/planning-workspace.cjs b/get-shit-done/bin/lib/planning-workspace.cjs index e24ac5b74..9c673be96 100644 --- a/get-shit-done/bin/lib/planning-workspace.cjs +++ b/get-shit-done/bin/lib/planning-workspace.cjs @@ -3,37 +3,23 @@ * * This module owns the planning workspace seam: * - planningDir/planningRoot/planningPaths - * - active workstream pointer policy (session-scoped > shared) - * - pointer storage adapters (session/shared/memory) + * - planning lock semantics + * + * Active workstream pointer policy/session identity lives in + * active-workstream-store.cjs and is consumed here via thin adapters. */ const fs = require('fs'); -const os = require('os'); const path = require('path'); -const crypto = require('crypto'); -const { probeTty, platformWriteSync, platformReadSync, platformEnsureDir } = require('./shell-command-projection.cjs'); +const { platformEnsureDir } = require('./shell-command-projection.cjs'); const { - validateWorkstreamName, - assertValidActiveWorkstreamName, -} = require('./workstream-name-policy.cjs'); - -const WORKSTREAM_SESSION_ENV_KEYS = [ - 'GSD_SESSION_KEY', - 'CODEX_THREAD_ID', - 'CLAUDE_SESSION_ID', - 'CLAUDE_CODE_SSE_PORT', - 'OPENCODE_SESSION_ID', - 'GEMINI_SESSION_ID', - 'CURSOR_SESSION_ID', - 'WINDSURF_SESSION_ID', - 'TERM_SESSION_ID', - 'WT_SESSION', - 'TMUX_PANE', - 'ZELLIJ_SESSION_NAME', -]; - -let cachedControllingTtyToken = null; -let didProbeControllingTtyToken = false; + createSharedPointerAdapter, + createSessionScopedPointerAdapter, + createMemoryPointerAdapter, + getActiveWorkstream: getStoredActiveWorkstream, + setActiveWorkstream: setStoredActiveWorkstream, + clearActiveWorkstream: clearStoredActiveWorkstream, +} = require('./active-workstream-store.cjs'); // Track .planning/.lock files held by this process so they can be removed on exit. const _heldPlanningLocks = new Set(); @@ -96,154 +82,6 @@ function planningPaths(cwd, ws) { }; } -function sanitizeWorkstreamSessionToken(value) { - if (value === null || value === undefined) return null; - const token = String(value).trim().replace(/[^a-zA-Z0-9._-]+/g, '_').replace(/^_+|_+$/g, ''); - return token ? token.slice(0, 160) : null; -} - -function probeControllingTtyToken() { - if (didProbeControllingTtyToken) return cachedControllingTtyToken; - didProbeControllingTtyToken = true; - - // `tty` reads stdin. When stdin is already non-interactive, spawning it only - // adds avoidable failures on the routing hot path and cannot reveal a stable token. - if (!(process.stdin && process.stdin.isTTY)) { - return cachedControllingTtyToken; - } - - const ttyPath = probeTty(); - if (ttyPath) { - const token = sanitizeWorkstreamSessionToken(ttyPath.replace(/^\/dev\//, '')); - if (token) cachedControllingTtyToken = `tty-${token}`; - } - - return cachedControllingTtyToken; -} - -function getControllingTtyToken() { - for (const envKey of ['TTY', 'SSH_TTY']) { - const token = sanitizeWorkstreamSessionToken(process.env[envKey]); - if (token) return `tty-${token.replace(/^dev_/, '')}`; - } - - return probeControllingTtyToken(); -} - -function getWorkstreamSessionKey() { - for (const envKey of WORKSTREAM_SESSION_ENV_KEYS) { - const raw = process.env[envKey]; - const token = sanitizeWorkstreamSessionToken(raw); - if (token) return `${envKey.toLowerCase().replace(/[^a-z0-9]+/g, '-')}-${token}`; - } - - return getControllingTtyToken(); -} - -function getSessionScopedWorkstreamFile(cwd, fixedSessionKey) { - const sessionKey = fixedSessionKey || getWorkstreamSessionKey(); - if (!sessionKey) return null; - - // Use realpathSync.native so the hash is derived from the canonical filesystem - // path. On Windows, path.resolve returns whatever case the caller supplied, - // while realpathSync.native returns the case the OS recorded — they differ on - // case-insensitive NTFS, producing different hashes and different tmpdir slots. - // Fall back to path.resolve when the directory does not yet exist. - let planningAbs; - try { - planningAbs = fs.realpathSync.native(planningRoot(cwd)); - } catch { - planningAbs = path.resolve(planningRoot(cwd)); - } - const projectId = crypto - .createHash('sha1') - .update(planningAbs) - .digest('hex') - .slice(0, 16); - - const dirPath = path.join(os.tmpdir(), 'gsd-workstream-sessions', projectId); - return { - sessionKey, - dirPath, - filePath: path.join(dirPath, sessionKey), - }; -} - -function createSharedPointerAdapter(cwd) { - const filePath = path.join(planningRoot(cwd), 'active-workstream'); - return { - read() { - const raw = platformReadSync(filePath); - return raw ? raw.trim() || null : null; - }, - write(name) { - platformWriteSync(filePath, name + '\n'); - }, - clear() { - try { fs.unlinkSync(filePath); } catch {} - }, - }; -} - -function createSessionScopedPointerAdapter(cwd, fixedSessionKey) { - const scoped = getSessionScopedWorkstreamFile(cwd, fixedSessionKey); - if (!scoped) return null; - - return { - read() { - const raw = platformReadSync(scoped.filePath); - return raw ? raw.trim() || null : null; - }, - write(name) { - platformEnsureDir(scoped.dirPath); - platformWriteSync(scoped.filePath, name + '\n'); - }, - clear() { - try { fs.unlinkSync(scoped.filePath); } catch {} - try { - const remaining = fs.readdirSync(scoped.dirPath); - if (remaining.length === 0) { - fs.rmdirSync(scoped.dirPath); - } - } catch {} - }, - }; -} - -function createMemoryPointerAdapter(initialName = null) { - let value = initialName; - return { - read() { - return value; - }, - write(name) { - value = name; - }, - clear() { - value = null; - }, - }; -} - -function pickActiveWorkstreamAdapter(cwd, opts = {}) { - if (opts.activeWorkstreamAdapter) { - return opts.activeWorkstreamAdapter; - } - - const sessionKey = getWorkstreamSessionKey(); - if (sessionKey) { - if (opts.activeWorkstreamAdapters && opts.activeWorkstreamAdapters.session) { - return opts.activeWorkstreamAdapters.session; - } - return createSessionScopedPointerAdapter(cwd, sessionKey); - } - - if (opts.activeWorkstreamAdapters && opts.activeWorkstreamAdapters.shared) { - return opts.activeWorkstreamAdapters.shared; - } - return createSharedPointerAdapter(cwd); -} - function withPlanningLock(cwd, fn) { const lockPath = path.join(planningDir(cwd), '.lock'); const lockTimeout = 10000; // 10 seconds @@ -320,52 +158,24 @@ function createPlanningWorkspace(cwd, opts = {}) { }, activeWorkstream: { get() { - const adapter = pickActiveWorkstreamAdapter(cwd, opts); - if (!adapter) return null; - - const name = adapter.read(); - if (!name || !validateWorkstreamName(name)) { - adapter.clear(); - return null; - } - - const wsDir = path.join(planningRoot(cwd), 'workstreams', name); - if (!fs.existsSync(wsDir)) { - adapter.clear(); - return null; - } - - return name; + return getStoredActiveWorkstream(cwd, opts); }, set(name) { - const adapter = pickActiveWorkstreamAdapter(cwd, opts); - if (!adapter) return; - - if (!name) { - adapter.clear(); - return; - } - assertValidActiveWorkstreamName(name); - - const wsDir = path.join(planningRoot(cwd), 'workstreams', name); - platformEnsureDir(wsDir); - adapter.write(name); + setStoredActiveWorkstream(cwd, name, opts); }, clear() { - const adapter = pickActiveWorkstreamAdapter(cwd, opts); - if (!adapter) return; - adapter.clear(); + clearStoredActiveWorkstream(cwd, opts); }, }, }; } function getActiveWorkstream(cwd) { - return createPlanningWorkspace(cwd).activeWorkstream.get(); + return getStoredActiveWorkstream(cwd); } function setActiveWorkstream(cwd, name) { - createPlanningWorkspace(cwd).activeWorkstream.set(name); + setStoredActiveWorkstream(cwd, name); } /** diff --git a/tests/active-workstream-store.test.cjs b/tests/active-workstream-store.test.cjs index 805c01a7b..058263959 100644 --- a/tests/active-workstream-store.test.cjs +++ b/tests/active-workstream-store.test.cjs @@ -1,11 +1,17 @@ const { describe, test } = require('node:test'); const assert = require('node:assert/strict'); +const fs = require('fs'); +const os = require('os'); +const path = require('path'); const { validateWorkstreamName, parseCliWorkstream, resolveActiveWorkstream, applyResolvedWorkstreamEnv, + createMemoryPointerAdapter, + getActiveWorkstream, + setActiveWorkstream, } = require('../get-shit-done/bin/lib/active-workstream-store.cjs'); describe('active-workstream-store', () => { @@ -93,5 +99,53 @@ describe('active-workstream-store', () => { applyResolvedWorkstreamEnv({ ws: 'new-ws' }, env); assert.equal(env.GSD_WORKSTREAM, 'new-ws'); }); -}); + test('getActiveWorkstream uses session adapter over shared adapter when session key exists', () => { + const savedSession = process.env.GSD_SESSION_KEY; + process.env.GSD_SESSION_KEY = 'session-123'; + + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-active-store-precedence-')); + try { + fs.mkdirSync(path.join(tmpDir, '.planning', 'workstreams', 'session-ws'), { recursive: true }); + fs.mkdirSync(path.join(tmpDir, '.planning', 'workstreams', 'shared-ws'), { recursive: true }); + + const session = createMemoryPointerAdapter('session-ws'); + const shared = createMemoryPointerAdapter('shared-ws'); + + const active = getActiveWorkstream(tmpDir, { + activeWorkstreamAdapters: { session, shared }, + }); + assert.equal(active, 'session-ws'); + } finally { + if (savedSession !== undefined) process.env.GSD_SESSION_KEY = savedSession; + else delete process.env.GSD_SESSION_KEY; + fs.rmSync(tmpDir, { recursive: true, force: true }); + } + }); + + test('getActiveWorkstream self-heals invalid pointer names', () => { + const adapter = createMemoryPointerAdapter('bad/name'); + const active = getActiveWorkstream('/fake/repo', { activeWorkstreamAdapter: adapter }); + assert.equal(active, null); + assert.equal(adapter.read(), null); + }); + + test('getActiveWorkstream self-heals stale pointers', () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-active-store-stale-')); + try { + fs.mkdirSync(path.join(tmpDir, '.planning', 'workstreams'), { recursive: true }); + const adapter = createMemoryPointerAdapter('ghost'); + const active = getActiveWorkstream(tmpDir, { activeWorkstreamAdapter: adapter }); + assert.equal(active, null); + assert.equal(adapter.read(), null); + } finally { + fs.rmSync(tmpDir, { recursive: true, force: true }); + } + }); + + test('setActiveWorkstream clears pointer on null value', () => { + const adapter = createMemoryPointerAdapter('alpha'); + setActiveWorkstream('/fake/repo', null, { activeWorkstreamAdapter: adapter }); + assert.equal(adapter.read(), null); + }); +});