From cc4832088c5fd8a65fa8daf66e5e53b752dc934e Mon Sep 17 00:00:00 2001 From: Duc Nguyen Date: Wed, 17 Jun 2026 10:14:57 +0700 Subject: [PATCH 1/2] feat(hooks): feature-first .workbench resolver + injection (gated) Phases 0-2 of the feature-first restructure, all gated on paths.layout (default type-first, byte-identical). Fix resolvePlanPath (main-worktree anchor, exact slug match, refuse ambiguity); add layout/schemaVersion/ticketPrefixes config + getMainWorktreeConfig; add identity resolver (extractTicketFromBranch, resolveFeatureId, resolveFeatureRoot, get{Global,Archive}Path) routed through path getters; inject VD_FEATURE/GLOBAL/ARCHIVE_PATH + Feature: line. Tests: resolveplanpath 7, featurefirst 23, contract 2, inject 11; parity 19/19. --- hooks/dev-rules-reminder.cjs | 8 +- hooks/lib/config.cjs | 36 +++- hooks/lib/paths.cjs | 202 +++++++++++++++++--- hooks/session-init.cjs | 18 +- hooks/subagent-init.cjs | 8 +- internal/hooks/feature-path-contract.mjs | 50 +++++ internal/hooks/featurefirst_inject_test.mjs | 61 ++++++ internal/hooks/featurefirst_test.mjs | 91 +++++++++ internal/hooks/resolveplanpath_test.mjs | 65 +++++++ 9 files changed, 503 insertions(+), 36 deletions(-) create mode 100644 internal/hooks/feature-path-contract.mjs create mode 100644 internal/hooks/featurefirst_inject_test.mjs create mode 100644 internal/hooks/featurefirst_test.mjs create mode 100644 internal/hooks/resolveplanpath_test.mjs diff --git a/hooks/dev-rules-reminder.cjs b/hooks/dev-rules-reminder.cjs index 9edae3f..8b5fdb3 100644 --- a/hooks/dev-rules-reminder.cjs +++ b/hooks/dev-rules-reminder.cjs @@ -26,7 +26,8 @@ try { resolveNamingPattern, resolvePlanPath, getGitBranch, - resolveSkillsVenv + resolveSkillsVenv, + resolveFeatureRoot } = require('./lib/paths.cjs'); const { readSessionState } = require('./lib/state.cjs'); @@ -42,7 +43,7 @@ try { const gitBranch = getGitBranch(baseDir); const namePattern = resolveNamingPattern(config.plan, gitBranch); - const resolved = resolvePlanPath(sessionId, config, readSessionState); + const resolved = resolvePlanPath(sessionId, config, readSessionState, baseDir); const reportsPath = getReportsPath(resolved.path, resolved.resolvedBy, config.plan, config.paths, baseDir, config); const plansPath = getPlansPath(baseDir, config); const docsPath = getDocsPath(baseDir, config); @@ -51,6 +52,8 @@ try { const visualsPath = umbrellaVal ? getVisualsPath(baseDir, config) : null; const journalsPath = umbrellaVal ? getJournalsPath(baseDir, config) : null; const statePath = umbrellaVal ? getStatePath(baseDir, config) : null; + const featureFirst = !!umbrellaVal && (config.paths?.layout === 'feature-first'); + const featureRoot = featureFirst ? resolveFeatureRoot(config, baseDir) : null; const skillsVenv = resolveSkillsVenv(baseDir); @@ -62,6 +65,7 @@ try { } else { lines.push(`Reports: ${reportsPath}/ | Plans: ${plansPath}/ | Docs: ${docsPath}/`); } + if (featureFirst) lines.push(`Feature: ${featureRoot}/`); lines.push(''); lines.push('## Naming'); diff --git a/hooks/lib/config.cjs b/hooks/lib/config.cjs index 9efd7a6..282bba4 100644 --- a/hooks/lib/config.cjs +++ b/hooks/lib/config.cjs @@ -12,12 +12,15 @@ const fs = require('fs'); const path = require('path'); const os = require('os'); const { execFileSync } = require('child_process'); +const { getMainWorktreeRoot } = require('./paths.cjs'); const DEFAULT_CONFIG = { + schemaVersion: 2, plan: { namingFormat: '{date}-{issue}-{slug}', dateFormat: 'YYMMDD-HHmm', issuePrefix: null, + ticketPrefixes: ['ELT', 'GH', 'PROJ'], reportsDir: 'reports', resolution: { order: ['session', 'branch'], @@ -36,6 +39,9 @@ const DEFAULT_CONFIG = { // Umbrella: null = legacy CWD-anchored layout. // Set to a relative name (e.g. ".workbench") in /.vd.json to opt in. umbrella: null, + // Layout: 'type-first' (flat type siblings) | 'feature-first' (per-feature folders). + // Default 'type-first' → byte-identical to legacy; opt in per-repo via .vd.json. + layout: 'type-first', visuals: 'visuals', journals: 'journals', state: 'state' @@ -146,7 +152,28 @@ function assertMigrated(vdPath, ckPath) { } /** - * Load config: DEFAULT ← global (~/.claude/.vd.json) ← project (/.vd.json). + * Read the MAIN worktree's .vd.json (or null). Layout-determining keys (umbrella, + * layout) come from here so linked worktrees can't disagree about the artifact layout. + */ +function getMainWorktreeConfig(cwd) { + const mainRoot = getMainWorktreeRoot(cwd); + if (!mainRoot) return null; + return readJson(path.join(mainRoot, '.vd.json')); +} + +/** Overlay the repo-wide layout keys (umbrella, layout) from the main worktree config. */ +function applyMainWorktreeLayout(merged, mainCfg) { + if (!mainCfg || !mainCfg.paths) return merged; + const out = Object.assign({}, merged); + out.paths = Object.assign({}, merged.paths); + if (typeof mainCfg.paths.umbrella === 'string') out.paths.umbrella = mainCfg.paths.umbrella; + if (typeof mainCfg.paths.layout === 'string') out.paths.layout = mainCfg.paths.layout; + return out; +} + +/** + * Load config: DEFAULT ← global (~/.claude/.vd.json) ← project (/.vd.json), + * then overlay layout+umbrella from the MAIN worktree (repo-wide artifact-layout keys). * No .ck.json fallback — a lingering legacy file raises a migration error. * Falls back to defaults on any error. */ @@ -162,12 +189,11 @@ function loadConfig() { const globalCfg = readJson(globalPath); const localCfg = localPath ? readJson(localPath) : null; - if (!globalCfg && !localCfg) return buildResult(layerConfigs({}, DEFAULT_CONFIG), gitRoot); - try { let merged = layerConfigs({}, DEFAULT_CONFIG); if (globalCfg) merged = layerConfigs(merged, globalCfg); if (localCfg) merged = layerConfigs(merged, localCfg); + merged = applyMainWorktreeLayout(merged, getMainWorktreeConfig(process.cwd())); return buildResult(merged, gitRoot); } catch { return buildResult(layerConfigs({}, DEFAULT_CONFIG), gitRoot); @@ -180,11 +206,13 @@ function buildResult(merged, gitRoot) { const umbrella = sanitizeUmbrella(rawPaths.umbrella, gitRoot || null); return { + schemaVersion: merged.schemaVersion ?? DEFAULT_CONFIG.schemaVersion, plan: merged.plan || DEFAULT_CONFIG.plan, paths: { docs: rawPaths.docs || DEFAULT_CONFIG.paths.docs, plans: rawPaths.plans || DEFAULT_CONFIG.paths.plans, umbrella, + layout: rawPaths.layout === 'feature-first' ? 'feature-first' : 'type-first', visuals: rawPaths.visuals || DEFAULT_CONFIG.paths.visuals, journals: rawPaths.journals || DEFAULT_CONFIG.paths.journals, state: rawPaths.state || DEFAULT_CONFIG.paths.state @@ -201,4 +229,4 @@ function buildResult(merged, gitRoot) { }; } -module.exports = { DEFAULT_CONFIG, layerConfigs, loadConfig, getGitRoot, sanitizeUmbrella, assertMigrated }; +module.exports = { DEFAULT_CONFIG, layerConfigs, loadConfig, getGitRoot, sanitizeUmbrella, assertMigrated, getMainWorktreeConfig, applyMainWorktreeLayout }; diff --git a/hooks/lib/paths.cjs b/hooks/lib/paths.cjs index f4e985e..90af159 100644 --- a/hooks/lib/paths.cjs +++ b/hooks/lib/paths.cjs @@ -101,9 +101,9 @@ function resolveUmbrellaRoot(config, baseDir) { * Umbrella-off: /plans (legacy, byte-identical) */ function getPlansPath(baseDir, config) { - const umbrellaRoot = resolveUmbrellaRoot(config, baseDir); - if (umbrellaRoot) { - return path.join(umbrellaRoot, stripTrailing(config.paths?.plans) || 'plans'); + const featureRoot = resolveFeatureRoot(config, baseDir); // == umbrellaRoot unless feature-first + if (featureRoot) { + return path.join(featureRoot, stripTrailing(config.paths?.plans) || 'plans'); } // Legacy: second arg was pathsConfig in P2 — accept both shapes const pathsConfig = config?.paths ? config.paths : config; @@ -119,26 +119,26 @@ function getDocsPath(baseDir, config) { } function getVisualsPath(baseDir, config) { - const umbrellaRoot = resolveUmbrellaRoot(config, baseDir); + const featureRoot = resolveFeatureRoot(config, baseDir); const name = stripTrailing(config?.paths?.visuals) || 'visuals'; - return umbrellaRoot - ? path.join(umbrellaRoot, name) + return featureRoot + ? path.join(featureRoot, name) : path.join(baseDir, 'plans', name); // legacy fallback mirrors plans/visuals } function getJournalsPath(baseDir, config) { - const umbrellaRoot = resolveUmbrellaRoot(config, baseDir); + const featureRoot = resolveFeatureRoot(config, baseDir); const name = stripTrailing(config?.paths?.journals) || 'journals'; - return umbrellaRoot - ? path.join(umbrellaRoot, name) + return featureRoot + ? path.join(featureRoot, name) : path.join(baseDir, 'plans', name); // legacy fallback: plans/journals } function getStatePath(baseDir, config) { - const umbrellaRoot = resolveUmbrellaRoot(config, baseDir); + const featureRoot = resolveFeatureRoot(config, baseDir); const name = stripTrailing(config?.paths?.state) || 'state'; - return umbrellaRoot - ? path.join(umbrellaRoot, name) + return featureRoot + ? path.join(featureRoot, name) : path.join(baseDir, 'plans', 'goals'); // legacy fallback: plans/goals } @@ -157,6 +157,15 @@ function getStatePath(baseDir, config) { function getReportsPath(planPath, resolvedBy, planConfig, pathsConfig, anchor, config) { const subdir = stripTrailing(planConfig?.reportsDir) || 'reports'; + // Feature-first: reports nest in the FEATURE dir (not the plan subdir) — kills the split-brain. + if (config && config.paths?.layout === 'feature-first') { + const featureRoot = resolveFeatureRoot(config, anchor || process.cwd()); + if (featureRoot) { + if (!anchor) return `${featureRoot}/${subdir}/`; + return path.join(featureRoot, subdir); + } + } + // Session-active plan overrides everything const activePlan = (planPath && resolvedBy === 'session') ? stripTrailing(planPath) : null; @@ -288,8 +297,13 @@ function slugFromBranch(branch, pattern) { * `readState` is injected to avoid circular deps and enable testing. * Returns { path, resolvedBy } where resolvedBy is 'session'|'branch'|null. */ -function resolvePlanPath(sessionId, config, readState) { - const plansDir = stripTrailing(config?.paths?.plans) || 'plans'; +/** Strip a leading `YYYYMMDD-HHMM-` or `YYMMDD-HHMM-` date prefix from a plan dir name. */ +function planDirSlug(name) { + return name.replace(/^\d{6,8}-\d{4}-/, ''); +} + +function resolvePlanPath(sessionId, config, readState, baseDir) { + baseDir = baseDir || process.cwd(); const resolution = config?.plan?.resolution || {}; const order = resolution.order || ['session', 'branch']; @@ -305,18 +319,24 @@ function resolvePlanPath(sessionId, config, readState) { } } else if (step === 'branch') { try { - const branch = getGitBranch(); + const branch = getGitBranch(baseDir); const slug = slugFromBranch(branch, resolution.branchPattern); - if (slug && fs.existsSync(plansDir)) { - const matches = fs.readdirSync(plansDir, { withFileTypes: true }) - .filter(e => e.isDirectory() && e.name.includes(slug)); - if (matches.length > 0) { - return { - path: path.join(plansDir, matches[matches.length - 1].name), - resolvedBy: 'branch' - }; - } - } + if (!slug) continue; + // Anchor to the umbrella/main-worktree plans dir — the old cwd-relative + // `plansDir` silently no-op'd inside linked worktrees. + const plansDir = getPlansPath(baseDir, config); + if (!fs.existsSync(plansDir)) continue; + const dirs = fs.readdirSync(plansDir, { withFileTypes: true }) + .filter(e => e.isDirectory()); + // Prefer an EXACT slug match; fall back to substring only when unambiguous. + // On >1 candidate, REFUSE — the old `matches[last]` silently mis-converged. + const exact = dirs.filter(e => planDirSlug(e.name) === slug); + const substr = dirs.filter(e => e.name.includes(slug)); + const pick = exact.length === 1 ? exact[0] + : exact.length > 1 ? null + : substr.length === 1 ? substr[0] + : null; + if (pick) return { path: path.join(plansDir, pick.name), resolvedBy: 'branch' }; } catch { /* ignore fs errors */ } } } @@ -329,6 +349,130 @@ function extractTaskListId(resolved) { return path.basename(resolved.path); } +// ── feature-first resolution (gated on config.paths.layout === 'feature-first') ── + +/** Prefix-preserving ticket extractor: `feat/ELT-3316-x` → `ELT-3316`; `gh3251` → `GH-3251`. */ +function extractTicketFromBranch(branch, prefixes) { + if (!branch) return null; + const list = (Array.isArray(prefixes) && prefixes.length) ? prefixes : ['ELT', 'GH', 'PROJ']; + const re = new RegExp(`\\b(${list.join('|')})-?(\\d+)\\b`, 'i'); + const m = branch.match(re); + return m ? `${m[1].toUpperCase()}-${m[2]}` : null; +} + +/** Feature id from `{ticket}-{slug}` or `{slug}` (lowercased, cleaned). + * Strips a leading duplicate ticket from slug so `feat/ELT-3316-manual-upload` + * → `elt-3316-manual-upload`, not `elt-3316-elt-3316-manual-upload`. */ +function computeFeatureId(ticket, slug) { + if (ticket) { + const [pre, num] = ticket.split('-'); + const desc = slug ? slug.replace(new RegExp(`^${pre}-?${num}-?`, 'i'), '') : ''; + return cleanSlug((desc ? `${ticket}-${desc}` : ticket).toLowerCase()); + } + if (slug) return cleanSlug(slug); + return null; +} + +/** Scan features//feature.json for field===value; return the id, or null (refuse on >1). */ +function findFeatureBy(featuresDir, field, value) { + let dirs; + try { dirs = fs.readdirSync(featuresDir, { withFileTypes: true }).filter(e => e.isDirectory()); } + catch { return null; } + const hits = []; + for (const d of dirs) { + try { + const meta = JSON.parse(fs.readFileSync(path.join(featuresDir, d.name, 'feature.json'), 'utf8')); + if (meta && meta[field] === value) hits.push(d.name); + } catch { /* missing/invalid feature.json — skip */ } + } + return hits.length === 1 ? hits[0] : null; +} + +/** Create features//feature.json if absent. Idempotent, atomic (rename), best-effort. */ +function ensureFeatureMeta(featuresDir, id, meta) { + const dir = path.join(featuresDir, id); + const metaPath = path.join(dir, 'feature.json'); + if (fs.existsSync(metaPath)) return; + try { + fs.mkdirSync(dir, { recursive: true }); + const tmp = `${metaPath}.${process.pid}.${Math.random().toString(36).slice(2)}.tmp`; + fs.writeFileSync(tmp, JSON.stringify(meta, null, 2)); + fs.renameSync(tmp, metaPath); // atomic commit; first rename wins (created/branches may differ on a race) + } catch { /* never block resolution on a write failure */ } +} + +const _featureIdCache = new Map(); + +/** + * Resolve the feature id for the current context. Pure read except a one-time idempotent + * feature.json create on first strong-signal resolution. First hit wins; no call-time date, + * no LLM slug. Returns null on no signal (caller routes to _global/scratch). + */ +function resolveFeatureId(config, baseDir, sessionId, readState) { + baseDir = baseDir || process.cwd(); + const umbrellaRoot = resolveUmbrellaRoot(config, baseDir); + if (!umbrellaRoot) return null; + const cacheKey = `${umbrellaRoot}|${sessionId || ''}`; + if (_featureIdCache.has(cacheKey)) return _featureIdCache.get(cacheKey); + + const featuresDir = path.join(umbrellaRoot, 'features'); + const remember = (id) => { _featureIdCache.set(cacheKey, id); return id; }; + + // 1. explicit per-session override (workbench switch) + const state = readState ? readState(sessionId) : null; + if (state && typeof state.featureId === 'string' && state.featureId) return remember(state.featureId); + + // branch signals + const branch = getGitBranch(baseDir); + const ticket = extractTicketFromBranch(branch, config?.plan?.ticketPrefixes); + const slug = slugFromBranch(branch, config?.plan?.resolution?.branchPattern); + + // 2-3. match an EXISTING feature (survives slug drift / relabel) + if (ticket) { const m = findFeatureBy(featuresDir, 'ticket', ticket); if (m) return remember(m); } + if (slug) { const m = findFeatureBy(featuresDir, 'slug', slug); if (m) return remember(m); } + + // 4. strong branch signal, no existing match → compute id + create the anchor (idempotent) + const computed = computeFeatureId(ticket, slug); + if (computed) { + ensureFeatureMeta(featuresDir, computed, { + id: computed, ticket: ticket || null, slug: slug || null, label: slug || computed, + status: 'active', created: new Date().toISOString(), parentId: null, + supersededBy: null, relatedDocs: [], branches: branch ? [branch] : [] + }); + return remember(computed); + } + + // 5. session-active plan → its parent feature (plan path stored absolute in state) + if (state && state.activePlan) { + let p = state.activePlan; + if (!path.isAbsolute(p) && state.sessionOrigin) p = path.join(state.sessionOrigin, p); + const seg = path.relative(featuresDir, p).split(path.sep); + if (seg[0] && !seg[0].startsWith('..')) return remember(seg[0]); + } + + // 6. no signal + return remember(null); +} + +/** Feature root: umbrella root verbatim when not feature-first (byte-identical); else features/ or _global/scratch. */ +function resolveFeatureRoot(config, baseDir, sessionId, readState) { + baseDir = baseDir || process.cwd(); + const u = resolveUmbrellaRoot(config, baseDir); + if (!u || config?.paths?.layout !== 'feature-first') return u; + const id = resolveFeatureId(config, baseDir, sessionId, readState); + return id ? path.join(u, 'features', id) : path.join(u, '_global', 'scratch'); +} + +/** Root-level cross-feature zones (umbrella only). */ +function getGlobalPath(baseDir, config) { + const u = resolveUmbrellaRoot(config, baseDir); + return u ? path.join(u, '_global') : null; +} +function getArchivePath(baseDir, config) { + const u = resolveUmbrellaRoot(config, baseDir); + return u ? path.join(u, '_archive') : null; +} + // ── venv resolution ─────────────────────────────────────────────────────── /** Check project-local then global ~/.claude for a skills venv python binary. */ @@ -360,5 +504,11 @@ module.exports = { getMainWorktreeRoot, slugFromBranch, extractIssueFromBranch, - resolveUmbrellaRoot + resolveUmbrellaRoot, + extractTicketFromBranch, + computeFeatureId, + resolveFeatureId, + resolveFeatureRoot, + getGlobalPath, + getArchivePath }; diff --git a/hooks/session-init.cjs b/hooks/session-init.cjs index 8aa800a..332d1d8 100644 --- a/hooks/session-init.cjs +++ b/hooks/session-init.cjs @@ -24,7 +24,10 @@ try { resolvePlanPath, extractTaskListId, getGitBranch, - getGitRoot + getGitRoot, + resolveFeatureRoot, + getGlobalPath, + getArchivePath } = require('./lib/paths.cjs'); const { readSessionState, updateSessionState } = require('./lib/state.cjs'); @@ -119,7 +122,7 @@ try { const config = loadConfig(); // Resolve plan (session lookup needs the state reader injected) - const resolved = resolvePlanPath(sessionId, config, readSessionState); + const resolved = resolvePlanPath(sessionId, config, readSessionState, baseDir); // Persist session state if (sessionId) { @@ -149,6 +152,11 @@ try { const visualsPathAbs = umbrellaVal ? getVisualsPath(baseDir, config) : null; const journalsPathAbs = umbrellaVal ? getJournalsPath(baseDir, config) : null; const statePathAbs = umbrellaVal ? getStatePath(baseDir, config) : null; + // Feature-first additions are gated so type-first output stays byte-identical. + const featureFirst = !!umbrellaVal && (config.paths?.layout === 'feature-first'); + const featurePathAbs = featureFirst ? resolveFeatureRoot(config, baseDir) : null; + const globalPathAbs = featureFirst ? getGlobalPath(baseDir, config) : null; + const archivePathAbs = featureFirst ? getArchivePath(baseDir, config) : null; const taskListId = extractTaskListId(resolved); @@ -194,6 +202,12 @@ try { writeEnv(envFile, 'VD_VISUALS_PATH', visualsPathAbs); writeEnv(envFile, 'VD_JOURNALS_PATH', journalsPathAbs); writeEnv(envFile, 'VD_STATE_PATH', statePathAbs); + // Feature-first only — keeps the type-first env output byte-identical. + if (featureFirst) { + writeEnv(envFile, 'VD_FEATURE_PATH', featurePathAbs); + writeEnv(envFile, 'VD_GLOBAL_PATH', globalPathAbs); + writeEnv(envFile, 'VD_ARCHIVE_PATH', archivePathAbs); + } } writeEnv(envFile, 'VD_PROJECT_TYPE', projectType); writeEnv(envFile, 'VD_PACKAGE_MANAGER', packageManager); diff --git a/hooks/subagent-init.cjs b/hooks/subagent-init.cjs index 4697de3..14ee98f 100644 --- a/hooks/subagent-init.cjs +++ b/hooks/subagent-init.cjs @@ -24,7 +24,8 @@ try { resolvePlanPath, extractTaskListId, getGitBranch, - resolveSkillsVenv + resolveSkillsVenv, + resolveFeatureRoot } = require('./lib/paths.cjs'); const { readSessionState } = require('./lib/state.cjs'); @@ -51,7 +52,7 @@ try { // Re-derive naming pattern independently (not from env) const namePattern = resolveNamingPattern(config.plan, gitBranch); - const resolved = resolvePlanPath(sessionId, config, readSessionState); + const resolved = resolvePlanPath(sessionId, config, readSessionState, baseDir); const reportsPath = getReportsPath(resolved.path, resolved.resolvedBy, config.plan, config.paths, baseDir, config); const plansPath = getPlansPath(baseDir, config); const docsPath = getDocsPath(baseDir, config); @@ -59,6 +60,8 @@ try { const visualsPath = umbrellaVal ? getVisualsPath(baseDir, config) : null; const journalsPath = umbrellaVal ? getJournalsPath(baseDir, config) : null; const statePath = umbrellaVal ? getStatePath(baseDir, config) : null; + const featureFirst = !!umbrellaVal && (config.paths?.layout === 'feature-first'); + const featureRoot = featureFirst ? resolveFeatureRoot(config, baseDir) : null; const activePlan = resolved.resolvedBy === 'session' ? resolved.path : ''; const suggestedPlan = resolved.resolvedBy === 'branch' ? resolved.path : ''; @@ -94,6 +97,7 @@ try { } else { lines.push(`- Paths: ${plansPath}/ | ${docsPath}/`); } + if (featureFirst) lines.push(`- Feature: ${featureRoot}/`); lines.push(''); const hasThinking = effectiveThinking && effectiveThinking !== responseLang; diff --git a/internal/hooks/feature-path-contract.mjs b/internal/hooks/feature-path-contract.mjs new file mode 100644 index 0000000..c8f1778 --- /dev/null +++ b/internal/hooks/feature-path-contract.mjs @@ -0,0 +1,50 @@ +#!/usr/bin/env node +// feature-path-contract.mjs — contract test for feature-first path resolution. +// +// Phase 1 asserts the resolver-level contract: +// - resolveFeatureId is DETERMINISTIC (same context → same id). +// - identity is NEVER derived from VD_* env (subagents re-derive from config/branch), +// so a planted VD_FEATURE_PATH must not change the result. +// Phase 2 will add cross-hook injection parity (session-init / subagent-init / +// dev-rules-reminder emit byte-identical Feature: paths) once injection is wired. +// +// Lives in internal/hooks/ so the harness convention ASSETS_DIR = ../../hooks holds. + +import { execFileSync } from 'node:child_process'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import P from '../../hooks/lib/paths.cjs'; + +const { resolveFeatureId } = P; +let pass = 0, fail = 0; +const ok = (n, c) => { if (c) { pass++; console.log(' ✓', n); } else { fail++; console.log(' ✗', n); } }; +const git = (cwd, ...a) => execFileSync('git', a, { cwd, stdio: ['ignore', 'ignore', 'ignore'] }); + +const cfg = () => ({ + paths: { umbrella: '.workbench', layout: 'feature-first' }, + plan: { ticketPrefixes: ['ELT', 'GH', 'PROJ'], resolution: { order: ['session', 'branch'], branchPattern: '(?:feat|fix|chore|refactor|docs)/(?:[^/]+/)?(.+)' } }, +}); +function repo(branch) { + const d = fs.mkdtempSync(path.join(os.tmpdir(), 'fc-')); + git(d, 'init', '-q'); git(d, 'checkout', '-q', '-b', branch); + return d; +} + +console.log('feature-path-contract (resolver-level):'); + +const d1 = repo('feat/ELT-3316-manual-upload'); +const a = resolveFeatureId(cfg(), d1); +const b = resolveFeatureId(cfg(), d1); +ok('deterministic across calls', a === b && a === 'elt-3316-manual-upload'); + +// Identity must come from config/branch, NOT VD_* env. +const d2 = repo('feat/ELT-3316-manual-upload'); +const saved = process.env.VD_FEATURE_PATH; +process.env.VD_FEATURE_PATH = '/tmp/bogus/features/planted-by-env'; +const viaEnv = resolveFeatureId(cfg(), d2); +if (saved === undefined) delete process.env.VD_FEATURE_PATH; else process.env.VD_FEATURE_PATH = saved; +ok('ignores planted VD_FEATURE_PATH env', viaEnv === 'elt-3316-manual-upload'); + +console.log(`\n${pass + fail} tests: ${pass} passed, ${fail} failed`); +process.exit(fail ? 1 : 0); diff --git a/internal/hooks/featurefirst_inject_test.mjs b/internal/hooks/featurefirst_inject_test.mjs new file mode 100644 index 0000000..7ce5f30 --- /dev/null +++ b/internal/hooks/featurefirst_inject_test.mjs @@ -0,0 +1,61 @@ +#!/usr/bin/env node +// featurefirst_inject_test.mjs — Phase 2 end-to-end: session-init emits feature-first env vars +// when a repo opts into layout:feature-first; subagent-init/dev-rules-reminder emit a Feature: line. + +import { execFileSync } from 'node:child_process'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const HOOKS = path.join(path.dirname(fileURLToPath(import.meta.url)), '..', '..', 'hooks'); +let pass = 0, fail = 0; +const ok = (n, c) => { if (c) { pass++; console.log(' ✓', n); } else { fail++; console.log(' ✗', n); } }; +const git = (cwd, ...a) => execFileSync('git', a, { cwd, stdio: ['ignore', 'ignore', 'ignore'] }); + +function repo(layout) { + const d = fs.mkdtempSync(path.join(os.tmpdir(), 'inj-')); + git(d, 'init', '-q'); git(d, 'checkout', '-q', '-b', 'feat/ELT-3316-manual-upload'); + fs.writeFileSync(path.join(d, '.vd.json'), JSON.stringify({ paths: { umbrella: '.workbench', layout } })); + return d; +} +const run = (hook, cwd, payload, extraEnv = {}) => + execFileSync('node', [path.join(HOOKS, hook)], { + cwd, input: JSON.stringify(payload), encoding: 'utf8', + env: { ...process.env, ...extraEnv }, + }); + +console.log('session-init env emission:'); +let d = repo('feature-first'); +let envFile = path.join(d, '.env.out'); +run('session-init.cjs', d, { session_id: 's1', source: 'startup' }, { CLAUDE_ENV_FILE: envFile }); +let env = fs.readFileSync(envFile, 'utf8'); +const has = (k) => new RegExp(`^export ${k}=`, 'm').test(env); +const val = (k) => (env.match(new RegExp(`^export ${k}="([^"]*)"`, 'm')) || [])[1] || ''; +ok('VD_FEATURE_PATH emitted', has('VD_FEATURE_PATH')); +ok('VD_FEATURE_PATH → features/elt-3316-manual-upload', val('VD_FEATURE_PATH').endsWith(path.join('features', 'elt-3316-manual-upload'))); +ok('VD_GLOBAL_PATH emitted', has('VD_GLOBAL_PATH') && val('VD_GLOBAL_PATH').endsWith('_global')); +ok('VD_ARCHIVE_PATH emitted', has('VD_ARCHIVE_PATH') && val('VD_ARCHIVE_PATH').endsWith('_archive')); +ok('VD_PLANS_PATH routes into feature', val('VD_PLANS_PATH').endsWith(path.join('features', 'elt-3316-manual-upload', 'plans'))); +ok('VD_REPORTS_PATH routes into feature', val('VD_REPORTS_PATH').includes(path.join('features', 'elt-3316-manual-upload', 'reports'))); + +console.log('type-first emits NO feature-first vars:'); +d = repo('type-first'); +envFile = path.join(d, '.env.out'); +run('session-init.cjs', d, { session_id: 's2', source: 'startup' }, { CLAUDE_ENV_FILE: envFile }); +env = fs.readFileSync(envFile, 'utf8'); +ok('no VD_FEATURE_PATH (type-first)', !/^export VD_FEATURE_PATH=/m.test(env)); +ok('no VD_GLOBAL_PATH (type-first)', !/^export VD_GLOBAL_PATH=/m.test(env)); +ok('VD_PLANS_PATH stays flat .workbench/plans', /\.workbench\/plans"/.test(env) && !/features\//.test(env)); + +console.log('dev-rules-reminder Feature: line:'); +d = repo('feature-first'); +let out = run('dev-rules-reminder.cjs', d, { session_id: 's3', cwd: d }); +ok('dev-rules emits Feature: line', /Feature: .*features\/elt-3316-manual-upload/.test(out)); + +d = repo('type-first'); +out = run('dev-rules-reminder.cjs', d, { session_id: 's4', cwd: d }); +ok('dev-rules type-first: NO Feature: line', !/^Feature:/m.test(out)); + +console.log(`\n${pass + fail} tests: ${pass} passed, ${fail} failed`); +process.exit(fail ? 1 : 0); diff --git a/internal/hooks/featurefirst_test.mjs b/internal/hooks/featurefirst_test.mjs new file mode 100644 index 0000000..b258ba6 --- /dev/null +++ b/internal/hooks/featurefirst_test.mjs @@ -0,0 +1,91 @@ +#!/usr/bin/env node +// featurefirst_test.mjs — Phase 1 unit tests for feature-first resolution. +// Covers: ticket extraction, id computation (dedup), gating, resolveFeatureId precedence +// (compute+create feature.json, ticket-match across slug drift, session override, no-signal), +// and the routed getters. Git-fixture based. + +import { execFileSync } from 'node:child_process'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import P from '../../hooks/lib/paths.cjs'; + +const { + extractTicketFromBranch, computeFeatureId, resolveFeatureId, resolveFeatureRoot, + getPlansPath, getVisualsPath, getReportsPath, getGlobalPath, getArchivePath, +} = P; + +let pass = 0, fail = 0; +const ok = (n, c) => { if (c) { pass++; console.log(' ✓', n); } else { fail++; console.log(' ✗', n); } }; +const git = (cwd, ...a) => execFileSync('git', a, { cwd, stdio: ['ignore', 'ignore', 'ignore'] }); +const PRE = ['ELT', 'GH', 'PROJ']; +const ends = (p, ...seg) => p && p.endsWith(path.join(...seg)); + +function repo(branch) { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'ff-')); + git(dir, 'init', '-q'); git(dir, 'checkout', '-q', '-b', branch); + return dir; +} +const ffCfg = () => ({ + paths: { umbrella: '.workbench', layout: 'feature-first', plans: 'plans', visuals: 'visuals', journals: 'journals', state: 'state' }, + plan: { reportsDir: 'reports', ticketPrefixes: PRE, resolution: { order: ['session', 'branch'], branchPattern: '(?:feat|fix|chore|refactor|docs)/(?:[^/]+/)?(.+)' } }, +}); +const tfCfg = () => { const c = ffCfg(); c.paths.layout = 'type-first'; return c; }; + +console.log('extractTicketFromBranch:'); +ok('ELT-3316', extractTicketFromBranch('feat/ELT-3316-manual-upload', PRE) === 'ELT-3316'); +ok('gh3251 no-dash → GH-3251', extractTicketFromBranch('fix/gh3251-thing', PRE) === 'GH-3251'); +ok('no ticket → null', extractTicketFromBranch('feat/just-a-slug', PRE) === null); +ok('GH ≠ ELT (no numeric collision)', extractTicketFromBranch('feat/GH-3316-x', PRE) === 'GH-3316'); + +console.log('computeFeatureId (dedup):'); +ok('strips duplicate ticket', computeFeatureId('ELT-3316', 'ELT-3316-manual-upload') === 'elt-3316-manual-upload'); +ok('joins ticket+slug', computeFeatureId('ELT-3316', 'manual-upload') === 'elt-3316-manual-upload'); +ok('ticket only', computeFeatureId('ELT-3316', '') === 'elt-3316'); +ok('slug only', computeFeatureId(null, 'retell-binding') === 'retell-binding'); + +console.log('gating (layout flag):'); +let d = repo('feat/ELT-3316-manual-upload'); +const tfRoot = resolveFeatureRoot(tfCfg(), d); // (macOS tmp /var→/private/var symlink: compare by suffix) +ok('type-first → umbrella root (no features/ routing)', ends(tfRoot, '.workbench') && !tfRoot.includes(`${path.sep}features${path.sep}`)); +ok('feature-first → features/', ends(resolveFeatureRoot(ffCfg(), d), '.workbench', 'features', 'elt-3316-manual-upload')); + +console.log('resolveFeatureId precedence:'); +d = repo('feat/ELT-3316-manual-upload'); +ok('computes id from branch', resolveFeatureId(ffCfg(), d) === 'elt-3316-manual-upload'); +const meta = path.join(d, '.workbench', 'features', 'elt-3316-manual-upload', 'feature.json'); +ok('creates feature.json', fs.existsSync(meta)); +ok('feature.json has ticket', JSON.parse(fs.readFileSync(meta, 'utf8')).ticket === 'ELT-3316'); + +d = repo('feat/ELT-3316-totally-different-slug'); +const fdir = path.join(d, '.workbench', 'features', 'elt-3316-manual-upload'); +fs.mkdirSync(fdir, { recursive: true }); +fs.writeFileSync(path.join(fdir, 'feature.json'), JSON.stringify({ id: 'elt-3316-manual-upload', ticket: 'ELT-3316', slug: 'manual-upload' })); +ok('matches existing by ticket across slug drift', resolveFeatureId(ffCfg(), d) === 'elt-3316-manual-upload'); + +d = repo('feat/ELT-3316-x'); +for (const n of ['elt-3316-a', 'elt-3316-b']) { + const fp = path.join(d, '.workbench', 'features', n); + fs.mkdirSync(fp, { recursive: true }); + fs.writeFileSync(path.join(fp, 'feature.json'), JSON.stringify({ id: n, ticket: 'ELT-3316', slug: n })); +} +const ambig = resolveFeatureId(ffCfg(), d); +ok('ambiguous ticket (>1) refuses — no silent pick of a/b', ambig !== 'elt-3316-a' && ambig !== 'elt-3316-b'); + +d = repo('feat/ELT-3316-x'); +ok('session override wins', resolveFeatureId(ffCfg(), d, 'sess', () => ({ featureId: 'forced-feature' })) === 'forced-feature'); + +d = repo('trunk'); +ok('no signal → null', resolveFeatureId(ffCfg(), d) === null); +ok('no signal → _global/scratch root', ends(resolveFeatureRoot(ffCfg(), d), '_global', 'scratch')); + +console.log('routed getters (feature-first):'); +d = repo('feat/ELT-3316-manual-upload'); +ok('getPlansPath', ends(getPlansPath(d, ffCfg()), 'features', 'elt-3316-manual-upload', 'plans')); +ok('getVisualsPath', ends(getVisualsPath(d, ffCfg()), 'features', 'elt-3316-manual-upload', 'visuals')); +ok('getReportsPath', ends(getReportsPath(null, null, ffCfg().plan, ffCfg().paths, d, ffCfg()), 'features', 'elt-3316-manual-upload', 'reports')); +ok('getGlobalPath', ends(getGlobalPath(d, ffCfg()), '.workbench', '_global')); +ok('getArchivePath', ends(getArchivePath(d, ffCfg()), '.workbench', '_archive')); + +console.log(`\n${pass + fail} tests: ${pass} passed, ${fail} failed`); +process.exit(fail ? 1 : 0); diff --git a/internal/hooks/resolveplanpath_test.mjs b/internal/hooks/resolveplanpath_test.mjs new file mode 100644 index 0000000..6b71a56 --- /dev/null +++ b/internal/hooks/resolveplanpath_test.mjs @@ -0,0 +1,65 @@ +#!/usr/bin/env node +// resolveplanpath_test.mjs — unit test for the Phase 0 resolvePlanPath branch-resolution fix: +// exact-match-prefer, substring-only-when-unique, REFUSE-on-ambiguity, and main-worktree anchoring. +// Git-fixture based (no go toolchain needed). Worktree case skips gracefully if `git worktree` fails. + +import { execFileSync } from 'node:child_process'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import paths from '../../hooks/lib/paths.cjs'; + +const { resolvePlanPath } = paths; +let pass = 0, fail = 0, skip = 0; +const ok = (name, cond) => { if (cond) { pass++; console.log(' ✓', name); } else { fail++; console.log(' ✗', name); } }; +const git = (cwd, ...args) => execFileSync('git', args, { cwd, stdio: ['ignore', 'ignore', 'ignore'] }); + +function repo(branch, planDirs, { umbrella = false } = {}) { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'rpp-')); + git(dir, 'init', '-q'); + git(dir, 'checkout', '-q', '-b', branch); + const plansRoot = umbrella ? path.join(dir, '.workbench', 'plans') : path.join(dir, 'plans'); + fs.mkdirSync(plansRoot, { recursive: true }); + for (const d of planDirs) fs.mkdirSync(path.join(plansRoot, d), { recursive: true }); + return dir; +} + +const cfg = { paths: { plans: 'plans' }, plan: { resolution: { order: ['branch'] } } }; + +console.log('resolvePlanPath branch resolution:'); + +let r = resolvePlanPath(null, cfg, null, repo('feat/auth', ['251101-1505-auth'])); +ok('single exact (date-prefixed) → resolves', r.resolvedBy === 'branch' && r.path.endsWith('251101-1505-auth')); + +r = resolvePlanPath(null, cfg, null, repo('feat/auth', ['auth-v2'])); +ok('single substring-only → resolves', r.resolvedBy === 'branch' && r.path.endsWith('auth-v2')); + +r = resolvePlanPath(null, cfg, null, repo('feat/auth', ['251101-1505-auth', '251101-1600-auth-extra'])); +ok('exact beats extra substring → picks exact', !!r.path && r.path.endsWith('251101-1505-auth')); + +r = resolvePlanPath(null, cfg, null, repo('feat/auth', ['251101-1505-auth', '251102-0900-auth'])); +ok('multiple exact → REFUSE (null)', r.path === null); + +r = resolvePlanPath(null, cfg, null, repo('feat/auth', ['auth-v1', 'auth-v2'])); +ok('multiple substring, no exact → REFUSE (null)', r.path === null); + +r = resolvePlanPath(null, cfg, null, repo('feat/nope', ['251101-1505-auth'])); +ok('no slug match → null', r.path === null); + +// Main-worktree anchoring: a linked worktree must resolve the MAIN repo's umbrella plans. +console.log('main-worktree anchoring (umbrella):'); +try { + const main = repo('main', ['251101-1505-auth'], { umbrella: true }); + git(main, '-c', 'user.email=t@t', '-c', 'user.name=t', 'commit', '--allow-empty', '-q', '-m', 'init'); + const wt = fs.mkdtempSync(path.join(os.tmpdir(), 'rpp-wt-')); + fs.rmSync(wt, { recursive: true, force: true }); + git(main, 'worktree', 'add', '-q', '-b', 'feat/auth', wt); + const ucfg = { paths: { umbrella: '.workbench', plans: 'plans' }, plan: { resolution: { order: ['branch'] } } }; + const rr = resolvePlanPath(null, ucfg, null, wt); + ok('linked worktree → resolves MAIN umbrella plan', !!rr.path && rr.path.includes(path.join('.workbench', 'plans', '251101-1505-auth'))); +} catch (e) { + skip++; console.log(' ⚠ SKIP worktree case (git worktree unavailable):', e.message.split('\n')[0]); +} + +console.log(`\n${pass + fail} tests: ${pass} passed, ${fail} failed${skip ? `, ${skip} skipped` : ''}`); +process.exit(fail ? 1 : 0); From d0a3d8e869b01e0d555fe0a709c927c12c61e1f0 Mon Sep 17 00:00:00 2001 From: Duc Nguyen Date: Wed, 17 Jun 2026 10:33:49 +0700 Subject: [PATCH 2/2] fix(hooks): lowercase slug-only feature id + export cleanSlug computeFeatureId lowercases the slug-only branch for parity with the ticket branch; export cleanSlug so the workbench CLI normalizes user slugs identically to branch resolution (idempotency contract). +mixed-case test. --- hooks/lib/paths.cjs | 3 ++- internal/hooks/featurefirst_test.mjs | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/hooks/lib/paths.cjs b/hooks/lib/paths.cjs index 90af159..e0e581e 100644 --- a/hooks/lib/paths.cjs +++ b/hooks/lib/paths.cjs @@ -369,7 +369,7 @@ function computeFeatureId(ticket, slug) { const desc = slug ? slug.replace(new RegExp(`^${pre}-?${num}-?`, 'i'), '') : ''; return cleanSlug((desc ? `${ticket}-${desc}` : ticket).toLowerCase()); } - if (slug) return cleanSlug(slug); + if (slug) return cleanSlug(slug.toLowerCase()); // lowercase for parity with the ticket branch return null; } @@ -505,6 +505,7 @@ module.exports = { slugFromBranch, extractIssueFromBranch, resolveUmbrellaRoot, + cleanSlug, extractTicketFromBranch, computeFeatureId, resolveFeatureId, diff --git a/internal/hooks/featurefirst_test.mjs b/internal/hooks/featurefirst_test.mjs index b258ba6..ad85ed0 100644 --- a/internal/hooks/featurefirst_test.mjs +++ b/internal/hooks/featurefirst_test.mjs @@ -43,6 +43,7 @@ ok('strips duplicate ticket', computeFeatureId('ELT-3316', 'ELT-3316-manual-uplo ok('joins ticket+slug', computeFeatureId('ELT-3316', 'manual-upload') === 'elt-3316-manual-upload'); ok('ticket only', computeFeatureId('ELT-3316', '') === 'elt-3316'); ok('slug only', computeFeatureId(null, 'retell-binding') === 'retell-binding'); +ok('slug-only lowercases for parity', computeFeatureId(null, 'My-Cool-Slug') === 'my-cool-slug'); console.log('gating (layout flag):'); let d = repo('feat/ELT-3316-manual-upload');