From 10fdaa33faf503dd456991b1dcc66aa14a539476 Mon Sep 17 00:00:00 2001 From: Dylan Trotter Date: Tue, 27 Oct 2026 20:51:31 +0000 Subject: [PATCH 1/2] stewards: stop hiding steward, worker and reviewer threads (BB-93) --- plugins/stewards/DESIGN.md | 45 ++--- plugins/stewards/README.md | 15 +- plugins/stewards/server.test.ts | 227 +++++++--------------- plugins/stewards/server.ts | 126 ++++-------- plugins/stewards/skills/stewards/SKILL.md | 7 +- 5 files changed, 131 insertions(+), 289 deletions(-) diff --git a/plugins/stewards/DESIGN.md b/plugins/stewards/DESIGN.md index 192f704..109ddc9 100644 --- a/plugins/stewards/DESIGN.md +++ b/plugins/stewards/DESIGN.md @@ -105,9 +105,9 @@ These were settled while designing; each changes what gets built. - **Stewards end their own threads; the rest of the sidebar is the user's.** Stewards pin nothing and create no sidebar sections. A finished steward says so in its last turn and the plugin archives it when the binding ends; - a later wake brings it back. A steward waiting on someone else is - hidden between wakes too, and its finished workers and their reviewers are - hidden with it (§5, "Waiting"). thread-briefs is asked, + a later wake brings it back. A steward waiting on someone else stays in + the sidebar between wakes too, and its finished workers and their reviewers + stay with it (§5, "Waiting"). thread-briefs is asked, through each thread's metadata, not to archive or rename stewards and not to group the project steward: between wakes a steward reads done while its entity is still open, and its title carries the entity key. (This @@ -270,20 +270,14 @@ plugins. - **Review stewards** end when the pull request is merged or closed. - **Waiting.** A wake turn whose report names `other` as the next actor (a review posted and the author's move, a ticket in review waiting on - reviewers) hides the steward's thread when the turn ends - (`threads.update { visibility: "hidden" }`). The binding stays, the thread - stays addressable, and the next wake reaches it. It is made visible again - when it needs the user — the report puts the user next, an escalation is - open, a question is unanswered, or a needs item is on the Stewards page — - and stays hidden across "nothing changed" wakes. A steward is shown again - when a worker of its binding starts or runs, so the sidebar's effort stays - rooted at it instead of dropping the worker to a bare row under Threads. - Hiding cascades to the - steward's finished workers and their reviewers, so a row is not left - behind without its steward; a running worker is unaffected, and showing the - steward again shows them. The open-escalation rule stays. A report that - needs nobody is also marked read (`threads.markRead`), so the unread dot - means "needs you"; + reviewers) leaves the steward's thread in the sidebar. The binding stays, + the thread stays addressable, and the next wake reaches it. A steward is + not hidden between wakes, and its finished workers and their reviewers are + not hidden with it either: hiding made the stewards sidebar drop a waiting + or held steward from the list entirely (BB-93). The report is marked read + (`threads.markRead`) when nothing needs the user, so the unread dot means + "needs you"; a report that needs the user is left unread. The + open-escalation rule stays. binding end still archives the thread. Workers stay lifecycle-owned: they are the steward's children in bb, and the plugin stops them explicitly only when the binding ends (a detach, or a terminal state without @@ -874,12 +868,10 @@ deployment: # where a merge is confirmed live (BB-53); omi would take every steward, worker and reviewer under it. - Whether a steward is waiting on you, waiting on someone else, or done is thread-briefs' judgment from its transcript. A steward waiting on someone - else is hidden (`threads.update { visibility: "hidden" }`), not archived, - so it leaves the sidebar but stays addressable; its finished workers and - their reviewers are hidden with it, and a wake that needs you makes them - visible again. A running worker is not hidden. The unread dot is cleared - after a report that needs nobody. Ended bindings are archived by this - plugin, not by thread-briefs. + else is left in the sidebar, not archived and not hidden (BB-93): it stays + addressable, and its finished workers and their reviewers stay visible with + it. The unread dot is cleared after a report that needs nobody. Ended + bindings are archived by this plugin, not by thread-briefs. ## 16. Architecture @@ -1028,7 +1020,7 @@ goes wrong there into Linear tickets for the project steward to adopt. ticket linking the old one. Every `requeue-default` the steward takes a turn whatever the look found; a healthy one is cheap, a wake that says "all healthy" and a - report with actor `other`, which hides the thread until the next + report with actor `other`, which leaves the thread visible until the next look. A sweep's look between requeues wakes it only for a new or returned problem. - **Triage is the project steward's.** The steward files one ticket per new @@ -1529,7 +1521,10 @@ project dropdown in the live view; the sections top to bottom are **Blocked** and Sentry bindings, collapsed), **Recently ended** (collapsed), and the project steward demoted to one small row at the bottom. Every effort and thread row carries bb's own thread actions context menu, so archive, pin, rename, -split and delete still work. +split and delete still work. Held and waiting stewards are ordinary rows +here: the plugin does not hide a thread between wakes (BB-93), so a steward +waiting on someone else shows in **Blocked** or **In flight** rather than +dropping out of the host's live feed. **Two-line effort rows and the four glyphs (BB-76).** An effort row is two lines. Line one is the glyph, the entity key in bold, and the title, which diff --git a/plugins/stewards/README.md b/plugins/stewards/README.md index dd2c499..9e7dbfe 100644 --- a/plugins/stewards/README.md +++ b/plugins/stewards/README.md @@ -157,14 +157,10 @@ bb. [DESIGN.md](DESIGN.md) is the design; this is how it is built and run. Reopening, or re-adding the label, wakes the same thread, bringing it back from the archive; an entity that leaves its terminal state before the binding has ended takes the ending back. A steward whose wake turn reports - `actor: other` (waiting on a reviewer or the author) is hidden rather than - archived: it leaves the sidebar's organisation and unread attention but - stays addressable, so a later wake can reach it. It is made visible again - when it needs you (the report puts you next, an escalation is open, or a - question is unanswered) and stays hidden across "nothing changed" wakes. - Hiding cascades to its finished workers and their reviewers, so no row is - left behind without its steward; a running worker is unaffected, and - showing the steward again shows them. Binding end still archives. + `actor: other` (waiting on a reviewer or the author) stays in the sidebar + rather than being hidden or archived: it stays addressable, so a later wake + reaches it, and its finished workers and their reviewers stay visible with + it. Binding end still archives. - **Finished workers and reviewers.** A worker whose turn ends is marked `finished`; once the reconcile that consumed its report has woken the steward, its thread is archived, so the sidebar keeps only work in flight. @@ -196,8 +192,7 @@ bb. [DESIGN.md](DESIGN.md) is the design; this is how it is built and run. section when it is spawned, and a worker or reviewer the plugin did not place is filed there by the `thread.created` handler. A finished worker leaves the sidebar once its report is consumed, and a consumed reviewer - once its round is; a waiting steward's finished workers and their reviewers - hide with it and come back when it does. The unread dot + once its round is. The unread dot means "needs you": a report that waits on someone else, with no escalation, question or needs item, is marked read. A report that needs you is left unread. diff --git a/plugins/stewards/server.test.ts b/plugins/stewards/server.test.ts index ea562b9..3a4812c 100644 --- a/plugins/stewards/server.test.ts +++ b/plugins/stewards/server.test.ts @@ -2346,57 +2346,40 @@ describe("pull requests", () => { describe("stewards waiting on others", () => { const waiting = { phase: "open", summary: "Reviewed.", nextAction: "Sam addresses the review", actor: "other", blockedOn: "sam" }; - it("hides a review steward after a wake turn that ends waiting, and keeps it hidden until a wake needs the user", async () => { + it("keeps a review steward visible after a wake turn that ends waiting", async () => { const t = await setup(); t.entities.set("acme/web#7", pull()); await t.ring("github", "acme/web#7"); const steward = t.spawns[1]!.id; const report = await t.tool("steward_report", waiting, steward); - expect(report.text).toMatch(/Your thread is hidden when the turn ends/u); + expect(report.text).not.toMatch(/hidden/u); expect(t.threads.get(steward)!.visibility).toBe("visible"); await t.idle(steward); - expect(t.threads.get(steward)!.visibility).toBe("hidden"); + expect(t.threads.get(steward)!.visibility).toBe("visible"); expect((await t.list())[0]).toMatchObject({ ended: false, threadId: steward }); - // Sam pushes: the same thread is woken for the round, but the wake does - // not need the user, so it stays hidden and the next report keeps it so. + // Sam pushes: the same thread is woken for the round and stays visible. t.entities.set("acme/web#7", pull({}, { headSha: "bbbbbbb2" })); await t.ring("github", "acme/web#7"); - expect(t.threads.get(steward)!.visibility).toBe("hidden"); + expect(t.threads.get(steward)!.visibility).toBe("visible"); expect(t.sends.at(-1)!.text).toMatch(/Pushed bbbbbbb/u); await t.tool("steward_report", waiting, steward); await t.idle(steward); - expect(t.threads.get(steward)!.visibility).toBe("hidden"); - expect(t.spawns).toHaveLength(2); - }); - - it("brings a hidden steward back on a wake that needs the user", async () => { - const t = await setup(); - t.entities.set("acme/web#7", pull()); - await t.ring("github", "acme/web#7"); - const steward = t.spawns[1]!.id; - await t.tool("steward_report", waiting, steward); - await t.idle(steward); - expect(t.threads.get(steward)!.visibility).toBe("hidden"); - - // The status now says the user is next (as a report would have left it), - // so the next wake is one that needs them and the row comes back. - const b = t.hooks.store.byThread(steward)!; - t.hooks.store.patch(b.id, { status: { ...b.status!, actor: "me" } }); - t.entities.set("acme/web#7", pull({}, { headSha: "ccccccc3" })); - await t.ring("github", "acme/web#7"); expect(t.threads.get(steward)!.visibility).toBe("visible"); + expect(t.spawns).toHaveLength(2); + // No `threads.update { visibility: "hidden" }` is issued anywhere. + expect(t.harness.sdk.callsTo("threads.update").some(([a]) => (a as { visibility?: string }).visibility === "hidden")).toBe(false); }); - it("hides a ticket steward waiting on reviews of its pull requests", async () => { + it("keeps a ticket steward waiting on reviews of its pull requests visible", async () => { const t = await setup(); t.entities.set("uuid-12", issue({ state: "In Review", stateType: "started" })); await t.ring("linear", "uuid-12"); const steward = t.spawns[1]!.id; await t.tool("steward_report", { ...waiting, phase: "In Review", blockedOn: "reviewers on acme/web#7" }, steward); await t.idle(steward); - expect(t.threads.get(steward)!.visibility).toBe("hidden"); + expect(t.threads.get(steward)!.visibility).toBe("visible"); }); it("leaves a steward visible when the user is next, and keeps it visible with a worker in flight", async () => { @@ -2430,7 +2413,7 @@ describe("stewards waiting on others", () => { expect(t.harness.sdk.callsTo("threads.update").some(([a]) => (a as { threadId: string; visibility?: string }).threadId === steward && (a as { visibility?: string }).visibility === "hidden")).toBe(false); }); - it("hides a waiting steward once its last worker has stopped", async () => { + it("keeps a waiting steward visible once its last worker has stopped", async () => { const t = await setup(); t.entities.set("uuid-12", issue({ state: "In Progress", stateType: "started" })); await t.ring("linear", "uuid-12"); @@ -2441,13 +2424,13 @@ describe("stewards waiting on others", () => { await t.flush(); expect(t.hooks.store.worker("thr_w1")!.status).toBe("stopped"); - // A stopped worker is not in flight, so the steward hides as before (BB-56). + // A stopped worker is not in flight, but the steward is not hidden either. await t.tool("steward_report", { phase: "In Progress", summary: "Implemented.", nextAction: "Wait", actor: "other" }, steward); await t.idle(steward); - expect(t.threads.get(steward)!.visibility).toBe("hidden"); + expect(t.threads.get(steward)!.visibility).toBe("visible"); }); - it("hides a waiting steward after its worker failed", async () => { + it("keeps a waiting steward visible after its worker failed", async () => { const t = await setup(); t.entities.set("uuid-12", issue({ state: "In Progress", stateType: "started" })); await t.ring("linear", "uuid-12"); @@ -2458,64 +2441,12 @@ describe("stewards waiting on others", () => { await t.flush(); expect(t.hooks.store.worker("thr_w1")!.status).toBe("failed"); - // A failed worker is not in flight, so the steward hides as before (BB-56). + // A failed worker is not in flight, but the steward is not hidden either. await t.tool("steward_report", { phase: "In Progress", summary: "Implemented.", nextAction: "Wait", actor: "other" }, steward); await t.idle(steward); - expect(t.threads.get(steward)!.visibility).toBe("hidden"); - }); - - it("shows a hidden steward when a worker of its binding starts (BB-64)", async () => { - const t = await setup(); - t.entities.set("uuid-12", issue({ state: "In Progress", stateType: "started" })); - await t.ring("linear", "uuid-12"); - const steward = t.spawns[1]!.id; - // The steward waits on someone else and its turn ends: hidden. - await t.tool("steward_report", { ...waiting, phase: "In Progress" }, steward); - await t.idle(steward); - expect(t.threads.get(steward)!.visibility).toBe("hidden"); - - // Its hold lifts and it starts a worker: the steward comes back so the - // effort roots at it, not a bare worker row under Threads. - await t.created("thr_w1", steward); - expect(t.threads.get(steward)!.visibility).toBe("visible"); - expect(t.threads.get("thr_w1")!.visibility).toBe("visible"); - }); - - it("shows a hidden steward when its finished worker runs again (BB-64)", async () => { - const t = await setup(); - t.entities.set("uuid-12", issue({ state: "In Progress", stateType: "started" })); - await t.ring("linear", "uuid-12"); - const steward = t.spawns[1]!.id; - await t.created("thr_w1", steward); - await t.idle("thr_w1", "Should I keep the reviewer open?"); - expect(t.hooks.store.worker("thr_w1")!.status).toBe("finished"); - - // The steward is hidden (a hold lift with the worker parked), and its - // worker is woken again: the steward comes back with it. - t.threads.get(steward)!.visibility = "hidden"; - await t.harness.behavior.emitThreadEvent("thread.active", { thread: makeThreadResponse({ id: "thr_w1" }) }); - await t.flush(); - expect(t.hooks.store.worker("thr_w1")!.status).toBe("running"); expect(t.threads.get(steward)!.visibility).toBe("visible"); }); - it("leaves a hidden steward with no worker hidden on a wake that needs nobody (BB-64)", async () => { - const t = await setup(); - t.entities.set("uuid-12", issue({ state: "In Progress", stateType: "started" })); - await t.ring("linear", "uuid-12"); - const steward = t.spawns[1]!.id; - await t.tool("steward_report", { ...waiting, phase: "In Progress" }, steward); - await t.idle(steward); - expect(t.threads.get(steward)!.visibility).toBe("hidden"); - - // A hold-lift wake reaches the steward, but it needs no user and has no - // worker in flight, so it stays hidden until it starts one. - t.entities.set("uuid-12", issue({ state: "In Progress", stateType: "started", comments: [comment({ createdAt: "2026-10-14T16:00:00Z" })] })); - await t.ring("linear", "uuid-12"); - expect(t.sends.at(-1)!.threadId).toBe(steward); - expect(t.threads.get(steward)!.visibility).toBe("hidden"); - }); - it("keeps a steward with an open escalation visible", async () => { const t = await setup(); t.entities.set("uuid-12", issue()); @@ -2526,21 +2457,7 @@ describe("stewards waiting on others", () => { expect(t.threads.get(steward)!.visibility).toBe("visible"); }); - it("does not hide a turn the user started", async () => { - const t = await setup(); - t.entities.set("acme/web#7", pull()); - await t.ring("github", "acme/web#7"); - const steward = t.spawns[1]!.id; - await t.tool("steward_report", waiting, steward); - await t.idle(steward); - // The user brings it back to talk to it; its report then is theirs to read. - t.threads.get(steward)!.visibility = "visible"; - await t.tool("steward_report", waiting, steward); - await t.idle(steward); - expect(t.threads.get(steward)!.visibility).toBe("visible"); - }); - - it("hides after compaction rather than during it", async () => { + it("keeps the steward visible after compaction rather than during it", async () => { const t = await setup(); t.entities.set("acme/web#7", pull()); await t.ring("github", "acme/web#7"); @@ -2550,9 +2467,9 @@ describe("stewards waiting on others", () => { await t.idle(steward); expect(t.harness.sdk.callsTo("threads.compact")).toHaveLength(1); expect(t.threads.get(steward)!.visibility).toBe("visible"); - // Compaction ends in another idle. + // Compaction ends in another idle; the thread stays visible either way. await t.idle(steward); - expect(t.threads.get(steward)!.visibility).toBe("hidden"); + expect(t.threads.get(steward)!.visibility).toBe("visible"); }); it("keeps a waiting steward visible while its finished worker's child still runs, and names the worker's question next wake (BB-11, BB-56)", async () => { @@ -2603,7 +2520,7 @@ describe("stewards waiting on others", () => { await t.tool("steward_report", { ...waiting, phase: "In Progress" }, steward); await t.idle(steward); - expect(t.threads.get(steward)!.visibility).toBe("hidden"); + expect(t.threads.get(steward)!.visibility).toBe("visible"); expect(t.threads.get("thr_r1")!.archivedAt).toBeNull(); }); @@ -2653,6 +2570,54 @@ describe("stewards waiting on others", () => { }); }); +describe("the one-time unhide (BB-93)", () => { + it("shows every live, unarchived hidden thread of an open binding, and does nothing on a second start", async () => { + const t = await setup(); + t.entities.set("uuid-12", issue({ state: "In Progress", stateType: "started" })); + await t.ring("linear", "uuid-12"); + const steward = t.spawns[1]!.id; + await t.created("thr_w1", steward); + await t.created("thr_r1", "thr_w1", "Review round 1"); + // As the old hide path left them: hidden, unarchived, of an open binding. + t.threads.get(steward)!.visibility = "hidden"; + t.threads.get("thr_w1")!.visibility = "hidden"; + t.threads.get("thr_r1")!.visibility = "hidden"; + // An archived hidden thread is left alone. + await t.created("thr_w2", steward); + t.threads.get("thr_w2")!.visibility = "hidden"; + t.threads.get("thr_w2")!.archivedAt = 1; + + await t.hooks.unhideOnce(); + expect(t.threads.get(steward)!.visibility).toBe("visible"); + expect(t.threads.get("thr_w1")!.visibility).toBe("visible"); + expect(t.threads.get("thr_r1")!.visibility).toBe("visible"); + expect(t.threads.get("thr_w2")!.visibility).toBe("hidden"); + + // The marker is recorded: a later start shows nothing. + t.threads.get(steward)!.visibility = "hidden"; + const updates = t.harness.sdk.callsTo("threads.update").length; + await t.hooks.unhideOnce(); + expect(t.threads.get(steward)!.visibility).toBe("hidden"); + expect(t.harness.sdk.callsTo("threads.update")).toHaveLength(updates); + }); + + it("runs on the plugin's loop service start, not only from the test seam", async () => { + const t = await setup(); + t.entities.set("uuid-12", issue({ state: "In Progress", stateType: "started" })); + await t.ring("linear", "uuid-12"); + const steward = t.spawns[1]!.id; + t.threads.get(steward)!.visibility = "hidden"; + + const service = t.harness.runService("loop"); + try { + await t.flush(); + expect(t.threads.get(steward)!.visibility).toBe("visible"); + } finally { + service.controller.abort(); + } + }); +}); + describe("archiving finished workers and reviewers (BB-47)", () => { it("archives a reviewer once its worker's next turn ends, and keeps the worker until the round is consumed", async () => { const t = await setup(); @@ -2729,50 +2694,6 @@ describe("archiving finished workers and reviewers (BB-47)", () => { expect(t.sends.find((s) => s.threadId === "thr_w1")!.text).toMatch(/The second one\./u); }); - it("shows a hidden steward's finished workers and their reviewers with it, and leaves a running worker's hidden reviewer hidden", async () => { - const t = await setup(); - t.entities.set("uuid-12", issue({ state: "In Progress", stateType: "started" })); - await t.ring("linear", "uuid-12"); - const steward = t.spawns[1]!.id; - await t.created("thr_w1", steward); - await t.created("thr_w2", steward); - await t.created("thr_w3", steward); - await t.created("thr_r1", "thr_w1", "Review round 1"); - // thr_w1 is finished (it asked a question, so it stays visible), its - // reviewer is still running, thr_w2 runs, and thr_w3's reviewer is a - // build-tickets hidden spawn (a running worker's reviewer is not shown - // by the cascade and must stay hidden when the steward comes back). - await t.created("thr_r2", "thr_w3", "Review round 1"); - t.threads.get("thr_r2")!.visibility = "hidden"; - await t.idle("thr_w1", "Should I keep the reviewer open?"); - await t.tool("steward_report", { phase: "In Progress", summary: "Implemented.", nextAction: "Wait for the worker", actor: "other", blockedOn: "the worker" }, steward); - await t.idle(steward); - expect(t.threads.get(steward)!.visibility).toBe("visible"); - - // The user hid the steward by hand, and the finished worker and reviewer - // went with it: the state the show cascade repairs (BB-47). A waiting - // steward is no longer hidden while a worker is in flight (BB-56), so - // this is the path that reaches the cascade. - t.threads.get(steward)!.visibility = "hidden"; - t.threads.get("thr_w1")!.visibility = "hidden"; - t.threads.get("thr_r1")!.visibility = "hidden"; - - // A wake that needs the user brings the steward back, and the finished - // worker and its reviewer with it; a running worker is unaffected, and - // its hidden reviewer stays hidden. - const b = t.hooks.store.byThread(steward)!; - t.hooks.store.patch(b.id, { status: { ...b.status!, actor: "me" } }); - t.entities.set("uuid-12", issue({ state: "In Progress", stateType: "started", comments: [comment({ createdAt: "2026-10-14T16:00:00Z" })] })); - await t.ring("linear", "uuid-12"); - expect(t.threads.get(steward)!.visibility).toBe("visible"); - expect(t.threads.get("thr_w1")!.visibility).toBe("visible"); - expect(t.threads.get("thr_r1")!.visibility).toBe("visible"); - expect(t.threads.get("thr_w2")!.visibility).toBe("visible"); - // The show cascade is symmetric: it does not flip a running worker's - // hidden reviewer into the sidebar. - expect(t.threads.get("thr_r2")!.visibility).toBe("hidden"); - }); - it("does not archive a finished worker when the reviewer listing fails, so a reviewer is never orphaned", async () => { const t = await setup(); t.entities.set("uuid-12", issue({ state: "In Progress", stateType: "started" })); @@ -2786,18 +2707,6 @@ describe("archiving finished workers and reviewers (BB-47)", () => { await t.idle("thr_w1", "Implemented the fix."); expect(t.threads.get("thr_w1")!.archivedAt).toBeNull(); }); - - it("shows a worker the waiting cascade hid once it is woken again", async () => { - const t = await setup(); - t.entities.set("uuid-12", issue({ state: "In Progress", stateType: "started" })); - await t.ring("linear", "uuid-12"); - const steward = t.spawns[1]!.id; - await t.created("thr_w1", steward); - t.threads.get("thr_w1")!.visibility = "hidden"; - await t.harness.behavior.emitThreadEvent("thread.active", { thread: makeThreadResponse({ id: "thr_w1", visibility: "hidden" }) }); - await t.flush(); - expect(t.threads.get("thr_w1")!.visibility).toBe("visible"); - }); }); describe("passes", () => { @@ -5222,7 +5131,7 @@ describe("ops stewards", () => { expect(healthy).toMatch(/Requeue time came: a scheduled look/u); expect(healthy).not.toMatch(/#### /u); const report = await t.tool("steward_report", { phase: "watching", summary: "All healthy.", nextAction: "Look again", actor: "other" }, ops.id); - expect(report.text).toMatch(/Your thread is hidden when the turn ends/u); + expect(report.text).not.toMatch(/hidden/u); }); it("compacts the ops steward's context before a wake that would push it over the limit, and still delivers it", async () => { diff --git a/plugins/stewards/server.ts b/plugins/stewards/server.ts index 8e0cc4e..8afde62 100644 --- a/plugins/stewards/server.ts +++ b/plugins/stewards/server.ts @@ -1284,10 +1284,6 @@ export function createStewardsPlugin(deps: StewardsDeps): (bb: BbPluginApi) => P const info = await threadInfo(threadId).catch(() => null); if (info === null || info.deletedAt !== null) return "gone"; if (info.archivedAt !== null) await quietly(`unarchive ${threadId}`, () => bb.sdk.threads.unarchive({ threadId })); - // A worker the waiting cascade hid comes back with the message, so a - // resumed worker shows under its effort again (BB-47). A steward's own - // thread keeps its hidden state: a wake that needs nobody stays hidden. - if (info.visibility === "hidden" && store.worker(threadId) !== null) await quietly(`show ${threadId}`, () => bb.sdk.threads.update({ threadId, visibility: "visible" })); return send(threadId, typeof text === "string" ? text : text()); } @@ -1585,37 +1581,33 @@ export function createStewardsPlugin(deps: StewardsDeps): (bb: BbPluginApi) => P } } - /** Set a thread's sidebar visibility, unless it is archived, gone, or already there. */ - async function setVisibility(threadId: string, visibility: "hidden" | "visible"): Promise { - const info = await threadInfo(threadId).catch(() => null); - if (info === null || info.deletedAt !== null || info.archivedAt !== null || info.visibility === visibility) return; - await quietly(`${visibility === "hidden" ? "hide" : "show"} ${threadId}`, () => bb.sdk.threads.update({ threadId, visibility })); - } - /** - * A waiting steward's finished workers and their reviewers share its - * visibility: hidden with it, and shown again when it comes back. Running - * workers are unaffected (BB-47). + * The one-time unhide (BB-93). Before this change a steward waiting on + * someone else had its thread hidden (`threads.update { visibility: + * "hidden" }`), and its finished workers and their reviewers with it. + * Nothing hides a thread any more, but a server upgrading still holds + * those rows, so the first start after the change shows every live, + * unarchived hidden thread of an open binding. The marker lives in the + * plugin's kv store, so a later start does nothing. */ - async function cascadeVisibility(b: Binding, visibility: "hidden" | "visible"): Promise { - for (const w of store.workers(b.id)) { - if (w.status !== "finished") continue; - await setVisibility(w.threadId, visibility); - for (const r of (await reviewersOf(w.threadId)) ?? []) await setVisibility(r.id, visibility); + const UNHIDE_ONCE_KEY = "migrations/unhide-hidden-threads"; + async function unhideOnce(): Promise { + if ((await bb.storage.kv.get(UNHIDE_ONCE_KEY)) != null) return; + for (const b of store.open()) { + if (b.threadId !== null) await showIfHidden(b.threadId); + for (const w of store.workers(b.id)) { + await showIfHidden(w.threadId); + for (const r of (await reviewersOf(w.threadId)) ?? []) await showIfHidden(r.id); + } } + await bb.storage.kv.set(UNHIDE_ONCE_KEY, String(now())); } - /** Show a steward, and with it the finished workers and reviewers that were hidden with it. */ - async function showSteward(b: Binding): Promise { - if (b.threadId === null) return; - const info = await threadInfo(b.threadId).catch(() => null); - // Already visible: nothing to show and no write to spend. This runs on - // every worker registration and turn, so the common case is a no-op. A - // lookup that failed is unknown, so the write is still attempted. - if (info !== null && info.visibility === "visible") return; - const wasHidden = info !== null && info.visibility === "hidden"; - await quietly(`show ${b.entityKey}'s steward`, () => bb.sdk.threads.update({ threadId: b.threadId!, visibility: "visible" })); - if (wasHidden) await cascadeVisibility(b, "visible"); + /** Show a thread hidden in the sidebar, unless it is archived, gone, or already visible. */ + async function showIfHidden(threadId: string): Promise { + const info = await threadInfo(threadId).catch(() => null); + if (info === null || info.deletedAt !== null || info.archivedAt !== null || info.visibility !== "hidden") return; + await quietly(`show ${threadId}`, () => bb.sdk.threads.update({ threadId, visibility: "visible" })); } /** @@ -1784,9 +1776,6 @@ export function createStewardsPlugin(deps: StewardsDeps): (bb: BbPluginApi) => P }); store.setSecretRequester(thread.id, need.id); store.addWorker(thread.id, b.id, title); - // A requester is a worker of the binding: a steward hidden by hideWaiting - // comes back so the effort roots at it (BB-64). - await showSteward(b); bb.log.info(`stewards: started a requester thread for ${b.entityKey}'s ${variable} (${thread.id})`); return thread.id; } @@ -1845,10 +1834,6 @@ export function createStewardsPlugin(deps: StewardsDeps): (bb: BbPluginApi) => P // compact first (criterion 4). If compaction makes the thread busy, // sendTo queues the wake for the next idle. await compactIfLarge(b.threadId, text.length); - // A wake that needs the user brings a hidden steward back into the - // sidebar, with the finished workers and reviewers that were hidden with - // it; a scheduled look that found nothing leaves it hidden. - if (needsUser(b)) await showSteward(b); const sent = await sendTo(b.threadId, text); if (sent === "gone") { // The user deleted the thread: that is a detach they meant. @@ -4215,10 +4200,6 @@ export function createStewardsPlugin(deps: StewardsDeps): (bb: BbPluginApi) => P const runaway = store.spawnsSince(owner.id, now() - 3_600_000) >= limit; store.addWorker(thread.id, owner.id, thread.title ?? thread.titleFallback ?? thread.id); if (!runaway) { - // A hidden steward (hideWaiting, "waits on others") comes back when a - // worker of its binding starts, so the sidebar roots the effort at it - // and the worker is not a bare row under Threads (BB-64). - await showSteward(owner); await validateWorkerSpawn(thread, owner); return; } @@ -4312,39 +4293,11 @@ export function createStewardsPlugin(deps: StewardsDeps): (bb: BbPluginApi) => P } } // Compaction makes the thread busy; a wake that finds it so waits as notes for the next idle. - const compacting = await compactIfLarge(thread.id); + await compactIfLarge(thread.id); const after = store.binding(b.id)!; if (after.pending.some((n) => n.quiet !== true)) scheduleReconcile(b.id); - // Compaction ends in another idle, which hides. - else if (after.archiveWhenIdle && !compacting) await hideWaiting(after); }); - /** - * Hide a steward whose wake turn ended waiting on someone else, so the - * sidebar holds only stewards with something to do. Hidden threads stay - * out of the sidebar's organisation and unread attention while remaining - * addressable; the binding stays, and the next wake brings it back - * (deliverWake). Not while anything needs the user — an open escalation, a - * question, or a needs item — which they should still see. Not while a - * worker of the binding is in flight either (running or finished): the - * sidebar's buildTab roots the effort at the steward thread, so hiding the - * steward while its worker runs drops the effort out of In flight and - * leaves a bare worker row under Threads. Once no worker is in flight, - * hiding cascades to any worker that finished in the meantime, so it is - * not left as a bare row without its steward (BB-47, BB-56). - */ - async function hideWaiting(b: Binding): Promise { - store.patch(b.id, { archiveWhenIdle: false }); - if (anyWorkerInFlight(store.workers(b.id))) return; - if (b.threadId === null || b.endedAt !== null || b.ending !== null) return; - if (needsUser(b)) return; - const info = await threadInfo(b.threadId).catch(() => null); - if (info === null || info.deletedAt !== null || info.archivedAt !== null || busy(info.status)) return; - await quietly(`hide ${b.entityKey}'s waiting steward`, () => bb.sdk.threads.update({ threadId: info.id, visibility: "hidden" })); - await cascadeVisibility(b, "hidden"); - bb.log.info(`stewards: ${b.entityKey} waits on others; hid its steward until the next wake`); - } - /** * The questions the binding's workers ended their last turn with. A * finished-but-archived worker is brought back so the steward's @@ -4432,13 +4385,8 @@ export function createStewardsPlugin(deps: StewardsDeps): (bb: BbPluginApi) => P bb.events.on("thread.active", async ({ thread }) => { const worker = store.worker(thread.id); if (worker === null) return; - // A worker the waiting cascade hid is shown again once it runs (BB-47). - if (thread.visibility === "hidden") await quietly(`show running worker ${thread.id}`, () => bb.sdk.threads.update({ threadId: thread.id, visibility: "visible" })); if (worker.status === "finished") store.setWorker(thread.id, "running", worker.result); const owner = store.binding(worker.bindingId); - // A hidden steward comes back when a worker of its binding runs again, - // so the effort stays rooted at it (BB-64). - if (owner !== null && owner.endedAt === null) await showSteward(owner); // The first turn is dispatched now, so the spawn's model can be read. if (worker.modelCheckPending && worker.status !== "stopped") { store.setWorkerModelCheckPending(thread.id, false); @@ -5664,11 +5612,7 @@ export function createStewardsPlugin(deps: StewardsDeps): (bb: BbPluginApi) => P at: now(), }; const moved = advanced(b.status, status); - // A wake turn that ends waiting on someone else is hidden when it - // goes idle; a turn the user started is left where they can see it. - const archiveWhenIdle = input.actor === "other" && (b.awaitingReport || b.archiveWhenIdle); b = store.patch(b.id, { - archiveWhenIdle, status, statusAt: now(), ...(moved ? { advancedAt: now() } : {}), @@ -5740,14 +5684,12 @@ export function createStewardsPlugin(deps: StewardsDeps): (bb: BbPluginApi) => P if (raised !== null && !(raised.route === "project" && requests.length > 0)) await deliver(b, raised); if (requests.length > 0) await wakeProject(b.projectId, raised !== null && raised.route === "project" ? `escalation and ${requests.join(" and ")} request from ${b.entityKey}` : `${requests.join(" and ")} requested for ${b.entityKey}`, { digestDue: false }); // The unread dot means "needs you": a turn that ends with nothing for - // the user is marked read and hidden when it goes idle, so a "nothing - // to do yet" look does not flag the sidebar like a question does. A - // turn that does need the user is made visible and left unread. A - // terminal ending archives the thread, so there is no read state to - // set. - if (b.ending !== "terminal") { - if (needsUser(b)) await showSteward(b); - else await quietly(`mark ${b.entityKey}'s steward read`, () => bb.sdk.threads.markRead({ threadId })); + // the user is marked read, so a "nothing to do yet" look does not + // flag the sidebar like a question does. A turn that does need the + // user is left unread. A terminal ending archives the thread, so + // there is no read state to set. + if (b.ending !== "terminal" && !needsUser(b)) { + await quietly(`mark ${b.entityKey}'s steward read`, () => bb.sdk.threads.markRead({ threadId })); } if (moved && b.parentBindingId !== null) { const parent = store.binding(b.parentBindingId); @@ -5802,12 +5744,11 @@ export function createStewardsPlugin(deps: StewardsDeps): (bb: BbPluginApi) => P if (b.followThrough === null) return `Reported.${replacedNote}${refusalNote}${escalated}${deploymentNote} The binding ends when this turn does: running workers are stopped and the thread is archived. End the turn now.`; return `Reported.${replacedNote}${refusalNote}${escalated}${deploymentNote} The binding stays open to follow through until ${iso(followThroughUntil!)}; running workers keep going. You will be woken when a worker finishes or something actionable changes, or in ${formatDuration(requeueMs)}. End the turn now.`; } - const hidden = archiveWhenIdle && !needsUser(b) ? " Your thread is hidden when the turn ends, and comes back on that wake." : ""; const recorded = opsTickets.length === 0 ? "" : ` Recorded ${opsTickets.map((t) => `${t.ticket} (${t.did})`).join(", ")} against ${opsTickets.length === 1 ? "its fingerprint" : "their fingerprints"}.`; if (gateJustPassed && nothingPending) { return `Reported.${replacedNote}${recorded}${refusalNote}${escalated} Ready gate: passed and nothing schedules you. Move the ticket to In Progress and start the worker now, in this turn, with the task in the In Progress instructions; do not end the turn first.`; } - return `Reported.${replacedNote}${recorded}${refusalNote}${gateNote}${escalated} You will be woken when something actionable changes, or in ${formatDuration(requeueMs)}.${hidden} End the turn now.`; + return `Reported.${replacedNote}${recorded}${refusalNote}${gateNote}${escalated} You will be woken when something actionable changes, or in ${formatDuration(requeueMs)}. End the turn now.`; } catch (error) { return toolError(error); } @@ -6621,6 +6562,9 @@ export function createStewardsPlugin(deps: StewardsDeps): (bb: BbPluginApi) => P } bb.background.service("loop", { async start(signal) { + // BB-93: nothing hides threads any more; the first start after the + // upgrade shows the ones the old hide path left hidden. + await unhideOnce().catch((e: unknown) => bb.log.warn(`stewards: one-time unhide: ${errorMessage(e)}`)); const requeue = setInterval(() => void requeueTick().catch((e: unknown) => bb.log.warn(`stewards: requeue: ${errorMessage(e)}`)), timing.requeueTickMs); const sweeper = setInterval(() => void sweep().catch((e: unknown) => bb.log.warn(`stewards: sweep: ${errorMessage(e)}`)), timing.sweepMs); // The first sweep tells linear and github what to watch after a restart. @@ -7298,12 +7242,12 @@ export function createStewardsPlugin(deps: StewardsDeps): (bb: BbPluginApi) => P kept.clear(); }; await pinSidebarThreadListProvider(bb); - testHooks.set(bb.pluginId, { sweep, requeueTick, store, db, settings, forgetTags, nightlyPass: (projectId: string, night: string) => loadConfig(projectId, true).then((c) => (c === null ? undefined : nightlyPass(c, night, true))), settle: () => Promise.all([...chains.values()]).then(() => undefined), sentryDoorbell: onSentryDoorbell, sentryLeases }); + testHooks.set(bb.pluginId, { sweep, requeueTick, unhideOnce, store, db, settings, forgetTags, nightlyPass: (projectId: string, night: string) => loadConfig(projectId, true).then((c) => (c === null ? undefined : nightlyPass(c, night, true))), settle: () => Promise.all([...chains.values()]).then(() => undefined), sentryDoorbell: onSentryDoorbell, sentryLeases }); }; } /** Test seams, keyed by plugin id. Not part of any contract. */ -export const testHooks = new Map; requeueTick(): Promise; store: Store; db: { prepare(sql: string): { run(...args: unknown[]): unknown } }; settings: { get(): Promise> }; forgetTags(): void; nightlyPass(projectId: string, night: string): Promise; settle(): Promise; sentryDoorbell(id: string, unit: string | null): void; sentryLeases(): Lease[] }>(); +export const testHooks = new Map; requeueTick(): Promise; unhideOnce(): Promise; store: Store; db: { prepare(sql: string): { run(...args: unknown[]): unknown } }; settings: { get(): Promise> }; forgetTags(): void; nightlyPass(projectId: string, night: string): Promise; settle(): Promise; sentryDoorbell(id: string, unit: string | null): void; sentryLeases(): Lease[] }>(); export { DEFAULT_PLAYBOOKS, STEWARDS_PLUGIN_ID }; diff --git a/plugins/stewards/skills/stewards/SKILL.md b/plugins/stewards/skills/stewards/SKILL.md index 3e87b95..e2d8bf7 100644 --- a/plugins/stewards/skills/stewards/SKILL.md +++ b/plugins/stewards/skills/stewards/SKILL.md @@ -210,10 +210,9 @@ Two tools are yours; everything else you do with the tools every thread has. brings it back. If someone moves the entity out of its terminal state before then, the ending is taken back and you carry on. - **Waiting.** Report `actor: other` only when someone else really is next. - The plugin then hides your thread after a wake turn, unless an escalation, - a question or a needs item is open; the next wake still reaches it, and a - wake that needs you makes it visible again. A report that needs nobody is - also marked read, so the sidebar's unread dot means "needs you". + Your thread stays in the sidebar between wakes; the next wake still + reaches it. A report that needs nobody is marked read, so the sidebar's + unread dot means "needs you"; a report that needs you is left unread. ## If you are an ops steward From 086f8f0be9dbc99e44271d8e340ad62d4917ce98 Mon Sep 17 00:00:00 2001 From: Dylan Trotter Date: Tue, 27 Oct 2026 20:57:22 +0000 Subject: [PATCH 2/2] stewards: retry the one-time unhide when a show fails (BB-93) --- plugins/stewards/DESIGN.md | 5 ++++- plugins/stewards/server.test.ts | 20 ++++++++++++++++++++ plugins/stewards/server.ts | 26 ++++++++++++++++++-------- 3 files changed, 42 insertions(+), 9 deletions(-) diff --git a/plugins/stewards/DESIGN.md b/plugins/stewards/DESIGN.md index 109ddc9..06c498a 100644 --- a/plugins/stewards/DESIGN.md +++ b/plugins/stewards/DESIGN.md @@ -276,7 +276,10 @@ plugins. not hidden with it either: hiding made the stewards sidebar drop a waiting or held steward from the list entirely (BB-93). The report is marked read (`threads.markRead`) when nothing needs the user, so the unread dot means - "needs you"; a report that needs the user is left unread. The + "needs you"; a report that needs the user is left unread. The plugin never + sets a thread's visibility now, so a thread the user hides by hand stays + hidden even when it later asks a question; showing it again is the user's + to undo. The open-escalation rule stays. binding end still archives the thread. Workers stay lifecycle-owned: they are the steward's children in bb, and the plugin stops them explicitly only diff --git a/plugins/stewards/server.test.ts b/plugins/stewards/server.test.ts index 3a4812c..de5b69d 100644 --- a/plugins/stewards/server.test.ts +++ b/plugins/stewards/server.test.ts @@ -2616,6 +2616,26 @@ describe("the one-time unhide (BB-93)", () => { service.controller.abort(); } }); + + it("retries on the next start when a show fails, rather than burning the marker", async () => { + const t = await setup(); + t.entities.set("uuid-12", issue({ state: "In Progress", stateType: "started" })); + await t.ring("linear", "uuid-12"); + const steward = t.spawns[1]!.id; + t.threads.get(steward)!.visibility = "hidden"; + + // The API is down for the first start: the marker is not written, so a + // later start tries again instead of leaving the thread hidden forever. + t.harness.sdk.stub("threads.update", async () => { + throw new Error("the API is down"); + }); + await t.hooks.unhideOnce(); + expect(t.threads.get(steward)!.visibility).toBe("hidden"); + + t.harness.sdk.stub("threads.update", t.sdk.threads.update); + await t.hooks.unhideOnce(); + expect(t.threads.get(steward)!.visibility).toBe("visible"); + }); }); describe("archiving finished workers and reviewers (BB-47)", () => { diff --git a/plugins/stewards/server.ts b/plugins/stewards/server.ts index 8afde62..119e2ec 100644 --- a/plugins/stewards/server.ts +++ b/plugins/stewards/server.ts @@ -1593,21 +1593,31 @@ export function createStewardsPlugin(deps: StewardsDeps): (bb: BbPluginApi) => P const UNHIDE_ONCE_KEY = "migrations/unhide-hidden-threads"; async function unhideOnce(): Promise { if ((await bb.storage.kv.get(UNHIDE_ONCE_KEY)) != null) return; + let failed = false; for (const b of store.open()) { - if (b.threadId !== null) await showIfHidden(b.threadId); + if (b.threadId !== null && !(await showIfHidden(b.threadId))) failed = true; for (const w of store.workers(b.id)) { - await showIfHidden(w.threadId); - for (const r of (await reviewersOf(w.threadId)) ?? []) await showIfHidden(r.id); + if (!(await showIfHidden(w.threadId))) failed = true; + for (const r of (await reviewersOf(w.threadId)) ?? []) if (!(await showIfHidden(r.id))) failed = true; } } - await bb.storage.kv.set(UNHIDE_ONCE_KEY, String(now())); + // A transient show failure must not burn the one-time marker: leaving it + // unset makes the next start retry, as the old show-on-every-wake path + // did. Only a clean pass records it. + if (!failed) await bb.storage.kv.set(UNHIDE_ONCE_KEY, String(now())); } - /** Show a thread hidden in the sidebar, unless it is archived, gone, or already visible. */ - async function showIfHidden(threadId: string): Promise { + /** Show a thread hidden in the sidebar; false when the update failed, so the caller can retry. */ + async function showIfHidden(threadId: string): Promise { const info = await threadInfo(threadId).catch(() => null); - if (info === null || info.deletedAt !== null || info.archivedAt !== null || info.visibility !== "hidden") return; - await quietly(`show ${threadId}`, () => bb.sdk.threads.update({ threadId, visibility: "visible" })); + if (info === null || info.deletedAt !== null || info.archivedAt !== null || info.visibility !== "hidden") return true; + try { + await bb.sdk.threads.update({ threadId, visibility: "visible" }); + return true; + } catch (error) { + bb.log.warn(`stewards: could not show ${threadId}: ${errorMessage(error)}`); + return false; + } } /**