fix(lanes): count every pipeline run, not one per lane
Lane Pipeline Pass Rates read `lanes.stages`, which is a snapshot of the
pass in flight: re-entering a node overwrites its record, and moving
backwards deletes every record ahead of the new stage. The report treated
one lane row as one run, so shipping ten features through a lane still
counted as one, and no node could ever exceed 1/1. History survived only
via `lane_features`, which is written when a feature is activated — a
path most users never take. The panel was not wrong, it was structurally
incapable of counting.
Add `lane_stage_events`, an append-only log written by every declaration
and every accepted detection, and aggregate from that. A run is one pass
through the template: the log starts a new one when the lane moves
backwards or leaves the template's last node. Existing snapshots are
seeded as one run each on first boot, so upgrading does not read as
"never run". Templates a lane is configured with are still listed at
zero — a pipeline going unused has to be visible as such.
Count inferred reaches too, in their own column and their own colour.
Detection stays out of the pass rate ("inference is never evidence"), but
reporting a stage the hooks demonstrably saw as "never reached" is what
made the panel look broken to anyone who works without running the
pipeline commands.
Also reject a stage naming no node of the lane's pipeline (400 EBADSTAGE,
listing the ones that exist) instead of storing it with a warning nobody
reads. A real lane had accumulated five such keys, one of them `test` on
a ship-feature lane whose CI node is `gates` — which is why that row read
never-reached while the stage had in fact been declared. Aliases still
resolve, so shorthand is unaffected.
This commit is contained in:
@@ -0,0 +1,137 @@
|
||||
/**
|
||||
* @file lane-stage-events.test.js
|
||||
* @description Tests for the append-only stage log behind Lane Pipeline Pass
|
||||
* Rates.
|
||||
*
|
||||
* The panel used to read `lanes.stages`, which is a snapshot of the current
|
||||
* pass: re-entering a node overwrote its record and moving backwards deleted
|
||||
* every record ahead. Ten features shipped through one lane therefore counted
|
||||
* as one run, and no amount of use ever moved the numbers. These tests pin the
|
||||
* two properties that fixes it — a pass is never overwritten, and passes are
|
||||
* separated into runs — plus the detected/declared split the panel draws.
|
||||
*
|
||||
* @author Nguyễn Ngọc Trí Vĩ <vinnt@smartgift.vn>
|
||||
*/
|
||||
|
||||
const { describe, it, before, beforeEach, after } = require("node:test");
|
||||
const assert = require("node:assert/strict");
|
||||
const fs = require("node:fs");
|
||||
const path = require("node:path");
|
||||
const os = require("node:os");
|
||||
|
||||
const TMP = fs.mkdtempSync(path.join(os.tmpdir(), "lane-stage-events-"));
|
||||
process.env.DASHBOARD_DB_PATH = path.join(TMP, "dashboard.db");
|
||||
|
||||
const { db } = require("../db");
|
||||
const lanesLib = require("../lib/lanes");
|
||||
|
||||
let laneId;
|
||||
|
||||
after(() => {
|
||||
try {
|
||||
fs.rmSync(TMP, { recursive: true, force: true, maxRetries: 5 });
|
||||
} catch {
|
||||
/* best-effort temp cleanup */
|
||||
}
|
||||
});
|
||||
|
||||
before(() => {
|
||||
db.prepare(
|
||||
"INSERT INTO lanes (title, cwd, pipeline, tool) VALUES ('t', ?, 'default', 'claude')"
|
||||
).run(path.join(TMP, "work"));
|
||||
laneId = db.prepare("SELECT id FROM lanes ORDER BY id DESC LIMIT 1").get().id;
|
||||
});
|
||||
|
||||
beforeEach(() => {
|
||||
db.prepare("DELETE FROM lane_stage_events WHERE lane_id = ?").run(laneId);
|
||||
db.prepare("UPDATE lanes SET stage = 'idle', stages = '{}' WHERE id = ?").run(laneId);
|
||||
});
|
||||
|
||||
const events = () =>
|
||||
db
|
||||
.prepare("SELECT node_id, run_seq, source, result, evidence FROM lane_stage_events ORDER BY id")
|
||||
.all();
|
||||
|
||||
describe("lane_stage_events", () => {
|
||||
it("records one event per declaration, including a node entered twice", () => {
|
||||
lanesLib.setStage(laneId, { stage: "plan" });
|
||||
lanesLib.setStage(laneId, { stage: "implement" });
|
||||
lanesLib.setStage(laneId, { stage: "implement", evidence: "second pass" });
|
||||
|
||||
const rows = events();
|
||||
assert.equal(rows.length, 3);
|
||||
assert.deepEqual(
|
||||
rows.map((r) => r.node_id),
|
||||
["plan", "implement", "implement"]
|
||||
);
|
||||
// The snapshot keeps one record for `implement`; the log keeps both.
|
||||
assert.equal(Object.keys(lanesLib.getLane(laneId).stages).length, 2);
|
||||
});
|
||||
|
||||
it("keeps the records of a finished pass when the lane moves backwards", () => {
|
||||
for (const stage of ["plan", "implement", "review"]) lanesLib.setStage(laneId, { stage });
|
||||
lanesLib.setStage(laneId, { stage: "plan" });
|
||||
|
||||
// The snapshot truncates ahead of `plan` — that is deliberate, it describes
|
||||
// the pass in flight. The log must not lose what already happened.
|
||||
const snapshot = Object.keys(lanesLib.getLane(laneId).stages);
|
||||
assert.deepEqual(snapshot.sort(), ["plan"]);
|
||||
assert.deepEqual(
|
||||
events().map((r) => r.node_id),
|
||||
["plan", "implement", "review", "plan"]
|
||||
);
|
||||
});
|
||||
|
||||
it("starts a new run when the lane moves backwards", () => {
|
||||
for (const stage of ["plan", "implement", "review"]) lanesLib.setStage(laneId, { stage });
|
||||
lanesLib.setStage(laneId, { stage: "plan" });
|
||||
lanesLib.setStage(laneId, { stage: "implement" });
|
||||
|
||||
assert.deepEqual(
|
||||
events().map((r) => `${r.node_id}#${r.run_seq}`),
|
||||
["plan#1", "implement#1", "review#1", "plan#2", "implement#2"]
|
||||
);
|
||||
});
|
||||
|
||||
it("starts a new run when the pipeline's last node is left behind", () => {
|
||||
lanesLib.setStage(laneId, { stage: "done" });
|
||||
lanesLib.setStage(laneId, { stage: "intake" });
|
||||
|
||||
assert.deepEqual(
|
||||
events().map((r) => `${r.node_id}#${r.run_seq}`),
|
||||
["done#1", "intake#2"]
|
||||
);
|
||||
});
|
||||
|
||||
it("carries the result and whether evidence was left", () => {
|
||||
lanesLib.setStage(laneId, { stage: "review", result: "fail" });
|
||||
lanesLib.setStage(laneId, { stage: "gate", evidence: "GO — 2 reviewers" });
|
||||
|
||||
const [review, gate] = events();
|
||||
assert.equal(review.result, "fail");
|
||||
assert.equal(review.evidence, 0);
|
||||
assert.equal(gate.result, null);
|
||||
assert.equal(gate.evidence, 1);
|
||||
});
|
||||
|
||||
it("records a detection as its own source, never as a declaration", () => {
|
||||
const wrote = lanesLib.recordDetection(laneId, { nodeId: "implement", signal: "`edit`" });
|
||||
assert.equal(wrote.written, true);
|
||||
|
||||
const rows = events();
|
||||
assert.equal(rows.length, 1);
|
||||
assert.equal(rows[0].source, "detected");
|
||||
assert.equal(rows[0].node_id, "implement");
|
||||
// A detection must not move the declared stage — same rule as before.
|
||||
assert.equal(lanesLib.getLane(laneId).stage, "idle");
|
||||
});
|
||||
|
||||
it("records nothing when a detection is rejected", () => {
|
||||
lanesLib.setStage(laneId, { stage: "review" });
|
||||
db.prepare("DELETE FROM lane_stage_events WHERE lane_id = ?").run(laneId);
|
||||
|
||||
const wrote = lanesLib.recordDetection(laneId, { nodeId: "plan", signal: "`read`" });
|
||||
assert.equal(wrote.written, false);
|
||||
assert.deepEqual(events(), []);
|
||||
});
|
||||
});
|
||||
@@ -88,6 +88,21 @@ describe("/api/lanes", () => {
|
||||
assert.equal(r.status, 409);
|
||||
});
|
||||
|
||||
it("rejects a stage that names no node of the lane's pipeline", async () => {
|
||||
// Silently storing it is what made "CI gates" read never-reached on a lane
|
||||
// where the agent had been declaring `test` (the default pipeline's name)
|
||||
// all along.
|
||||
const r = await request("POST", `/api/lanes/${laneId}/stage`, { stage: "implemnt" });
|
||||
assert.equal(r.status, 400);
|
||||
assert.equal(r.body.error.code, "EBADSTAGE");
|
||||
assert.ok(r.body.error.stages.includes("implement"));
|
||||
});
|
||||
|
||||
it("accepts a stage declared by one of the node's aliases", async () => {
|
||||
const r = await request("POST", `/api/lanes/${laneId}/stage`, { stage: "coding" });
|
||||
assert.equal(r.status, 200);
|
||||
});
|
||||
|
||||
it("reports a stage and returns node states", async () => {
|
||||
const r = await request("POST", `/api/lanes/${laneId}/stage`, {
|
||||
stage: "review",
|
||||
|
||||
@@ -203,29 +203,29 @@ describe("agenthub stage", () => {
|
||||
assert.match(`${r.stdout}${r.stderr}`, /no lane/i);
|
||||
});
|
||||
|
||||
it("warns, but still records, a stage name matching no pipeline node", async () => {
|
||||
// setStage stores the string verbatim, so a typo'd stage is accepted and
|
||||
// then renders nowhere (phaseIdx -1, progress 0). Silent acceptance is how
|
||||
// a lane ends up looking unstarted for an entire pipeline run.
|
||||
it("refuses a stage name matching no pipeline node", async () => {
|
||||
// It used to be recorded with a warning, which renders nowhere (phaseIdx
|
||||
// -1, progress 0) and counts nowhere. A warning on stderr proved easy to
|
||||
// miss — a real lane accumulated five such names — so the server rejects
|
||||
// it and the agent is told which names exist.
|
||||
const r = await cli(["stage", "revieww"], LANE_DIR);
|
||||
assert.equal(r.status, 0, r.stderr);
|
||||
assert.match(r.stdout, /revieww/);
|
||||
assert.match(r.stderr, /matches no node/i);
|
||||
assert.match(r.stderr, /revieww/);
|
||||
assert.notEqual(r.status, 0);
|
||||
const out = `${r.stdout}${r.stderr}`;
|
||||
assert.match(out, /revieww/);
|
||||
assert.match(out, /not a stage/i);
|
||||
assert.match(out, /review/);
|
||||
});
|
||||
|
||||
it("does not warn for a stage declared by alias", async () => {
|
||||
it("accepts a stage declared by alias", async () => {
|
||||
const r = await cli(["stage", "planning"], LANE_DIR);
|
||||
assert.equal(r.status, 0, r.stderr);
|
||||
assert.doesNotMatch(r.stderr, /matches no node/i);
|
||||
});
|
||||
|
||||
it("does not warn for a valid stage reported as failed", async () => {
|
||||
// --result fail paints the node `failed`, not `current`; the warning must
|
||||
// not read that as an unknown stage.
|
||||
it("accepts a valid stage reported as failed", async () => {
|
||||
// --result fail paints the node `failed`, not `current`; the validation
|
||||
// must not read that as an unknown stage.
|
||||
const r = await cli(["stage", "review", "--result", "fail"], LANE_DIR);
|
||||
assert.equal(r.status, 0, r.stderr);
|
||||
assert.doesNotMatch(r.stderr, /matches no node/i);
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -22,6 +22,7 @@ const assert = require("node:assert/strict");
|
||||
|
||||
const { createApp } = require("../index");
|
||||
const lanesLib = require("../lib/lanes");
|
||||
const laneFeatures = require("../lib/lane-features");
|
||||
const { db, stmts } = require("../db");
|
||||
|
||||
let server;
|
||||
@@ -61,8 +62,14 @@ function makeLane() {
|
||||
return lanesLib.createLane({ title: `lane ${laneSeq}`, cwd, kind: "managed" });
|
||||
}
|
||||
|
||||
function setStages(laneId, stages) {
|
||||
db.prepare("UPDATE lanes SET stages = ? WHERE id = ?").run(JSON.stringify(stages), laneId);
|
||||
// Declare stages the way an agent does. Poking `lanes.stages` directly would
|
||||
// test the snapshot the pass-rate report no longer reads: that blob describes
|
||||
// the pass in flight, while the report counts the append-only stage log every
|
||||
// declaration writes.
|
||||
function declare(laneId, stages) {
|
||||
for (const [stage, rec] of Object.entries(stages)) {
|
||||
lanesLib.setStage(laneId, { stage, evidence: rec.evidence, result: rec.result });
|
||||
}
|
||||
}
|
||||
|
||||
function node(pipelineStats, pipelineId, nodeId) {
|
||||
@@ -73,10 +80,10 @@ function node(pipelineStats, pipelineId, nodeId) {
|
||||
describe("GET /api/lanes/pipeline-stats", () => {
|
||||
it("counts a pass with evidence apart from a pass without one, and a fail as neither", async () => {
|
||||
const lane = makeLane();
|
||||
setStages(lane.id, {
|
||||
plan: { enteredAt: "2026-01-01T00:00:00Z", evidence: "plan.md", result: null },
|
||||
implement: { enteredAt: "2026-01-01T01:00:00Z", evidence: null, result: null },
|
||||
tests: { enteredAt: "2026-01-01T02:00:00Z", evidence: null, result: "fail" },
|
||||
declare(lane.id, {
|
||||
plan: { evidence: "plan.md", result: null },
|
||||
implement: { evidence: null, result: null },
|
||||
tests: { evidence: null, result: "fail" },
|
||||
});
|
||||
|
||||
const res = await get("/api/lanes/pipeline-stats");
|
||||
@@ -106,33 +113,35 @@ describe("GET /api/lanes/pipeline-stats", () => {
|
||||
assert.equal(node(res.body, "default", "ship").entered, 0);
|
||||
});
|
||||
|
||||
it("keeps counting an archived feature's stages after the lane moves on", async () => {
|
||||
it("keeps a finished feature's run after the lane starts the next one", async () => {
|
||||
// The regression this guards: the report used to read the lane's live
|
||||
// `stages` blob, which the next feature overwrites. Ship two features
|
||||
// through one lane and the first one's gate had never happened.
|
||||
const lane = makeLane();
|
||||
db.prepare(
|
||||
`INSERT INTO lane_features (lane_id, slug, pipeline, stage, stages, archived_at)
|
||||
VALUES (?, 'old', 'default', 'done', ?, '2026-01-02T00:00:00Z')`
|
||||
).run(lane.id, JSON.stringify({ gate: { enteredAt: "x", evidence: "GO", result: null } }));
|
||||
declare(lane.id, { gate: { evidence: "GO" } });
|
||||
laneFeatures.activateFeature(lane.id, "next-feature");
|
||||
declare(lane.id, { plan: { evidence: null } });
|
||||
|
||||
const res = await get("/api/lanes/pipeline-stats");
|
||||
const res = await get(`/api/lanes/pipeline-stats?lane=${lane.id}`);
|
||||
const gate = node(res.body, "default", "gate");
|
||||
assert.equal(gate.passed, 1);
|
||||
assert.equal(gate.withEvidence, 1);
|
||||
assert.equal(node(res.body, "default", "plan").passed, 1);
|
||||
// Two passes through the pipeline, counted as two runs rather than one lane.
|
||||
assert.equal(res.body.pipelines.find((p) => p.id === "default").runs, 2);
|
||||
});
|
||||
|
||||
it("narrows to one lane and its own archived features with ?lane=", async () => {
|
||||
it("narrows to one lane and all of its own runs with ?lane=", async () => {
|
||||
const mine = makeLane();
|
||||
const other = makeLane();
|
||||
setStages(mine.id, { plan: { enteredAt: "x", evidence: "plan.md", result: null } });
|
||||
setStages(other.id, { plan: { enteredAt: "x", evidence: null, result: null } });
|
||||
db.prepare(
|
||||
`INSERT INTO lane_features (lane_id, slug, pipeline, stage, stages, archived_at)
|
||||
VALUES (?, 'mine-old', 'default', 'done', ?, '2026-01-02T00:00:00Z')`
|
||||
).run(mine.id, JSON.stringify({ plan: { enteredAt: "x", evidence: "old.md", result: null } }));
|
||||
declare(mine.id, { plan: { evidence: "plan.md" }, done: {} });
|
||||
declare(mine.id, { plan: { evidence: "again.md" } });
|
||||
declare(other.id, { plan: { evidence: null } });
|
||||
|
||||
const res = await get(`/api/lanes/pipeline-stats?lane=${mine.id}`);
|
||||
assert.equal(res.status, 200);
|
||||
const plan = node(res.body, "default", "plan");
|
||||
// The lane itself plus its one archived feature — never the other lane's.
|
||||
// Both of this lane's passes — never the other lane's.
|
||||
assert.deepEqual(
|
||||
{ entered: plan.entered, withEvidence: plan.withEvidence },
|
||||
{ entered: 2, withEvidence: 2 }
|
||||
|
||||
Reference in New Issue
Block a user