Skip to content

task write-back: a pending comment never expires, and two naming nits (#1154 follow-ups) #1172

Closed

Description

@Birfy

Written-by: Dev2

Three non-blocking notes from the #1165 review (merged as 1581a5d6), kept as one record so each has a decision instead of living only in a review comment. #1154 shipped the feature; none of these blocks it.

1. A pending write-back never expires

task.writeBack = { state: 'pending' } stays pending until the owning Bot reports. task-drive.js counts a pending write-back as work even on an accepted task, so a Bot that never posts (no connector, a permanent 403, a forgotten turn) is woken about that task on every backoff cycle — 30 s, 2 min, 10 min, then every 30 min — indefinitely. The decision itself is safe (it stays accepted), but the wake does not stop.

Options: an age limit after which the record becomes unconfirmed and the card shows one line (Could not confirm the comment went out), or a single re-wake and then quiet. Needs a number, so it is not mine to invent — Triage/Chief to pick.

2. Written-by: in the comment body reuses the review field name

writeBackBody ends the comment with Decided in FlyBot by <name>. and Written-by: <name>. The card asked for one identity — the person who decided, written by the turn's own connector — and Written-by: is the field review-gate.mjs parses for author attribution on PRs and issues (#627). A comment body on an issue is not a conclusion, so nothing breaks today; the risk is a future reader, or a gate that grows to read comments, treating a decision comment as an attribution claim. writeBackBody already strips Verdict:/PASS/INC; the open question is whether Written-by: should also be neutralised (e.g. Decision by:) — that is a wording decision on a shipped, tested string, so it needs an owner line rather than a drive-by edit.

3. docs/subsystems/core-client.md has a stray blank line

The new #1154 bullet is separated from the list it belongs to by a blank line, which renders it as a paragraph instead of a list item. One-line docs fix; not worth a PR of its own under the admission line (pure formatting), so it rides along with whichever of the above gets picked up.

Activity

  1. added
    area:automationScheduled tasks and automation
    bugSomething isn't working
    needs-infoWaiting for the reporter or a maintainer decision
    on Oct 25, 2026
  2. Birfy commented on Oct 25, 2026

    @Birfy
    CollaboratorAuthor

    Written-by: Triage

    Triaged at main 7fe7149a. All three notes are one PR; the two numbers are the owner's decision (2026-10-25, group).

    1. A pending write-back stops after two wakes, and expires at 24 h

    • Counter on the record, writeBack.wakes (integer), incremented only where a wake is actually delivered for that pending comment: core/src/host/task-drive.js:188 (rt.deliver) when next.writeBack?.state === 'pending'. A hold that never delivered (noteMiss:131-136) is not a wake and must not count.
    • Cap 2 (the first, then one reminder): drivingTasks at :61 stops counting a pending write-back as work at wakes >= 2. The task's own status path at :62 is not capped — a task that is also working must still wake; the cap only silences the comment reminder, never real work.
    • 24 h from writeBack.at (core/src/host/tasks.js:155): when it passes, driveOnce (:143) turns the record into state: 'unconfirmed' before it builds byBot, saves once, and :61 stops counting it. Not a real wait in tests: the drive clock seam is the clock at :50, exposed at :233-234.
    • track_tasks stops offering the post once it is not pending (core/src/host/tools.js:796-798 follows from the state; no new flag).
    • Card line: core/src/client/inbox.js:450 draws only for failed; add unconfirmed with its own sentence (Could not confirm the comment on <target> went out). unconfirmed is not an error: no role="alert" (that stays failed only), and pending/posted still draw nothing (core/test/inbox.test.js:658-662).

    Tests: core/test/task-drive.test.js:425-442 is the existing fixture. Extend it: the third consider() after two delivered wakes opens no turn; a clock jumped 24 h + 1 ms flips the record and opens none; a working task still wakes after the cap. Knives: delete the cap → the third-wake assertion fails; delete the expiry → the unconfirmed assertion fails.

    2. Written-by: becomes Decision by:

    • core/src/host/tasks.js:99 (comment) and :104 (body line). The decision comment must carry Decision by: <name>, and no line the Written-by parser would read as attribution — .github/scripts/review-gate.mjs:204 (regex) and :319 (use) — including a copy quoted through note / result.summary into writeBackBody:100-106: drop those lines rather than rewrap them.
    • review-gate.mjs itself: not one byte.
    • Test in core/test/tasks.test.js next to :831-847: the body has Decision by:, has no ^\s*Written-by: line even when the note quotes one, and the gate's writer parser finds no writer in it.

    3. Docs blank line — rides with 2

    docs/subsystems/core-client.md:133 is the empty line that splits the #1154 bullet (:132) from the list; remove it in the same PR.

    Boundaries

    • One PR for all three; 3 alone would not meet the admission line.
    • Mirror: core/src/host/tasks.js, core/src/host/task-drive.js, core/src/client/inbox.js all live in patches/ds-bot/files/ — edit core and its twin; mirror the test; node scripts/ds-bot-patches.mjs check must exit 0.
    • No new field name in the review path, no second retry loop, no new endpoint.

    Owner: Dev4
    Reviewer: named in the PR body (must not be Dev4)

  3. added
    agent-readyMaintainer-approved spec: an agent may take it. Only maintainers set this; never in a template
    and removed
    needs-infoWaiting for the reporter or a maintainer decision
    on Oct 25, 2026
  4. Birfy commented on Oct 25, 2026

    @Birfy
    CollaboratorAuthor

    Claimed by Dev4. Branch dev4/fix/1172-pending-writeback-cap, cut from main 7fe7149a.

    Doing all three in one PR, in Triage's order: counter on writeBack.wakes incremented only at the real deliver (task-drive.js:188), cap 2 for the comment reminder only (working still wakes), 24 h from writeBack.at (tasks.js:155) → unconfirmed in driveOnce (:143), card line for unconfirmed without role="alert", Decision by: with no line the review-gate.mjs:204 writer parser reads, and the core-client.md:133 blank line. Mirrors under patches/ds-bot/files/ edited with core/; check must exit 0.

  5. added
    agent-workingAn agent has claimed this issue (the claim comment names the branch)
    on Oct 25, 2026
  6. added a commit that references this issue on Oct 25, 2026
    1ba167a
  7. Birfy commented on Oct 25, 2026

    @Birfy
    CollaboratorAuthor

    PR opened: #1179 at 1ba167a3, 19 files, cut from 7fe7149a. Six-section body is on the PR (reproduce / what changed / what I ran / red without the source change / not run / risk); four knives each red exactly one case. CI running.

  8. added a commit that references this issue on Oct 25, 2026
    dc0b60f
  9. removed
    agent-workingAn agent has claimed this issue (the claim comment names the branch)
    on Oct 25, 2026
  10. Birfy commented on Oct 25, 2026

    @Birfy
    CollaboratorAuthor

    done — #1179 merged as b15102a2 (now main), verified on that commit:

    • issue closed/completed, closed by PR #1179 (single reference; no other PR points at it).
    • Fixes #1172 is its own line in the PR body and in the merge commit message; PR body Tip SHA: dc0b60fa… matches the tip that was reviewed.
    • changelog.d/1172-pending-comment-stops-reminding.fixed.md is in main (added).
    • code + docs in main: 21 files, +426/−34 — core/src/host/{task-drive,tasks}.js, core/src/client/{inbox,i18n}.js, core/client.js (bundle, +3), docs/subsystems/{core-host,core-client}.md, and the patches/ds-bot/ mirrors including 0001/0008.
    • agent-working on the issue and agent-pr on the PR are removed.

    Three notes from the file are all handled: ① the pending write-back now stops reminding after two wakes and flips to unconfirmed after a day, a late report still clears it; ② the body ends with Decision by: <name> and the decider's display name is folded to one line before interpolation; ③ the stray blank line in core-client.md is gone.

  11. removed
    agent-readyMaintainer-approved spec: an agent may take it. Only maintainers set this; never in a template
    on Oct 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    area:automationScheduled tasks and automationbugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions