diff --git a/bin/ccam.js b/bin/ccam.js index 378c1bc2..76f83d72 100755 --- a/bin/ccam.js +++ b/bin/ccam.js @@ -1742,15 +1742,24 @@ async function cmdLanesLifecycle(action, args) { */ /** * Whether `lane.detected_stage` is strictly ahead of `lane.stage` in the - * pipeline's node order — the same rule `LaneCard.tsx`'s - * `detectionLeadsDeclaration` uses, so the terminal and the browser never - * disagree about which one is the headline. Unmatched ids sort as -1, so an - * unmatched detected stage never outranks a matched declaration. + * pipeline's node order — the same rule `LaneStripCard.tsx`'s `detectionLeads` + * uses, so the terminal and the browser never disagree about which one is the + * headline. Unmatched ids sort as -1, so an unmatched detected stage never + * outranks a matched declaration. + * + * The declaration is located by the node the SERVER resolved it onto (`state + * === "current"`), never by `indexOf(lane.stage)`: a stage declared under an + * ALIAS — which is most of them, ship-feature's skill says `tests`, `booting`, + * `ship`, `gate-blocked` — is not a node id, so indexOf answers -1 and every + * detection, however far behind, reads as leading. */ function detectionLeadsDeclaration(l) { if (!l.detected_stage) return false; - const ids = (l.pipeline_nodes || []).map((n) => n.id); - return ids.indexOf(l.detected_stage) > ids.indexOf(l.stage); + const nodes = l.pipeline_nodes || []; + return ( + nodes.findIndex((n) => n.id === l.detected_stage) > + nodes.findIndex((n) => n.state === "current") + ); } async function cmdLanes() { diff --git a/client/src/components/lanes/LaneStripCard.tsx b/client/src/components/lanes/LaneStripCard.tsx index a5e1d97c..da14024b 100644 --- a/client/src/components/lanes/LaneStripCard.tsx +++ b/client/src/components/lanes/LaneStripCard.tsx @@ -17,11 +17,22 @@ const LIVENESS_DOT: Record = { dead: "bg-status-danger", }; -/** Whether the inferred stage sits ahead of the declared one in node order. */ +/** + * Whether the inferred stage sits ahead of the declared one in node order. + * + * The declaration is located by the node the server resolved it onto (`state + * === "current"`), not by `indexOf(lane.stage)`: a stage declared under an + * ALIAS (`tests`, `booting`, `ship` — most of what the ship-feature skill + * declares) is not a node id, so indexOf answers -1 and a detection sitting + * well BEHIND the declaration would still be badged as the live one. + */ function detectionLeads(lane: Lane): boolean { if (!lane.detected_stage) return false; - const ids = lane.pipeline_nodes.map((n) => n.id); - return ids.indexOf(lane.detected_stage) > ids.indexOf(lane.stage); + const nodes = lane.pipeline_nodes; + return ( + nodes.findIndex((n) => n.id === lane.detected_stage) > + nodes.findIndex((n) => n.state === "current") + ); } export default function LaneStripCard({ diff --git a/client/src/components/lanes/__tests__/LaneStripCard.test.tsx b/client/src/components/lanes/__tests__/LaneStripCard.test.tsx new file mode 100644 index 00000000..f8bd35b6 --- /dev/null +++ b/client/src/components/lanes/__tests__/LaneStripCard.test.tsx @@ -0,0 +1,97 @@ +/** + * @file Rendering tests for the Workspace lane tile, focused on the amber + * "detected" badge: it may only appear when the inferred stage is genuinely + * AHEAD of the declared one, including when the declaration used a node alias + * (which is most of what the ship-feature skill declares). + * @author Nguyễn Ngọc Trí Vĩ + */ + +import { describe, it, expect } from "vitest"; +import { render, screen } from "@testing-library/react"; +import LaneStripCard from "../LaneStripCard"; +import type { Lane, LaneNode } from "../../../lib/types"; + +/** Four nodes of the ship-feature shape: `tests` is an ALIAS of `gates`. */ +function nodes(currentId: string): LaneNode[] { + return (["intake", "implementing", "gates", "review"] as const).map((id) => ({ + id, + label: id, + icon: "", + gate: false, + state: id === currentId ? "current" : "pending", + })); +} + +function lane(overrides: Partial): Lane { + return { + id: 1, + title: "lane", + cwd: "/tmp/lane", + branch: null, + kind: "adopted", + tool: "claude", + source_repo: null, + pipeline: "ship-feature", + session_id: null, + run_id: null, + stage: "gates", + stage_since: null, + status: "running", + gate_decision: null, + ci_status: null, + needs_action: null, + links: {}, + stages: {}, + notes: null, + pipeline_name: "Ship feature", + pipeline_nodes: nodes("gates"), + progress: 50, + stage_seconds: null, + last_event_seconds: null, + liveness: "active", + detected_stage: null, + detected_signal: null, + slot: null, + ports: {}, + active_feature_id: null, + ...overrides, + } as Lane; +} + +describe("LaneStripCard detected badge", () => { + it("badges a detection that is ahead of the declaration", () => { + render( + {}} + /> + ); + expect(screen.getByTestId("lane-tile-auto-1")).toHaveTextContent("review"); + }); + + it("stays silent when the declaration was made under an alias and the detection is behind it", () => { + // `tests` is an alias of the `gates` node — not a node id. Locating the + // declaration by id would score it -1 and badge this stale `implementing` + // detection as though the lane had moved backwards. + render( + {}} + /> + ); + expect(screen.queryByTestId("lane-tile-auto-1")).toBeNull(); + }); + + it("still badges a detection ahead of an alias declaration", () => { + render( + {}} + /> + ); + expect(screen.getByTestId("lane-tile-auto-1")).toHaveTextContent("review"); + }); +}); diff --git a/docs/LANES.md b/docs/LANES.md index 973753b7..c581dd83 100644 --- a/docs/LANES.md +++ b/docs/LANES.md @@ -838,10 +838,14 @@ key back onto its node, so a stage declared by alias keeps its record — and it node has both a canonical record and an alias record, the canonical one wins. The same rule governs `ccam lanes`: the inferred stage is printed only when it -leads the declared one (see "Viewing lanes" below), and `LaneCard.tsx`'s -`detectionLeadsDeclaration` shows its `auto: ` chip under the identical -condition — the terminal and the browser never disagree about which one is -the headline. +leads the declared one (see "Viewing lanes" below), and `LaneStripCard.tsx`'s +`detectionLeads` shows its badge under the identical condition — the terminal +and the browser never disagree about which one is the headline. Both locate the +declaration by the node the server already resolved it onto (the one whose +`state` is `current`), never by looking `lane.stage` up as a node id: most of +what the ship-feature skill declares is an ALIAS (`tests`, `booting`, `ship`, +`gate-blocked`), which is not an id, and an id lookup scores it -1 — every +detection, however far behind, would then read as leading. ### Which lane a signal is credited to @@ -881,6 +885,14 @@ contain whole code blocks: - `description` - `subagent_type` +Each rule is tested against these fields **one at a time**, never against a join +of them. An `Agent` event really carries `description` + `prompt` + +`subagent_type` together, so a rule anchored to the agent's identity — +`{"tool": "Agent", "match": "^Explore$"}` in `ship-feature.json` — would never +fire against a concatenation of all three, and a rule could otherwise match +across the seam between two unrelated fields. Write anchored rules against the +one field that identifies the tool call. + The signal shown to the user is the span the rule's regex actually matched, plus 24 characters of context on each side, with an ellipsis marking either end that was trimmed. A shell line like diff --git a/server/__tests__/stage-detect.test.js b/server/__tests__/stage-detect.test.js index 7652fea0..77650caa 100644 --- a/server/__tests__/stage-detect.test.js +++ b/server/__tests__/stage-detect.test.js @@ -95,6 +95,29 @@ test("flattenInput handles awkward tool-input shapes without traversing deeply", assert.equal(flattenInput({ nested: { command: "never recurse" } }), ""); }); +test("matches a rule per field, so an anchored rule is not killed by its neighbours", () => { + // A real Agent input carries {description, prompt, subagent_type}. Matched + // against a join of the three, `^Explore$` can never fire and the intake and + // plan rules ship inert; matched per field it hits subagent_type verbatim. + const anchored = { + nodes: [{ id: "intake", detect: [{ tool: "Agent", match: "^Explore$" }] }], + }; + const event = { + tool_name: "Agent", + tool_input: { + description: "Find the stage rules", + prompt: "Search the repo for stage detection rules and report back.", + subagent_type: "Explore", + }, + }; + assert.equal(detect(anchored, event).nodeId, "intake"); + + // The seam between two fields is not matchable either — "rules Search" only + // exists once description and prompt are glued together. + const seam = { nodes: [{ id: "intake", detect: [{ tool: "Agent", match: "rules Search" }] }] }; + assert.equal(detect(seam, event), null); +}); + test("caps a long signal so it never inflates a DB column or a title attribute", () => { const longCommand = `git push ${"x".repeat(4990)}`; const result = detect(pipeline, { tool_name: "Bash", tool_input: { command: longCommand } }); @@ -512,6 +535,22 @@ test.describe("ship-feature pipeline template", () => { const pipeline = getPipeline("ship-feature"); const cases = [ ["intake", "Skill", { skill: "superpowers:brainstorming" }], + // Agent inputs always arrive with a description and a prompt alongside + // the type, which is exactly what these two anchored rules must survive. + [ + "intake", + "Agent", + { + description: "Scout the lane code", + prompt: "Read-only sweep.", + subagent_type: "Explore", + }, + ], + [ + "plan", + "Agent", + { description: "Design the fix", prompt: "Propose an approach.", subagent_type: "Plan" }, + ], ["intake", "Bash", { command: "ccam feature activate lane-stage-detect" }], ["plan", "Skill", { skill: "superpowers:writing-plans" }], ["plan", "Write", { file_path: "docs/superpowers/specs/lane-foo.md" }], diff --git a/server/lib/stage-detect.js b/server/lib/stage-detect.js index a156a2ed..389086a9 100644 --- a/server/lib/stage-detect.js +++ b/server/lib/stage-detect.js @@ -51,11 +51,22 @@ function matchedSpan(regex, input) { return `${prefix}${input.slice(start, end)}${suffix}`; } -/** Return known identifying string fields from a tool input without recursively walking it. */ -function flattenInput(toolInput) { +/** + * Known identifying string fields of a tool input, kept SEPARATE, without + * recursively walking it. + * + * Rules are tested one field at a time (see detect) rather than against a join + * of them. An `Agent` input really carries `{description, prompt, + * subagent_type}`, so on a joined string a rule anchored to the agent's + * identity — `^Explore$`, `^Plan$` in ship-feature.json — can never match: the + * anchors sit against the description and the prompt instead. Per field they + * match the thing they were written for, and no rule can match across the seam + * between two unrelated fields. + */ +function inputFields(toolInput) { try { - if (typeof toolInput === "string") return toolInput; - if (toolInput === null || typeof toolInput !== "object") return ""; + if (typeof toolInput === "string") return [toolInput]; + if (toolInput === null || typeof toolInput !== "object") return []; const strings = []; if (Array.isArray(toolInput)) { @@ -72,12 +83,18 @@ function flattenInput(toolInput) { if (FLATTEN_KEYS.has(key) && typeof value === "string") strings.push(value); } } - return strings.join(" "); + return strings; } catch { - return ""; + return []; } } +/** The same fields as one string — the human-readable signal, and the shape + * the ingest paths log. Never what a rule is matched against. */ +function flattenInput(toolInput) { + return inputFields(toolInput).join(" "); +} + /** Compile valid node rules once, omitting malformed nodes and regex patterns. */ function compileRules(pipeline) { if (!pipeline || typeof pipeline !== "object") return []; @@ -122,19 +139,25 @@ function compileRules(pipeline) { function detect(pipeline, event) { try { if (!event || typeof event.tool_name !== "string" || !event.tool_name) return null; - const input = flattenInput(event.tool_input); - const fallback = input || event.tool_name; + const fields = inputFields(event.tool_input); + const fallback = fields.join(" ") || event.tool_name; let match = null; for (const node of compileRules(pipeline)) { if (typeof node.nodeId !== "string" || !node.nodeId) continue; for (const rule of node.rules) { if (rule.tool !== event.tool_name) continue; - if (!rule.regex || rule.regex.test(input)) { - const span = matchedSpan(rule.regex, input); - match = { nodeId: node.nodeId, signal: `\`${capSignal(span || fallback)}\`` }; - break; - } + // A rule with no regex matched on the tool name alone. + const hit = rule.regex + ? fields.find((f) => { + rule.regex.lastIndex = 0; // a /g rule from a user template is stateful + return rule.regex.test(f); + }) + : ""; + if (hit === undefined) continue; + const span = hit ? matchedSpan(rule.regex, hit) : null; + match = { nodeId: node.nodeId, signal: `\`${capSignal(span || fallback)}\`` }; + break; } } return match; @@ -143,4 +166,4 @@ function detect(pipeline, event) { } } -module.exports = { flattenInput, compileRules, detect }; +module.exports = { flattenInput, inputFields, compileRules, detect };