fix(lanes): revive dead detect rules and stop alias-blind detection badges
Two defects that made the 16-node ship-feature pipeline look like stage detection was broken: - stage-detect matched each rule against a JOIN of the tool input's identifying fields. An Agent event really carries description + prompt + subagent_type, so `^Explore$` (intake) and `^Plan$` (plan) could never fire and both nodes shipped inert — the map jumped straight to `implementing`. Rules are now tested one field at a time, which also stops a rule matching across the seam between two unrelated fields. - `detectionLeadsDeclaration` (bin/ccam.js) and `detectionLeads` (LaneStripCard.tsx) located the declaration with `indexOf(lane.stage)`. Most of what the skill declares is an ALIAS (`tests`, `booting`, `ship`, `gate-blocked`), which is not a node id, so indexOf answered -1 and every detection — however far behind — was badged as leading. Both now use the node the server already resolved it onto (`state === "current"`). server 1346/1346, client 447/447.
This commit is contained in:
+15
-6
@@ -1742,15 +1742,24 @@ async function cmdLanesLifecycle(action, args) {
|
|||||||
*/
|
*/
|
||||||
/**
|
/**
|
||||||
* Whether `lane.detected_stage` is strictly ahead of `lane.stage` in the
|
* Whether `lane.detected_stage` is strictly ahead of `lane.stage` in the
|
||||||
* pipeline's node order — the same rule `LaneCard.tsx`'s
|
* pipeline's node order — the same rule `LaneStripCard.tsx`'s `detectionLeads`
|
||||||
* `detectionLeadsDeclaration` uses, so the terminal and the browser never
|
* uses, so the terminal and the browser never disagree about which one is the
|
||||||
* disagree about which one is the headline. Unmatched ids sort as -1, so an
|
* headline. Unmatched ids sort as -1, so an unmatched detected stage never
|
||||||
* unmatched detected stage never outranks a matched declaration.
|
* 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) {
|
function detectionLeadsDeclaration(l) {
|
||||||
if (!l.detected_stage) return false;
|
if (!l.detected_stage) return false;
|
||||||
const ids = (l.pipeline_nodes || []).map((n) => n.id);
|
const nodes = l.pipeline_nodes || [];
|
||||||
return ids.indexOf(l.detected_stage) > ids.indexOf(l.stage);
|
return (
|
||||||
|
nodes.findIndex((n) => n.id === l.detected_stage) >
|
||||||
|
nodes.findIndex((n) => n.state === "current")
|
||||||
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
async function cmdLanes() {
|
async function cmdLanes() {
|
||||||
|
|||||||
@@ -17,11 +17,22 @@ const LIVENESS_DOT: Record<Lane["liveness"], string> = {
|
|||||||
dead: "bg-status-danger",
|
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 {
|
function detectionLeads(lane: Lane): boolean {
|
||||||
if (!lane.detected_stage) return false;
|
if (!lane.detected_stage) return false;
|
||||||
const ids = lane.pipeline_nodes.map((n) => n.id);
|
const nodes = lane.pipeline_nodes;
|
||||||
return ids.indexOf(lane.detected_stage) > ids.indexOf(lane.stage);
|
return (
|
||||||
|
nodes.findIndex((n) => n.id === lane.detected_stage) >
|
||||||
|
nodes.findIndex((n) => n.state === "current")
|
||||||
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
export default function LaneStripCard({
|
export default function LaneStripCard({
|
||||||
|
|||||||
@@ -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ĩ <vinnt@smartgift.vn>
|
||||||
|
*/
|
||||||
|
|
||||||
|
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>): 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(
|
||||||
|
<LaneStripCard
|
||||||
|
lane={lane({ detected_stage: "review" })}
|
||||||
|
selected={false}
|
||||||
|
onSelect={() => {}}
|
||||||
|
/>
|
||||||
|
);
|
||||||
|
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(
|
||||||
|
<LaneStripCard
|
||||||
|
lane={lane({ stage: "tests", detected_stage: "implementing" })}
|
||||||
|
selected={false}
|
||||||
|
onSelect={() => {}}
|
||||||
|
/>
|
||||||
|
);
|
||||||
|
expect(screen.queryByTestId("lane-tile-auto-1")).toBeNull();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("still badges a detection ahead of an alias declaration", () => {
|
||||||
|
render(
|
||||||
|
<LaneStripCard
|
||||||
|
lane={lane({ stage: "tests", detected_stage: "review" })}
|
||||||
|
selected={false}
|
||||||
|
onSelect={() => {}}
|
||||||
|
/>
|
||||||
|
);
|
||||||
|
expect(screen.getByTestId("lane-tile-auto-1")).toHaveTextContent("review");
|
||||||
|
});
|
||||||
|
});
|
||||||
+16
-4
@@ -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.
|
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
|
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
|
leads the declared one (see "Viewing lanes" below), and `LaneStripCard.tsx`'s
|
||||||
`detectionLeadsDeclaration` shows its `auto: <stage>` chip under the identical
|
`detectionLeads` shows its badge under the identical condition — the terminal
|
||||||
condition — the terminal and the browser never disagree about which one is
|
and the browser never disagree about which one is the headline. Both locate the
|
||||||
the headline.
|
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
|
### Which lane a signal is credited to
|
||||||
|
|
||||||
@@ -881,6 +885,14 @@ contain whole code blocks:
|
|||||||
- `description`
|
- `description`
|
||||||
- `subagent_type`
|
- `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,
|
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
|
plus 24 characters of context on each side, with an ellipsis marking either end
|
||||||
that was trimmed. A shell line like
|
that was trimmed. A shell line like
|
||||||
|
|||||||
@@ -95,6 +95,29 @@ test("flattenInput handles awkward tool-input shapes without traversing deeply",
|
|||||||
assert.equal(flattenInput({ nested: { command: "never recurse" } }), "");
|
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", () => {
|
test("caps a long signal so it never inflates a DB column or a title attribute", () => {
|
||||||
const longCommand = `git push ${"x".repeat(4990)}`;
|
const longCommand = `git push ${"x".repeat(4990)}`;
|
||||||
const result = detect(pipeline, { tool_name: "Bash", tool_input: { command: longCommand } });
|
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 pipeline = getPipeline("ship-feature");
|
||||||
const cases = [
|
const cases = [
|
||||||
["intake", "Skill", { skill: "superpowers:brainstorming" }],
|
["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" }],
|
["intake", "Bash", { command: "ccam feature activate lane-stage-detect" }],
|
||||||
["plan", "Skill", { skill: "superpowers:writing-plans" }],
|
["plan", "Skill", { skill: "superpowers:writing-plans" }],
|
||||||
["plan", "Write", { file_path: "docs/superpowers/specs/lane-foo.md" }],
|
["plan", "Write", { file_path: "docs/superpowers/specs/lane-foo.md" }],
|
||||||
|
|||||||
+37
-14
@@ -51,11 +51,22 @@ function matchedSpan(regex, input) {
|
|||||||
return `${prefix}${input.slice(start, end)}${suffix}`;
|
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 {
|
try {
|
||||||
if (typeof toolInput === "string") return toolInput;
|
if (typeof toolInput === "string") return [toolInput];
|
||||||
if (toolInput === null || typeof toolInput !== "object") return "";
|
if (toolInput === null || typeof toolInput !== "object") return [];
|
||||||
|
|
||||||
const strings = [];
|
const strings = [];
|
||||||
if (Array.isArray(toolInput)) {
|
if (Array.isArray(toolInput)) {
|
||||||
@@ -72,12 +83,18 @@ function flattenInput(toolInput) {
|
|||||||
if (FLATTEN_KEYS.has(key) && typeof value === "string") strings.push(value);
|
if (FLATTEN_KEYS.has(key) && typeof value === "string") strings.push(value);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
return strings.join(" ");
|
return strings;
|
||||||
} catch {
|
} 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. */
|
/** Compile valid node rules once, omitting malformed nodes and regex patterns. */
|
||||||
function compileRules(pipeline) {
|
function compileRules(pipeline) {
|
||||||
if (!pipeline || typeof pipeline !== "object") return [];
|
if (!pipeline || typeof pipeline !== "object") return [];
|
||||||
@@ -122,19 +139,25 @@ function compileRules(pipeline) {
|
|||||||
function detect(pipeline, event) {
|
function detect(pipeline, event) {
|
||||||
try {
|
try {
|
||||||
if (!event || typeof event.tool_name !== "string" || !event.tool_name) return null;
|
if (!event || typeof event.tool_name !== "string" || !event.tool_name) return null;
|
||||||
const input = flattenInput(event.tool_input);
|
const fields = inputFields(event.tool_input);
|
||||||
const fallback = input || event.tool_name;
|
const fallback = fields.join(" ") || event.tool_name;
|
||||||
let match = null;
|
let match = null;
|
||||||
|
|
||||||
for (const node of compileRules(pipeline)) {
|
for (const node of compileRules(pipeline)) {
|
||||||
if (typeof node.nodeId !== "string" || !node.nodeId) continue;
|
if (typeof node.nodeId !== "string" || !node.nodeId) continue;
|
||||||
for (const rule of node.rules) {
|
for (const rule of node.rules) {
|
||||||
if (rule.tool !== event.tool_name) continue;
|
if (rule.tool !== event.tool_name) continue;
|
||||||
if (!rule.regex || rule.regex.test(input)) {
|
// A rule with no regex matched on the tool name alone.
|
||||||
const span = matchedSpan(rule.regex, input);
|
const hit = rule.regex
|
||||||
match = { nodeId: node.nodeId, signal: `\`${capSignal(span || fallback)}\`` };
|
? fields.find((f) => {
|
||||||
break;
|
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;
|
return match;
|
||||||
@@ -143,4 +166,4 @@ function detect(pipeline, event) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
module.exports = { flattenInput, compileRules, detect };
|
module.exports = { flattenInput, inputFields, compileRules, detect };
|
||||||
|
|||||||
Reference in New Issue
Block a user