From f6b35422d3c2aae4c27c3c17eea7f43636e7159d Mon Sep 17 00:00:00 2001 From: Darren Zhu Date: Thu, 3 Sep 2026 23:07:17 -0700 Subject: [PATCH] sync(plugin): retime the mid-session offer and make declining recoverable (v1.17.0) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Propagates forge-mcp's plugin/ at 3eeeae60. Five hooks, one of them new. The mid-session tracking offer used to become eligible at the end of the very first turn, before anything was known about the session, so it read as onboarding noise — and dismissal was terminal, which meant one reflexive decline disabled tracking for the rest of the session, including at the moment it would have helped. Those were one failure, not two. - The offer now waits for real signal: 4 turns, 5 minutes of active work, or a commit. Where the session log cannot be read it degrades to turns only, never wall-clock. - Declining is graded. "Not now" comes back later and the returning offer acknowledges the earlier decline; only an explicit opt-out is terminal. - The git-milestone read moves into its own module, hooks/git-head.cjs. It follows the gitdir: pointer so worktrees resolve to their own HEAD rather than the main checkout's, walks up from subdirectories to the enclosing checkout, and stops at the home directory so a dotfiles repo is never mistaken for the project. - The baseline is seeded on the first prompt, so a commit made during the first turn counts instead of becoming its own baseline. - The offer's copy reads as prose — "going for 1 turn", not "1 turns". Source verified green before syncing: 8072 tests pass, 0 fail. Localized files were left alone: token-usage.cjs, workflow-guard.cjs, hooks.json and the SKILL.md files are untouched in every target. Co-Authored-By: Claude Opus 5 --- plugins/forge/.codex-plugin/plugin.json | 2 +- plugins/forge/hooks/git-head.cjs | 210 +++++++++++++++++++++++ plugins/forge/hooks/prompt-router.cjs | 24 ++- plugins/forge/hooks/session-state.cjs | 20 ++- plugins/forge/hooks/stop-observer.cjs | 144 +++++++++++++++- plugins/forge/hooks/workflow-tracker.cjs | 25 ++- 6 files changed, 411 insertions(+), 14 deletions(-) create mode 100644 plugins/forge/hooks/git-head.cjs diff --git a/plugins/forge/.codex-plugin/plugin.json b/plugins/forge/.codex-plugin/plugin.json index 2e4a3b9..d37ce00 100644 --- a/plugins/forge/.codex-plugin/plugin.json +++ b/plugins/forge/.codex-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "forge", - "version": "1.16.0", + "version": "1.17.0", "description": "Forge by ShipToday brings free, AI-powered product development lifecycle automation into Codex.", "author": { "name": "ShipToday", diff --git a/plugins/forge/hooks/git-head.cjs b/plugins/forge/hooks/git-head.cjs new file mode 100644 index 0000000..d34a29d --- /dev/null +++ b/plugins/forge/hooks/git-head.cjs @@ -0,0 +1,210 @@ +/** + * git-head.cjs — resolve the current HEAD sha without spawning git. + * + * Shared by two hooks that between them implement the git-milestone route to + * observer eligibility (SHI-906 AC2): + * + * - prompt-router.cjs (UserPromptSubmit) SEEDS `git_head_baseline` on the + * session's first prompt, BEFORE any work has happened. + * - stop-observer.cjs (Stop) COMPARES HEAD against that baseline after the + * turn, fires the nudge when it moved, and advances the baseline. + * + * The seed has to happen on the prompt side. Seeded at the first Stop — after + * the turn's work — a commit made during turn 1 became the baseline itself + * and was never seen, which is exactly the high-intent moment the milestone + * exists to catch. + * + * WORKTREE-AWARE, and that is the whole difficulty. In a normal checkout + * `.git` is a directory. In a worktree it is a FILE containing a `gitdir:` + * pointer, the real HEAD lives at `/HEAD`, and the main repository's + * own `.git/HEAD` names a DIFFERENT branch. Reading `.git/HEAD` naively + * therefore returns the WRONG branch rather than nothing — it would compare + * the developer's feature-branch work against `main` and either miss the + * milestone or report a phantom one. A lot of agent work happens in + * worktrees, so this is the common case, not an edge case. + * + * NO SUBPROCESS. This runs on every prompt and every Stop in every session, + * which makes it the hottest path in the plugin; `git rev-parse` was rejected + * on that ground alone. Everything here is a handful of bounded local file + * reads. + * + * Every failure — not a repo, malformed pointer, dangling ref, unreadable + * file — returns null. A milestone we cannot prove is not a milestone. + */ + +'use strict'; + +const fs = require('fs'); +const os = require('os'); +const path = require('path'); + +/** + * Where do this git dir's refs actually live? + * + * For a normal repo, in the git dir itself. For a WORKTREE, in the shared + * git dir of the main checkout — the worktree's own dir holds only HEAD and + * a few per-worktree files. + * + * Git answers this itself: it writes a `commondir` file inside every + * worktree git dir, holding a path (usually the relative `../..`) to the + * shared dir. Reading it is exact. + * + * The earlier version instead sniffed the gitdir path for + * `${path.sep}worktrees${path.sep}`, which NEVER matched on Windows: git + * writes the gitdir pointer with FORWARD slashes on every platform, while + * `path.sep` there is a backslash. The check silently failed, refs resolved + * against the worktree dir where they do not exist, and readHeadRef fell + * through to returning the branch NAME as the baseline — so a later commit + * compared equal and the milestone never fired. Silently, and only on + * Windows worktrees. Hence: ask git, and keep a separator-agnostic regex + * as the fallback rather than a separator-specific one. + */ +function resolveCommonDir(gitDir, fromPointer) { + try { + const commonDir = fs.readFileSync(path.join(gitDir, 'commondir'), 'utf8').trim(); + if (commonDir) return path.resolve(gitDir, commonDir); + } catch { /* not a worktree, or unreadable — fall through */ } + // Only a git dir reached THROUGH a `.git` file can be a worktree dir. A + // plain `.git` DIRECTORY is its own common dir however its path happens + // to be spelled — a checkout living under a folder called `worktrees` + // must not have its refs looked up two levels above the repo. + if (!fromPointer) return gitDir; + // Match either separator: the pointer git writes and the one the host + // platform uses are not necessarily the same character. + return /[\\/]worktrees[\\/]/.test(gitDir) + ? path.resolve(gitDir, '..', '..') + : gitDir; +} + +/** + * Find the nearest `.git` at or above `cwd`, the way git itself does. + * + * A session is very often started somewhere below the repository root — + * `src/`, a package directory, a test folder. Checking only `cwd/.git` meant + * the milestone never fired for any of those, silently: no error, just an + * eligibility route that quietly did not exist. Git walks up to the + * filesystem root, so this does too. + * + * STOPS AT THE HOME DIRECTORY, and that boundary is load-bearing. A `.git` + * at exactly `~` is almost always a dotfiles repo, so without the stop a + * session in any non-repo folder under home (`~/Documents/notes`, a temp + * dir) walks up and adopts it — and then an unrelated dotfiles commit fires + * the nudge for a session that has nothing to do with it. That is a WRONG + * signal, not a missing one, which is the more expensive of the two: the + * error-handling NFR ranks over-prompting above a missed offer. Projects + * living under home are unaffected, because the walk stops AT `~` rather + * than before reaching them. + * + * Otherwise bounded by construction — `path.dirname` reaches a fixed point + * at the filesystem root (and at a drive root on Windows), which ends the + * loop. MAX_DEPTH is a belt-and-braces stop so a pathological path can never + * spin: this runs on every prompt and every Stop in every session. + */ +const MAX_GIT_WALK_DEPTH = 64; + +// Path equality for the home stop. Windows paths are case-insensitive, and +// the two sides here come from different sources — `process.cwd()` keeps +// whatever drive-letter case the session was launched with, `os.homedir()` +// comes from USERPROFILE — so a byte compare can miss `c:\Users\me` against +// `C:\Users\me`, walk straight past home, and adopt `~/.git` after all. +function samePath(a, b) { + return process.platform === 'win32' + ? a.toLowerCase() === b.toLowerCase() + : a === b; +} + +/** + * Shape check for a symbolic ref read from HEAD, before it is used as a + * path segment. Git's own rules are stricter than this; the point here is + * only that nothing outside `/refs/…` can be reached through it. + */ +function isSafeRefName(ref) { + if (!/^refs\/[A-Za-z0-9._@+\-\/]+$/.test(ref)) return false; + return ref.split('/').every((segment) => segment !== '' && segment !== '.' && !segment.includes('..')); +} + +function findDotGit(startDir) { + let home = null; + try { home = path.resolve(os.homedir()); } catch { /* no home — rely on the root stop */ } + let dir = path.resolve(startDir); + for (let depth = 0; depth < MAX_GIT_WALK_DEPTH; depth += 1) { + // Reaching home means every real project directory has been checked + // already. Stop before adopting a dotfiles repo as this session's repo. + if (home && samePath(dir, home)) return null; + const candidate = path.join(dir, '.git'); + try { + return { dotGit: candidate, stat: fs.statSync(candidate) }; + } catch { /* not here — keep walking up */ } + const parent = path.dirname(dir); + if (parent === dir) return null; // reached the root + dir = parent; + } + return null; +} + +/** + * The current HEAD sha for the repository containing `cwd`, the ref name + * when the ref resolves to nothing (an unborn branch still distinguishes + * one branch from another), or null when there is no repository to read. + */ +function readHeadRef(cwd) { + try { + const found = findDotGit(cwd); + if (!found) return null; + const { dotGit, stat } = found; + + // A worktree (or submodule): follow the gitdir: pointer to the real dir. + let gitDir = dotGit; + if (stat.isFile()) { + const pointer = fs.readFileSync(dotGit, 'utf8').trim(); + if (!pointer.startsWith('gitdir:')) return null; + const target = pointer.slice('gitdir:'.length).trim(); + if (!target) return null; + // Git writes the pointer ABSOLUTE for worktrees and RELATIVE for + // submodules (`gitdir: ../.git/modules/`), relative to the + // `.git` file itself. Resolve against that file's directory, never the + // process cwd — after the walk-up the two can be several levels apart, + // and a cwd-relative resolve lands on a path that does not exist. + gitDir = path.resolve(path.dirname(dotGit), target); + } + + const head = fs.readFileSync(path.join(gitDir, 'HEAD'), 'utf8').trim(); + + // Detached HEAD: the file holds the sha directly. + if (!head.startsWith('ref:')) return head || null; + + const ref = head.slice('ref:'.length).trim(); + if (!ref) return null; + // The ref is joined into a filesystem path below, and HEAD is content + // the repository controls. Refuse anything git itself would refuse — + // `.`/`..` segments, names outside refs/, stray characters — rather than + // read an arbitrary file's first line into the state file. + if (!isSafeRefName(ref)) return null; + + // Loose ref. A worktree's refs live in the SHARED git dir, not its own, + // so resolve against the common dir when this is a worktree. + const commonDir = resolveCommonDir(gitDir, stat.isFile()); + try { + const sha = fs.readFileSync(path.join(commonDir, ref), 'utf8').trim(); + if (sha) return sha; + } catch { /* fall through to packed-refs */ } + + // Packed ref: a branch that has never been checked out loosely. + try { + const packed = fs.readFileSync(path.join(commonDir, 'packed-refs'), 'utf8'); + for (const line of packed.split('\n')) { + const [sha, name] = line.trim().split(/\s+/); + if (name === ref && sha) return sha; + } + } catch { /* no packed-refs */ } + + // A ref that resolves to nothing (e.g. an unborn branch) is not a + // milestone, but the ref NAME still distinguishes one branch from + // another, so it is a usable baseline on its own. + return ref; + } catch { + return null; + } +} + +module.exports = { readHeadRef }; diff --git a/plugins/forge/hooks/prompt-router.cjs b/plugins/forge/hooks/prompt-router.cjs index 7b18ae9..6441856 100644 --- a/plugins/forge/hooks/prompt-router.cjs +++ b/plugins/forge/hooks/prompt-router.cjs @@ -26,6 +26,8 @@ * - snoozed wake check (session_observer writes this) * * Execution order (first match wins): + * 0. Seed the git baseline (SHI-906) — silent, once per session, before + * any of the routing below and before this turn's work happens * 1. Linked → silent (already tracked, no directive needed) * 2. Active workflow → emit continuation directive * 3. Epic key in prompt → emit epic-key routing directive @@ -40,6 +42,7 @@ 'use strict'; const sessionStateModule = require('./session-state.cjs'); +const { readHeadRef } = require('./git-head.cjs'); // -- Detection patterns ------------------------------------------------------ @@ -62,8 +65,8 @@ function emitWakeConditionCheck(wakeCondition) { } function emitEpicKeyRouting(key) { - // Advisory tone (was forced "MUST invoke"). The orchestrator - // now handles cited-reference disambiguation via `needsKeyConfirmation`, + // Advisory tone (was forced "MUST invoke"). The server now handles + // cited-reference disambiguation via `needsKeyConfirmation`, // so the hook no longer needs to force the routing path. The hint // remains because it's the structural signal that nudges Claude away // from grabbing the work item directly via tracker MCP tools when @@ -123,6 +126,23 @@ async function main() { const sessionState = sessionStateModule.forSession(event.session_id); const state = sessionState.read(); + // Step 0 (SHI-906): seed the git baseline BEFORE this turn's work happens. + // stop-observer.cjs detects a commit by comparing HEAD against this value + // after the turn. Seeded there — at the first Stop — a commit made during + // turn 1 became the baseline itself and was never a milestone, which is + // the high-intent moment AC2 exists to catch. Ownership is split: this + // hook ESTABLISHES the baseline once, the Stop hook ADVANCES it whenever a + // milestone is consumed, so nothing here touches a value already set. + // Outside a repository readHeadRef is null and the field stays null; the + // cost is a few bounded stat calls per prompt, no subprocess. + if (!state.git_head_baseline) { + const head = readHeadRef(process.cwd()); + if (head) { + sessionState.write({ git_head_baseline: head }); + state.git_head_baseline = head; + } + } + // Re-arm the session observer on each new turn when it's safe to do so. // // `observer_blocked` is intended as a "this turn only" gate — it prevents diff --git a/plugins/forge/hooks/session-state.cjs b/plugins/forge/hooks/session-state.cjs index c3f6458..0b6532e 100644 --- a/plugins/forge/hooks/session-state.cjs +++ b/plugins/forge/hooks/session-state.cjs @@ -126,8 +126,22 @@ function freshState(sessionId) { session_start: new Date().toISOString(), turn_count: 0, nudge_shown: false, - status: null, // null | "snoozed" | "dismissed" | "linked" | "logged" + // null | "snoozed" | "dismissed" | "linked" | "logged". + // SHI-907: a soft decline ("not this one") is persisted AS "snoozed" so + // the existing re-fire path picks it up unchanged; only the audit + // `outcome` distinguishes it from an explicit snooze. "dismissed" + // remains terminal and means "stop asking". + status: null, wake_condition: null, + // SHI-907: set when the user softly declines the observer offer. NOT + // cleared by the snooze re-fire, so a returning offer can acknowledge + // the earlier "no" instead of repeating itself verbatim (AC4). + declined_once: false, + // SHI-906: HEAD sha (or ref name) seen when this session was first + // observed inside a repository. The git-milestone eligibility route in + // stop-observer.cjs seeds it on first sight and advances it whenever a + // milestone is consumed; null until then, and forever outside a repo. + git_head_baseline: null, routing_emitted: false, active_workflow: false, observer_blocked: false, @@ -170,7 +184,7 @@ function freshState(sessionId) { // of the active-time window it stamps onto forge__update_state's // duration_ms. null until the first step begins. step_active_since: null, - // ── forge_observation_enabled ────────────────────────────────────── + // ── observation gate contract: forge_observation_enabled ─────────── // Per-Claude-Code-session cache of the org-admin's observation // toggle (Clerk publicMetadata.forgeObservationEnabled, surfaced // on the MCP side as context.org_settings.forgeObservationEnabled). @@ -191,7 +205,7 @@ function freshState(sessionId) { // logic on either side. Admin toggles take effect at the next // Claude Code session start (which begins with a fresh state file). // Field name shared verbatim with the Cursor stop-observer.cjs - // parity — do NOT rename without coordinating both halves. + // (parity) — do NOT rename without coordinating both halves. forge_observation_enabled: null, }; } diff --git a/plugins/forge/hooks/stop-observer.cjs b/plugins/forge/hooks/stop-observer.cjs index 7b05346..3c23302 100644 --- a/plugins/forge/hooks/stop-observer.cjs +++ b/plugins/forge/hooks/stop-observer.cjs @@ -81,6 +81,7 @@ const sessionStateModule = require('./session-state.cjs'); const { resolveSessionRecords, captureTokenUsageFromResolved } = require('./token-usage.cjs'); const { activeMsFromResolved } = require('./active-time.cjs'); +const { readHeadRef } = require('./git-head.cjs'); // -- Constants ---------------------------------------------------------------- @@ -97,6 +98,50 @@ const FLUSH_INTERVAL = 3; // turns between checkpoints when skill invocatio // silent forced-continuation turns (lower token overhead). const TIME_FLOOR_MS = 10 * 60 * 1000; // 10 minutes +// SHI-906: eligibility floor for the FIRST nudge of a session. The observer +// used to become eligible at the end of turn 1, before there was any signal +// about what the session was even about, so it read as onboarding noise — +// and because a dismissal was terminal, that one bad impression was also the +// last one. These hold it back until the session has actually produced +// something to talk about. Tunable: smaller = the offer arrives sooner but +// on thinner evidence, larger = fewer interruptions but more missed moments. +// Deliberately lower than the checkpoint constants above: this is "has +// anything happened yet", not "how often should we bank time". +const TURN_FLOOR = 4; // turns before the first nudge is eligible +const ACTIVE_FLOOR_MS = 5 * 60 * 1000; // ...or this much ACTIVE working time + +/** + * Has HEAD moved since this session was first observed? + * + * The baseline is normally seeded by prompt-router.cjs on the session's + * FIRST PROMPT — before any work has happened — so a commit made during + * turn 1 is already a difference by the time this runs. Seeding here is the + * fallback for a session whose prompt hook never ran (older state, a hook + * that failed): the first sighting SEEDS and reports no milestone, because + * treating it as one would fire the nudge on turn 1 of every repo session — + * reintroducing exactly the noise the eligibility floor removes. + * + * The read itself lives in git-head.cjs, shared with the prompt hook. + */ +function milestoneReached(state, sessionState) { + const head = readHeadRef(process.cwd()); + if (!head) return false; + if (!state.git_head_baseline) { + sessionState.write({ git_head_baseline: head }); + return false; + } + if (head === state.git_head_baseline) return false; + // ADVANCE the baseline when a milestone is consumed. Leaving it stale + // would make one commit justify every subsequent check for the rest of + // the session — harmless while the nudge fires only once, but SHI-907 + // lets a soft decline bring the offer back, and a permanently-true + // milestone would re-fire it on every Stop from then on. That is the + // over-prompting the error-handling NFR explicitly ranks as worse than + // a missed offer. + sessionState.write({ git_head_baseline: head }); + return true; +} + // -- Directives --------------------------------------------------------------- /** @@ -193,11 +238,38 @@ function buildCheckpointResponse(durationMs, state, stateFilePath, event, resolv // skill. Keeping this short matters // because some clients (Codex, Cursor) surface the Stop-hook block reason to // the user verbatim, where the old ~30-line block read as noise. -function buildBlockResponse(stateFilePath) { +function buildBlockResponse(stateFilePath, turnCount = null, activeMs = null, declinedOnce = false) { + // SHI-906 AC3: the nudge's "why now" must match how long the session has + // ACTUALLY been running. The eligibility gate already computed both + // figures to make its decision, so they are threaded through here rather + // than re-derived — a separately-computed number could disagree with the + // one that opened the gate, which is precisely the inconsistency the AC + // is about. Omitted (null) from the snooze re-fire path, where the + // session's elapsed figures are not the reason the offer is returning. + // Some clients surface this copy verbatim, so it has to read as prose: + // singular at 1 (a git milestone can open the gate on turn 1), and no + // "0 minutes" clause when active time rounds down to nothing. + const minutes = Number.isFinite(activeMs) && activeMs > 0 ? Math.round(activeMs / 60000) : 0; + const elapsed = [ + Number.isFinite(turnCount) ? `${turnCount} ${turnCount === 1 ? 'turn' : 'turns'}` : null, + minutes >= 1 ? `${minutes} ${minutes === 1 ? 'minute' : 'minutes'} of active work` : null, + ].filter(Boolean).join(' and '); + const context = elapsed + ? `This session has been going for ${elapsed}. Say so if you explain why you are checking in now. ` + : ''; + // SHI-907 AC4: a re-offer after a soft decline must not repeat the first + // one verbatim. The user already said no once; asking again in identical + // words reads as not having listened — and since a soft decline and a + // snooze now behave alike, this copy is the only difference they can + // actually perceive. + const reoffer = declinedOnce + ? `The user already declined this once in this session, so do NOT repeat the earlier offer word for word: ` + + `open by acknowledging that, say briefly what has changed since, and keep it shorter than the first. ` + : ''; return JSON.stringify({ decision: 'block', reason: - `FORGE OBSERVATION: Before stopping, decide whether this session involved any product ` + + `FORGE OBSERVATION: ${context}${reoffer}Before stopping, decide whether this session involved any product ` + `or engineering work across the software development lifecycle (SDLC) — defining, planning, ` + `building, testing, reviewing, or discussing code or features (reading or analyzing code to ` + `understand it counts). If it did, you MUST invoke the "forge-autopilot" skill via the Skill ` + @@ -373,7 +445,7 @@ async function main() { // engineering-time tracking on already-tracked sessions continues // independently of the observation toggle — the toggle gates the // initial nudge, not silent checkpoints on linked/logged work. - // Field name shared verbatim with the Cursor parity. + // Field name shared verbatim with the Cursor build (parity). // // This cache is intentionally per-session (not cross-session): each new // Claude Code / Codex / Cursor session starts with a fresh state file, so @@ -390,14 +462,26 @@ async function main() { // Reset state so observer can re-prompt the user. Also reset // observer_fired so the per-session "fire once" counter restarts — // the user explicitly asked to be re-prompted by snoozing. + // + // SHI-907: `declined_once` is deliberately NOT cleared here. It is what + // lets the returning offer acknowledge that the user already said no + // rather than repeating itself verbatim (AC4). A soft decline reaches + // this same branch — that reuse is the whole point of the design, since + // both planes already speak the snooze contract end to end and adding a + // parallel status would fail invisibly across the plane boundary. sessionState.write({ observer_blocked: false, observer_fired: false, status: null, last_observer_turn: state.turn_count, }); - // Block with the standard observer directive - process.stdout.write(buildBlockResponse(sessionState.stateFilePath)); + // Block with the observer directive. The elapsed figures are omitted: + // the reason this offer is BACK is the earlier decline, not how long + // the session has run, so quoting a duration here would answer a + // question the user did not ask. + process.stdout.write(buildBlockResponse( + sessionState.stateFilePath, null, null, Boolean(state.declined_once), + )); return; } @@ -415,6 +499,38 @@ async function main() { // Step 7: Already blocked once this session — don't re-block if (state.observer_blocked) return; + // Step 7b: Eligibility floor — real signal must exist before the FIRST + // nudge of a session (SHI-906 AC1/AC2). + // + // PLACEMENT IS LOAD-BEARING. This sits AFTER Step 7's observer_blocked + // check and BEFORE Step 8's write. Moved below that write, the one-shot + // latch trips on turn 1 and the session is permanently spent WITHOUT ever + // nudging — strictly worse than the bug this fixes, and silent: the user + // simply never sees the offer again and nothing is logged anywhere. The + // test `suppressing a turn must NOT consume the session's one-shot + // eligibility` in stop-observer-eligibility.test.js exists to catch that. + // + // Fires on turns OR active time OR a git milestone, mirroring the + // either/or shape of the checkpoint gate in Step 3 above. + // + // The active-time read deliberately does NOT fall back to wall-clock the + // way Step 3 does. Where the transcript is unreadable — which is always + // the case on Cursor — a session left open for hours with a single turn + // would otherwise become eligible on elapsed time alone, which is exactly + // the noise AC1 removes. It degrades to TURNS ONLY. + // + // Everything here is inside main()'s catch, so a fault in the new logic + // fails toward silence rather than toward prompting — the asymmetry the + // error-handling NFR asks for. + const hasTurns = state.turn_count >= TURN_FLOOR; + const eligibilityActiveMs = activeMsFromResolved( + resolveSessionRecords(event), + new Date(state.session_start).getTime(), + ); + const hasActiveTime = Number.isFinite(eligibilityActiveMs) + && eligibilityActiveMs >= ACTIVE_FLOOR_MS; + if (!hasTurns && !hasActiveTime && !milestoneReached(state, sessionState)) return; + // Step 8: Mark as blocked so we don't fire again on the same turn, AND // mark observer_fired so prompt-router.cjs preserves the "fire once" UX // on subsequent turns (the workflow-completion clear path keys off @@ -422,8 +538,22 @@ async function main() { // observer never fired" case, not the "observer fired, user ignored it" case). sessionState.write({ observer_blocked: true, observer_fired: true }); - // Step 9: Block Claude's exit and direct it to evaluate the session - process.stdout.write(buildBlockResponse(sessionState.stateFilePath)); + // Step 9: Block Claude's exit and direct it to evaluate the session. + // Pass the SAME figures the eligibility gate used (SHI-906 AC3). + // + // `declined_once` has to be threaded here too, not only on Step 4's + // re-fire path. Step 4 writes `status: null` when it re-offers, so the + // NEXT Stop no longer matches Step 4 and arrives HERE instead. Omitting + // the flag meant the second and every later re-offer silently reverted to + // the original first-offer wording — the exact "asked again as if it had + // never asked" behaviour SHI-907 AC4 exists to prevent, and invisible + // because the copy still reads perfectly well on its own. + process.stdout.write(buildBlockResponse( + sessionState.stateFilePath, + state.turn_count, + eligibilityActiveMs, + Boolean(state.declined_once), + )); } main().catch(() => { diff --git a/plugins/forge/hooks/workflow-tracker.cjs b/plugins/forge/hooks/workflow-tracker.cjs index 5459d54..6682b87 100644 --- a/plugins/forge/hooks/workflow-tracker.cjs +++ b/plugins/forge/hooks/workflow-tracker.cjs @@ -206,6 +206,17 @@ const OUTCOME_TO_STATUS = { ad_hoc: 'logged', snoozed: 'snoozed', dismissed: 'dismissed', + // SHI-907: a SOFT decline. Distinct from `dismissed`, which stays + // terminal. Mapped to the `snoozed` status because both planes already + // speak that vocabulary end to end — the re-fire branch in + // stop-observer.cjs and the wake check in prompt-router.cjs both read it. + // Introducing a new status value instead would have to cross the + // schema-free final_session_state boundary, where the validation below is + // ONE-DIRECTIONAL: an unrecognised value is silently dropped to this map + // rather than raising, so the mistake would never surface. The two + // outcomes stay separable in the audit trail via `outcome`, which is what + // AC3 actually needs; only the local session STATUS is shared. + declined_for_now: 'snoozed', }; // The tracking statuses stop-observer.cjs recognises. A skill-declared @@ -486,8 +497,20 @@ async function main() { const observerEvent = extractObserverEvent(event); if (observerEvent) { - const { status: observerStatus, sdlcStage } = observerEvent; + const { status: observerStatus, outcome: observerOutcome, sdlcStage } = observerEvent; const statusUpdates = {}; + // SHI-907: a soft decline is DERIVED from the outcome here rather than + // read from a field on final_session_state. That is not a stylistic + // choice — `extractObserverEvent` returns only { status, outcome, + // sdlcStage }, so any other key the skill puts on final_session_state + // is silently discarded on this side of the plane boundary. A + // `declined_once` sent across directly would simply never arrive, with + // no error at either end, and AC4's acknowledging re-offer would + // quietly never fire. The outcome already crosses validated, so the + // client-local flag is computed from it instead. + if (observerOutcome === 'declined_for_now') { + statusUpdates.declined_once = true; + } // Status-carrying outcomes (ad_hoc → logged, snoozed, dismissed) set the // tracking status and advance the checkpoint baseline. Stage-only // outcomes (linked/created) carry no status mapping — they persist the