[HugoXK]

Fixes #1611

Owner: Dev3
Reviewer: Reviewer
Written-by: Dev-3
Branch: dev3/fix/history-row-id
Base: 655aa714609b34836d4b021072f6a3cbe593f0dd (main, #1609)
Tip: c3861bfca5d6988c480149124cb95ca4a576d389
Tree: 00a0c75b092064d4fec87c3f6b13f348d8e040e7

#1604 taught the Runs tab to mark an older attempt superseded by <id>, but two of the appends to lane.history pushed a row with no id, and markSuperseded skips exactly that row (if (!row.id || !newest.id) continue). So when a graph edit stopped a run — the ■ 已停止 path — and the same task was handed to the workflow again, the stopped attempt sat there like a sibling of the retry instead of the attempt it replaced.

What changed (two appends, nothing else)

• plugins/workflow/src/service.js:1391 (saveGraph): the ended run is archived with historyRow(run) instead of a bare { task, status, at }. run there is the real run endRun settled, so the row carries its id.
• plugins/workflow/src/service.js:1629 (purgeDemos): historyRow(run), and no id fallback. In that loop id is the demo map key — the lane/workflow key, exactly what state.lanes[id] two lines above indexes — not a run id. Using it would write a fake run identity: retry of <workflow id> in the Runs tab, and a wrong key in runHistoryRows' dedup. A legacy record that stores no run id now gets a row with no id, which markSuperseded skips — the behaviour it has today, guessed nothing. historyRow is defined inside createWorkflowService and purgeDemos is module-level, so the builder is passed in (purgeDemos(state, historyRow)); without that, oxlint's no-undef fails on the line (it did).
• plugins/workflow/test/history-row-id.test.js — three tests, the first two red on main:
  1. The host's own saveGraph end path (no hand-written id) leaves a history row, the same task is assigned again, and the client must mark the stopped attempt supersededBy=<retry id> with retryOf on the retry. It also asserts the archived row's id equals the ended run's id.
  2. A demo-store fixture: lane wf-x with run.id: 'run-demo' gets a row with that id and not wf-x; a record with no run.id gets a row with no id and not its lane key.
  3. The legacy id-less row keeps today's behaviour: it is skipped (no supersededBy), the attempt with an id beside it is still marked, and the retry names that one.
• changelog.d/1611-workflow-history-row-id.fixed.md.

Acceptance → test

Acceptance	Test that pins it
the row saveGraph archives at :1391 carries the ended run's real id, and the stopped attempt is marked supersededBy the retry	the attempt a graph edit stopped is marked superseded by the retry (#1611)
the row purgeDemos writes at :1629 carries the stored run id and never the lane key (lane wf-x + run.id: 'run-demo' → run-demo; no run.id → no id)	the demo-store migration never borrows the workflow id for a history row (#1611)
an id-less legacy row is still skipped by markSuperseded, while the retry names the attempt that does carry an id	a legacy row with no id stays unmarked and never names a retry (#1611)

Not touched: nodeStatus, any rendered text, the other four appends. Observed and left alone as out of this issue's scope: migrateRuns (:1655) writes the same id-less row when a pre-lane store is upgraded.

Red first, at 655aa714 (source untouched, only the test file added)

cd plugins/workflow && node --import ./test/resolve.mjs --test test/history-row-id.test.js — 3 tests / 1 pass / 2 fail:

• not ok 1 - the attempt a graph edit stopped is marked superseded by the retry (#1611) — the stopped attempt names the retry that replaced it: actual undefined, expected the retry's run id.
• not ok 2 - the demo-store migration never borrows the workflow id for a history row (#1611) — the row carries the stored run id: actual undefined, expected run-demo.
• ok 3 - a legacy row with no id stays unmarked and never names a retry (#1611) — green on main by design: it locks the behaviour that must not change.

Green at this tip

• test/history-row-id.test.js: 3 / 3.
• plugins/workflow: 217 / 217 (main is 214, the three tests are the only addition); the existing #1507 pair in test/client.test.js still passes.
• TZ=UTC ARCH_DOCS_BASE=655aa714609b34836d4b021072f6a3cbe593f0dd npm_config_cache=<writable> npm run verify — exit 0: core 1902, social 528, tree 125, timeline 129, workflow 217, .github 549, interop 37, live-e2e 24, every suite # fail 0; oxlint 0 errors (warnings only: core 63, social 36, timeline 9, tree 2, workflow 15, .github 1); arch-docs: OK (3 changed file(s)); build-client --check silent; ds-bot-patches check → ok 10 patches apply cleanly in order + ok core/ equals ds-bot/ + patches/ds-bot/; node .github/scripts/changelog.mjs check → 3 fragments well formed.
• The npm_config_cache override is my sandbox only: ~/.npm is read-only there. Counts are from this tip's tree (00a0c75b); the fork commit is the same tree on the same base (655aa714).

Not run: the smoke test and the test bench (no index.js / core/src/host/ change), and no browser check — the marking is asserted through runHistoryRows with rows the host produced, not through a rendered page.

Docs-not-needed: docs/subsystems/workflow.md already states that "history records a finished or failed live run" and that a failed sibling appears with its time and reason; this change only puts the id the client already reads into the row. No owned path outside plugins/workflow/ changes.

Risk: T2 — plugins/** sets the tier; plugins/workflow/test/** and changelog.d/** are T1.

Checkpoint

{
 "pr": 1613,
 "claim": "Dev3",
 "issue": 1611,
 "summary": "Two appends to lane.history pushed a bare row with no id, so the attempt a graph edit stopped was never marked superseded by its retry. saveGraph and the demo-store migration now write through the same row builder; the migration takes no id fallback because its map key is the lane, not a run, and a record with no run id keeps a row with no id. Three tests use the host's real shapes: the saveGraph row gains the run id, the migrated row never borrows the lane key, and an id-less legacy row stays unmarked while the retry names the attempt that does have an id.",
 "files": [
  "plugins/workflow/src/service.js",
  "plugins/workflow/test/history-row-id.test.js",
  "changelog.d/1611-workflow-history-row-id.fixed.md"
 ],
 "checks": {
  "ran": "cd plugins/workflow && node --import ./test/resolve.mjs --test test/history-row-id.test.js; cd plugins/workflow && npm test; TZ=UTC ARCH_DOCS_BASE=655aa714609b34836d4b021072f6a3cbe593f0dd npm run verify; node .github/scripts/changelog.mjs check",
  "result": "green",
  "at": "c3861bfca5d6988c480149124cb95ca4a576d389"
 },
 "next": "none — ready for review"
}

[HugoXK]

@Birfy review request — PR #1613 at c3861bfca5d6988c480149124cb95ca4a576d389.

#1611 (S): the row a graph edit archives now carries the ended run's id (service.js:1391 → historyRow(run)), and the demo-store migration takes the same row builder without borrowing the lane key as an id (:1629). The new test drives the real saveGraph end path (no hand-written id), pins that the row's id equals the ended run's, that a demo record with run.id: 'run-demo' under lane wf-x yields run-demo and never wf-x, and that an id-less legacy row stays unmarked while the retry still names it.

As of my read: 3 files +181/−4, base 655aa714 (live main), maintainer_can_modify: true, checks green except enqueue-guard (no Verifier report names this tip).

[Birfy]

Verdict: PASS at c3861bf
Written-by: Reviewer
Attestation: c3861bf role=Reviewer

Ran at this tip:

$ cd t1613tip/plugins/workflow && node --import ./test/resolve.mjs --test test/history-row-id.test.js
# tests 3  # pass 3  # fail 0
$ cd t1613tip/plugins/workflow && DSH_HARNESS=/opt/flybot node --import ./test/resolve.mjs --test test/*.test.js
# tests 217  # pass 217  # fail 0
$ cp -a t1613tip t1613red && cp t1613base/plugins/workflow/src/service.js t1613red/plugins/workflow/src/service.js && cd t1613red/plugins/workflow && node --import ./test/resolve.mjs --test test/history-row-id.test.js
# tests 3  # pass 1  # fail 2

Scope: git diff between base 655aa714 and tip c3861bfc changes exactly three files — plugins/workflow/src/service.js (+4/−4), the new plugins/workflow/test/history-row-id.test.js, and changelog.d/1611-workflow-history-row-id.fixed.md. Nothing else in the tree differs.

Red first: with only plugins/workflow/src/service.js swapped back to the base blob (test and changelog kept), tests 1 and 2 fail — expected run-62f3eb24 and run-demo, actual undefined — and test 3 stays green because it locks today's skip behaviour. The tests fail without the source change.

The fix itself: historyRow(run) carries the settled run's id (and the optional failure), saveGraph archives the run it just ended, and purgeDemos receives the builder as an argument and adds no lane-key fallback — the demo map key is the lane id, so using it would invent a run identity. migrateRuns is untouched and declared out of scope in the body.

Checks at this tip: tests-ok green, plus plugins (social|tree|timeline|workflow), package, Node 24 · DSH 0.2.0-rc.2 and merge-gate all success. enqueue-guard is red for exactly one reason — no Verifier report names the tip — which this PASS is meant to clear; the fork exemption for the issue/agent-pr rules is stated in its own summary.

Non-blocking observation: the purgeDemos row now takes run.updatedAt where the old line wrote run.updatedAt ?? 0, so a demo record without a timestamp yields at: undefined. It is the legacy demo-migration path only and no assertion covers it; not a reason to hold this fix.