Repository navigation
Conversation
|
Review (Birfy, 2026-10-20 20:07 Asia/Shanghai): PASS (code) — held for the owner's confirmation of #534 Tip SHA: d423209
Findings (non-blocking, for the owner's decision):
#534 is a new feature the owner has not confirmed yet, so this is not merged regardless of tier. It shares the same |
|
@HugoXK Owner decision (2026-10-20 21:09 Asia/Shanghai): accepted. Required change: also show the waiting-longest (stuck) list in the dashboard/panel, not only in timeline/window. |
|
Waiting for approval. It needs (1) the owner's approval through the GitHub UI (a chat reply does not count) and (2) a review PASS at the exact tip SHA; any new commit voids both. Automation does not merge T2/T3 by itself and does not approve on the owner's behalf. |
Birfy
left a comment
There was a problem hiding this comment.
PASS — tip d42320935879be7e9b82cd02db5d105aa904482c (Maintainer review, acting as Birfy)
Design check: no change to Bot authorization, to the approval path (still a UI button only) or to user-facing English text; the new surface is read-only and part of the existing answer.
Local evidence: I applied this PR's diff to my local main snapshot and ran the touched tests there (TZ=UTC), plus node scripts/ds-bot-patches.mjs check (with the new files staged, as a commit would):
plugins/timeline:node --import ./test/resolve.mjs --test test/window.test.js→ 22/22 passnode scripts/ds-bot-patches.mjs check→ ok
stuck keeps only open handoffs, sorts worst-first, caps at 20, and waitedMs is floored at 0.
Not run locally: the full npm run verify in one shot (this box has a 2 GiB memory cap, so I run the steps one at a time), the smoke test, Desktop, and the real-model steward.
CI: tests-ok is green at the tip SHA above (GitHub Actions ran the full suite there).
Tier: T2/T3, so this PASS does not approve the PR: the owner still has to approve it through the GitHub UI, and any new commit voids both the approval and this PASS. Not merged by automation.
d423209 to
77725ec
Compare
Birfy
left a comment
There was a problem hiding this comment.
Verdict: PASS at 77725ec
Tip SHA: 77725ec6be0183901e98062116e56b5f45943e5e
The tip moved (rebased onto current main); the PASS at d4232093 is void. Read the whole new diff: plugins/timeline/src/window.js (+9 -1, a `stuck` list of open waits inside the existing timeline/window answer), test/window.test.js (+8), one docs/subsystems/timeline.md row, one changelog fragment. Nothing else changed.
Design: read-only and additive — `stuck` is another field of the same answer, worst-first, capped at 20, waitedMs floored at 0, ids/labels from the existing cleaners; no new endpoint, no config, no user-facing string, no new approval path.
TDD: the new test reads `win.stuck`, which does not exist at the base window.js — 0 pass / 1 fail with only window.js reverted, and it passes with the change.
Ran at the tip: CI tests-ok green at this SHA (test, plugins tree/social/timeline, package all success). On a copy of current main with this diff applied: plugins/timeline 96/96 (TZ=UTC), changelog.mjs check OK.
Not run: one-shot npm run verify (2 GiB OOM), core suite, smoke, Desktop.
Tier: T2. Awaiting the owner's click in the GitHub UI plus this PASS at the tip. Any new commit voids this PASS. Note: #531/#535/#539 all touch the same return line in window.js and the same docs row, so after one of them merges the other two will need a rebase, not a new PASS at a stale tip.
|
Do not approve this tip — it no longer merges. The PASS at (Same for #539 and #568: all three carry a row of the same timeline doc table, so after |
The window now reports the open handoffs, worst first, with how long each has waited, so a stuck Bot-to-Bot request is visible instead of only counted. Read-only, part of timeline/window, ids only.
77725ec to
c13ba07
Compare
Birfy
left a comment
There was a problem hiding this comment.
Verdict: PASS at c13ba07
Reviewed again at this tip: the branch was rebased onto main 33fb55ed, so the previous PASS (77725ec6) is void.
Diff (4 files, +19 -2): `plugins/timeline/src/window.js` adds `stuck` (open request arrows with `waiting`, worst first, capped at 20, `waitedMs` floored at 0) to the window it already builds; the new test in `plugins/timeline/test/window.test.js`; the `window.js` row of `docs/subsystems/timeline.md` now reads "...per-Bot outcomes and recent failures (#471) and the open requests waiting longest (#469)" — the rebase kept #531's wording; new `changelog.d/timeline-stuck.added.md`.
The rebase carried no behaviour change: the only difference from the version I reviewed at 77725ec6 is the docs row, which now sits on #531's line.
Ran at the tip: CI tests-ok green (test, plugins tree/social/timeline, package, Node 24). On a copy of this PR's own base with the whole diff applied: `ds-bot-patches.mjs check` OK (10 patches, core == ds-bot + patches), `check-arch-docs.mjs` OK (4 files), `changelog.mjs check` OK (12 fragments), plugins/timeline 97/97, `scripts/lint-plugins.mjs timeline` 0 errors / 8 warnings, TZ=UTC.
TDD: with only `window.js` at the base commit, the new "stuck" test fails and the rest of window.test.js 23/24 pass; all 97 pass with the change.
Conflicts: with #539 (both edit the same return line of `window.js`, the same `timeline.md` row and the same test file) and with #568 (`timeline.md`). Clean with main, #566, #572, #574, #578, #580. Only one of the timeline trio (#535, #539, #568) can land per cycle; whoever does not land needs a rebase and a fresh PASS.
Not run: one-shot npm run verify (2 GiB OOM), full core suite, smoke, Desktop, a real model call.
Merge matrix (local 3-way merges against main 33fb55e and against every other open PR): see the lines above; every claim is measured, not assumed.
Tier: T2 (plugins/**, core/src/host/**, core/types/**). The owner's own click in the GitHub UI is the approval and this PASS must sit on the tip; I do not approve or merge it myself. Any new commit voids this PASS.
|
One precision on the PASS above: I could not byte-diff the pre-rebase branch (the force-push removed the old commit), so "the only difference is the docs row" is my reading of the source hunks against my review notes for the old tip, not a diff I could run. What is verified at the new tip is everything else in the PASS: the full check set, the plugin test suite and the red-without-fix run were all redone here. |
|
Superseded by #586 (one combined timeline change, at main The maintenance round merged this PR's change, its test and its docs wording into #586 together with #535 and #568, because all three edit the same Closing this PR as superseded. Thank you for the original — the tests and the docs row came through unchanged. |
…lowLab#586) Fixes ThinkFlowLab#534, Fixes ThinkFlowLab#538, Fixes ThinkFlowLab#567 **Risk: T2** (`plugins/timeline/**`). ## What One PR for the three timeline read-model proposals that all touch the same return line of `plugins/timeline/src/window.js` and the same `docs/subsystems/timeline.md` row, so the owner gets one PASS and one click instead of three rebases: - `stuck`: the open request arrows, worst first (`waitedMs` floored at 0, capped at 20) — ThinkFlowLab#534 / ThinkFlowLab#469. - `toolUse`: per-Bot tool calls by cleaned name with their failure counts — ThinkFlowLab#538 / ThinkFlowLab#359. - `timeline/health`: new pure `plugins/timeline/src/health.js` (`buildHealth(window)`) plus the read-only endpoint, per-Bot signals and flags and a team `ok` — ThinkFlowLab#567 / ThinkFlowLab#357. Both new window fields are additive; the window version and every existing field are unchanged. Host-side has no copy of this code (the timeline is a plugin), so `patches/ds-bot/` is untouched and `ds-bot-patches.mjs check` still passes. ## Supersedes Closes ThinkFlowLab#535, ThinkFlowLab#539 and ThinkFlowLab#568 as superseded: their content, review and tests are all carried here, including the authorship trail in the three PRs. Thanks to HugoXK for the three originals. ## Checks (run by the maintenance round on this tree) - `ds-bot-patches.mjs check` — OK (10 patches, core == ds-bot + patches). - `check-arch-docs.mjs` with `ARCH_DOCS_BASE=HEAD~1` — OK (7 changed files). - `changelog.mjs check` — OK (12 fragments). - `plugins/timeline` tests — 101/101. - `scripts/lint-plugins.mjs timeline` — 0 errors, 8 warnings (all pre-existing). - TDD: with `window.js` and `timeline.js` at the base commit and `health.js` absent, 3 tests fail (the two new window tests and the health test file); all pass with the change. Not run: one-shot `npm run verify` (the container OOMs at 2 GiB), full core suite, smoke, Desktop.
…inkFlowLab#590) **Risk: T3** (`AGENTS.md` and the policy docs). The owner's new hard rule: when an issue already has a pull request, work in that one — do not open a second and close the first. This records it next to the admission line, so the two are read together. ## What changes (4 files, +7 -1) - `AGENTS.md`: a new paragraph in the admission-line section — re-base the open branch, use `update-branch`, or push a commit to its head, and put the tests there too; a contributor's fork is treated the same while it allows maintainer edits; the only exception is a branch that cannot be pushed, and then the reason goes on the issue, never a silent swap. - `docs/code-standards.md`: the same rule as a bullet under "Merge only a reviewed tip". - `docs/autonomous-dev.md`: one sentence in the merge-by-tier item. - `.claude/skills/fix-issue/SKILL.md`: a bullet in step 4 (PR). It applies to new work from now on: ThinkFlowLab#586 and its closed originals (ThinkFlowLab#535/ThinkFlowLab#539/ThinkFlowLab#568) are left exactly as they are, as the Chief asked. ## Checks - `node --test .github/scripts/*.test.mjs` — 93/93. - `check-arch-docs.mjs` with `ARCH_DOCS_BASE=HEAD~1` — OK (4 changed files). - `scripts/ds-bot-patches.mjs check` — OK (no code touched). - `changelog.mjs check` — OK; no fragment, a change users cannot see and that ships no code. - `TZ=UTC`. Not run: one-shot `npm run verify` (2 GiB OOM in this container), full core suite, smoke, Desktop.
Timeline: list the requests waiting longest
#469 wants a "waiting too long" signal. The timeline window flagged waiting requests per edge but never listed them, so a stuck Bot-to-Bot handoff was easy to miss.
timeline/windownow carriesstuck: the open requests, sorted by waited time, worst first (up to 20), each with its ids, role andwaitedMs. Read-only, ids only.Ran at the tip:
npm run verify(green,TZ=UTC),node scripts/lint-plugins.mjs timeline(0 errors). Not run: smoke, Desktop, real-model steward.