Repository navigation
List a Bot's scheduled tasks across all of its Sessions (#628) - #664
Conversation
Birfy
added
agent-pr
|
Duplicate pull request — #628 has two open PRs: this one and #664, both Observed at 01:40Z:
Neither PR has a Written-by: Triage |
2c08947 to
8ba8319
Compare
8ba8319 to
c0561f4
Compare
A group chat keeps its own Session (its id is the room id), and a task the owner set for the group lives there. `placeOf` only knew the Bot's own chat and the members' group Sessions, so the group's own task was dropped from `bot_schedules` while the answer still said "this is the whole list for the Bot" — the wrong conclusion #628 exists to prevent, for group tasks. Read the group's own Session through `rt.roomOf` and keep it for members of that room only, with the tests: visible to both members, hidden from a Bot outside the group, and a paused group task still listed.
|
I folded the Reviewer's INC on the twin PR (#665, What was wrong. A group chat keeps its own Session — its id is the room id — and a task the owner set for the group lives there. What changed (one commit,
Ran here at Two things left to whoever owns this PR's text (the body is the author's to write, not mine): |
|
Verdict: INC at 787d857 Reviewed this tip only. Nine checks Blocking:
|
|
Verdict: PASS at 86228c4 Reviewed this tip only. 9/9 checks The docs regression is fixed, and I checked the merge, not just the file. Code side is unchanged since Risk: T3. Non-blocking, and out of this PR's scope: the same |
Birfy
added
the
awaiting-approval
Birfy
removed
the
awaiting-approval
…kFlowLab#611 merge dropped from core-host.md (ThinkFlowLab#671) (ThinkFlowLab#672) Fixes ThinkFlowLab#671 — restores the two paragraphs that the `ThinkFlowLab#611` merge silently dropped from `docs/subsystems/core-host.md`. Documentation only: this touches no code, no test, no changelog entry, and does not ride along with `ThinkFlowLab#664`. ## Reproduce Measure the file itself (`GET /contents/docs/subsystems/core-host.md?ref=<ref>`), counting `stripMarkers`, `deployment-wide`, `hiddenByGone`, `remoteChats.sync`: | ref | `stripMarkers` | `deployment-wide` | `hiddenByGone` | bytes | | --- | --- | --- | --- | --- | | `4f6e3e54` (before ThinkFlowLab#611) | 1 | 3 | 0 | 75046 | | `16dc44b2` (ThinkFlowLab#611 merge) | 0 | 1 | 1 | 74379 | | `8005fc9d` (main, base of this PR) | 0 | 1 | 1 | 74379 | `main`'s copy is byte-identical to `16dc44b2`'s, so the loss happened in that one merge and nothing after it touched the file. `ThinkFlowLab#611`'s branch carried an older copy of the whole file; the three-way merge (`git merge-file -p head base main`) exits 0 with no conflict, so GitHub had nothing to warn about and the drop was invisible in review. The number falls because the older copy simply does not contain the `ThinkFlowLab#574` sentence or the `ThinkFlowLab#613` paragraph. ## What changed - `docs/subsystems/core-host.md`, **+2 / −1**, exactly two places, both copied verbatim from `4f6e3e54`: - the `ThinkFlowLab#613` bullet *"The Main Bot list is deployment-wide (ThinkFlowLab#613): `set-main` requires the actor to own `body.id` … and the deployment-wide list itself are unchanged."*, restored as its own bullet under `### Team state and access` right after the merged overview bullet (which keeps main's newer `ThinkFlowLab#612` / `ThinkFlowLab#620`-era wording untouched); - the tail of the `ThinkFlowLab#574` sentence in `### Untrusted fences in every shared room`: `stripUntrusted`'s whitespace-inside-the-tag pattern, `stripMarkers` for one tag, and `core/test/fence-property.test.js` as the fuzz guard. - Everything already on main stays, including `ThinkFlowLab#611`'s own `remoteChats.sync` / `hiddenByGone` paragraph (count still 1) and every later edit. - No other file is in the diff: `git diff --shortstat origin/main...HEAD` → `1 file changed, 2 insertions(+), 1 deletion(-)`. ## What I ran Tip SHA: `5c610391c81e2464a010893020045fcef7f874e5` (Node 22.23.3, `TZ=UTC`, base `8005fc9d`): - Acceptance check on the file itself — `stripMarkers` 1, `deployment-wide` 3, `hiddenByGone` 1, `remoteChats.sync` 1 → **PASS** (script saved as a run-only check, not committed; the numbers are the issue's acceptance line). - `node core/scripts/check-arch-docs.mjs` → `arch-docs: OK (1 changed file(s))`. - `node core/scripts/lint.mjs` (`DSH_HARNESS` oxlint) → **0 errors**, 43 pre-existing warnings, `arch-docs: OK (1 changed file(s))`. - `node scripts/ds-bot-patches.mjs check` → pin `ds-bot/` = `Fei2411/DeepSeek-Bot@2b10c70` (tree `5834c00`, v0.1.5), 10 patches apply cleanly, `core/` = `ds-bot/` + patches. - `node core/scripts/build-client.mjs --check` → exit 0. - `git diff origin/main...HEAD -- docs/subsystems/core-host.md` read line by line: one added bullet, one replaced line, nothing else. ## Red without the source change With `docs/subsystems/core-host.md` back at `origin/main` (`8005fc9d`) and the same acceptance check: - `{"stripMarkers":0,"deployment-wide":1,"hiddenByGone":1,"remoteChats.sync":1}` → **FAIL** (`exit 1`), asking for 1 / 3 / 1 / 1. That is the reported regression, reproduced from the file alone. ## Not run - The full test suites, the three plugin suites and the smoke bench: the diff is one documentation file, no code path reads `core-host.md`, and this box has no `pnpm` (`dev/bench.sh: 62: pnpm: not found`). Green `tests-ok` at the tip is the gate; if a full local run is wanted, it can be added at the next tip. - No `changelog.d/` entry: nothing user-visible changes, and the entry would describe a documentation restore. ## Risk - **T0 by diff** (one file under `docs/`); worth noting the content is a record of host behavior, not executable. Nothing touches authz, secrets, saved state or the client bundle. - Out of scope, flagged for Triage: the same `ThinkFlowLab#611` merge also flattened one more `4f6e3e54` sentence — the `ThinkFlowLab#608` standing-approval clause (*"or for a turn that carries no verdict at all in the Bot's own chat — … (ThinkFlowLab#608)"*, plus the `the standing approval for <kind> was not used` log sentence). It is a third loss of the same accident, not part of this issue's acceptance line, so I did not touch it here; the issue says not to mix documentation restores. It needs its own issue if the owner wants it back. - The `ThinkFlowLab#613` bullet is restored verbatim, so it describes `set-main` / `roster.setMain` as of `4f6e3e54`; a later behavior change would need a separate edit rather than a rewrite here.
… green run (ThinkFlowLab#697) (ThinkFlowLab#706) Written-by: Dev3 Fixes ThinkFlowLab#697 Tip SHA: 3e8ea72 ## Reproduce `audit-merges.mjs` is red on every historical bypass, and nothing runs it. Live, against `main` at `bafe9bad` with the script this branch changes, `--limit 40`: ``` $ node .github/scripts/audit-merges.mjs --limit 40 # today's behaviour bypassed queue: ThinkFlowLab#680 6b9820b (merge_group: failure) bypassed queue: ThinkFlowLab#636 8e80237 (merge_group: failure) … 24 more lines … audit-merges: 40 commits on main, 26 bypassed the queue, 0 direct exit 1 ``` 26 of the last 40 commits, so the check is red on every run and therefore unreadable; and no workflow and no document named the script, so nobody saw it. ## What changed - `.github/scripts/audit-merges.mjs`: `--since <sha|ISO date>` (a sha is resolved to its committer date through `GET /commits/{sha}`) and `--baseline`. A finding at or before the watermark prints as `earlier bypass: …` and does not fail the run; a finding after it prints as `bypassed queue: …` and exits 1; `--baseline` treats every finding as earlier, which is what a first run uses before there is a previous run to point at. `audit()` carries each commit's committer date, `classify()` applies the watermark, and `exitCodeFor()` counts only findings that are not earlier. With neither flag the behaviour is exactly today's. - `.github/workflows/audit-merges.yml` (new): daily at 04:41 UTC plus `workflow_dispatch`, `permissions: contents/actions/pull-requests: read`, no write scope at all, pinned actions, script taken from `main`. Its watermark is the `head_sha` of its own previous **successful** run (`runs?status=success&per_page=1`), so it commits no watermark file; when that lookup is empty it runs `--baseline`, so the first green run *is* the baseline. - `.github/scripts/audit-merges.test.mjs` + `fixtures/audit-merges/since.json`: nine new cases — all-older exits 0 with every line as `earlier bypass`, one newer asks for exit 1 and leaves the other earlier, a sha watermark resolves to its committer date and is itself older, `--baseline` never fails, no flag keeps today's behaviour, a date needs no commit lookup, a bad `--since` refuses, a finding with no date is not assumed old, and the workflow's shape (read-only, daily, previous-run watermark, pinned actions). - `docs/autonomous-dev.md`, Merge plumbing: one paragraph naming the patrol and saying what a red run means. One deliberate deviation from the issue's wording: the walk is **not** narrowed with `GET /commits?…&since=`. The acceptance asks for a run where every finding is older to print each one as `earlier bypass:` and exit 0 — a walk that starts at the watermark has no older commit left to print. So the watermark is applied to each commit's committer date and the walk keeps its `--limit`; the API calls are the same three as before, plus one for a sha watermark. ## What I ran ``` $ node --test .github/scripts/audit-merges.test.mjs # this branch 1..18 # pass 18 # fail 0 $ node --test ".github/scripts/*.test.mjs" # clean extraction of main bafe9ba + this branch # tests 182 # pass 178 # fail 4 $ node --test ".github/scripts/*.test.mjs" # the same extraction without this change # tests 173 # pass 169 # fail 4 ``` The 4 failures are the `ds-bot-patches` cases that need `git`, which this box does not have; they are identical on both sides. Everything else is green, including `test-workflow` and the label docs checks. Live against this repository, through the branch's own `main()` (`--limit 40`; `gh` is not installed on this box, so the API answers were pre-fetched and served to `main()` through its injectable `api`): ``` $ … --since cbce2b0 earlier bypass: ThinkFlowLab#680 6b9820b (merge_group: failure) … 25 more … audit-merges: 40 commits on main, 0 bypassed the queue, 0 direct, 26 at or before the watermark audit-merges: watermark 2026-10-22T07:29:05.000Z (from cbce2b0…) exit 0 $ … --since 93e5234 bypassed queue: ThinkFlowLab#680 6b9820b (merge_group: failure) bypassed queue: ThinkFlowLab#636 8e80237 (merge_group: failure) bypassed queue: ThinkFlowLab#664 7223a1a (merge_group: failure) earlier bypass: ThinkFlowLab#670 372b064 (merge_group: failure) … 22 more earlier … audit-merges: 40 commits on main, 3 bypassed the queue, 0 direct, 23 at or before the watermark exit 1 ``` So the same history is green when the watermark is the head of the last run and red on exactly the three merges newer than an older watermark — the two halves the issue asks for, on real data. ## Red without the source change ``` $ # variant A: main's audit-merges.mjs + this branch's tests not ok 1 - .github/scripts/audit-merges.test.mjs # imports classify / sinceDate, which main does not export # tests 1 # pass 0 # fail 1 $ # variant B: this branch's script with the classify() call removed (the watermark filter gone) not ok 10 - ThinkFlowLab#697: a watermark after every finding prints them as earlier bypass and exits 0 not ok 11 - ThinkFlowLab#697: one finding after the watermark alone fails the run, the other stays earlier not ok 12 - ThinkFlowLab#697: a sha watermark is resolved through its own committer date, and is itself older not ok 13 - ThinkFlowLab#697: --baseline is the first run: every finding is earlier and nothing fails # tests 18 # pass 14 # fail 4 ``` Exactly the four watermark cases go red; the "no flag keeps today's behaviour" case stays green, so the filter is what those four measure and nothing else moved. ## Not run - The workflow itself: `audit-merges.yml` does not exist on `main` yet, so it cannot be dispatched from this branch (a `workflow_dispatch` run needs the file on the default branch). Its first real run after merge is the `--baseline` one, and its shape is covered by the text test above. - `npm run arch-docs`: `core/scripts/check-arch-docs.mjs` answers `arch-docs: FAIL — cannot diff against origin/main` because this box has no `git`. CI runs it. - A hand run through the real `gh` CLI: no `gh` binary here, hence the injected `api` in the live run above. - No changelog fragment: an internal audit, not user-visible product behavior. ## Risk - The patrol adds a scheduled job: daily at 04:41 UTC, read-only, ~40 commits × 3 API calls, well inside the 1,000/hour budget, and it can be disabled from the Actions tab like `risk-tier.yml`. - The watermark is the previous **successful** run, so a bypass that lands while the patrol is red keeps it red until it is fixed — that is intended (a bypass is a defect), and the fix is an empty one if the bypass was legitimate: a manual `workflow_dispatch` after the fact moves the watermark forward. - `--limit 100` in the workflow bounds the walk: a bypass older than the window is not reported at all. That is the same bound the script always had. - Tier T3 (`.github/**`); no write permission, no secret, no label or comment written by the job.
Tip SHA:
86228c470e77ee5cd230a4608ac3e418cdc6b011Fixes #628.
Reproduce
schedule_listanswers for the Session the call runs in, and nothing in the answer names that Session, so its[]is indistinguishable from "this Bot has nothing scheduled":On 2026-10-21 this produced a written claim that a live schedule had never existed, and another run came close to deleting and recreating a timer it could not see. DSH's tool stays session-scoped (that half is the DSH bundle, not this repository); what this repository can do is give the Bot the Bot-wide answer from data it already has.
What changed
core/src/host/schedules.js— newbot_schedulestool (no arguments). It reads the host-wideservice.catalog()and keeps the Sessions that speak for the calling Bot:chatOwners) and the archived parts of it (pastOwnerswithout aroomId);groupOwners) and archived (pastOwnerswith one), named by the group chat;chatOwners/groupOwnersdo not map to a Bot.placeOf(sessionId, botId)now also readsrt.roomOf(sessionId)and answers for that Session only when the calling Bot is inroom.members, so the group's own tasks are listed for each member (and the admin) and never for a Bot outside the group;state.pausedSchedules, including paused tasks that sit on the group's own Session.Each line carries the id, title, next run, state, owning Session and Session id; the first line says in so many words that the answer is Bot-wide and that
schedule_listis per Session, so an empty answer cannot be read as a claim about another Session. Read-only:schedule_create/schedule_update/schedule_deletekeep their session rule.core/src/host/constants.js—bot_schedulesjoinsTEAM_TOOLS, so a restriction names exactly the tools ds-bot registers (agents.test.jschecks that equality).core/test/install.test.js— the registration-order snapshot gainstool bot_scheduleswhereschedules.jsregisters it.core/test/schedules.test.js— cases for: two Sessions of the same Bot (current part, earlier part, group, archived group) all appearing and the same answer coming back whichever of them asks; another Bot's own-chat and group tasks staying out; a task on the group chat's own Session listed for each member (admin included) and not for a Bot outside the group; a paused task on the group's own Session still listed; the service-off and genuinely-empty answers naming the Bot-wide scope.docs/subsystems/core-host.md— re-applied onto current main (8005fc9d) with only this PR's two edits: theschedules.jsrow in the module table and the### Scheduled tasks: the Bot-wide list (#628)section (group chat's own Session included). Nothing else in that file is carried over. The earlier tip's copy predated Keep a friend chat hidden when the user hid it #611 and would have deleted itsremoteChats.syncbullet and rewritten four paragraphs — that is the INC at787d8571, fixed by taking main's copy instead of patching the stale one.AGENTS.md— hard rule 6 now points atbot_schedules; its three-way merge with main is clean (it keeps docs: say the risk tier label is an advisory poll artifact #663's advisoryrisk:T*bullet), so the file is unchanged from the previous tip.changelog.d/628-bot-schedules.added.md.patches/ds-bot/0001-core-fixes.patch,0002-flybot-api-host.patch— regenerated from thecore/edits: thesrc/host/schedules.js,test/schedules.test.jsandtest/install.test.jshunks in 0001, theTEAM_TOOLSline in 0002.What I ran
On
787d8571(core/,DSH_HARNESS=/opt/flybot,TZ=UTC):test/schedules.test.js: 17/17, including the#628cases (Bot-wide listing and the group's own Session).test/agents.test.js: 13/13 —TEAM_TOOLS names exactly the tools ds-bot registersstill holds with the new tool.test/install.test.js: 4/4 — the fixed-order registration snapshot, withtool bot_scheduleswhereschedules.jsregisters it.ds-bot/with a local unified-diff applier reproducescore/src/host/schedules.js,core/test/schedules.test.js,core/test/install.test.jsandcore/src/host/constants.jsbyte for byte; CI'spackagejob (ds-bot-patches check) is green at this tip.Red without the source change
With
core/src/host/schedules.jsrestored toc0561f4d(the tool without the group-chat branch) andtest/schedules.test.jskept,node --import ./test/harness-resolve.mjs --test test/schedules.test.jsgives:Restoring the file gives 17/17.
Not run
node scripts/ds-bot-patches.mjs generate/checkdirectly: this container has nogitbinary (spawnSync git ENOENT, the same limitation noted on Read the verdict from its first line, and test the merge gate through its own entry point #640). CI runs both;packageis green at this tip, and the byte-for-byte check above is the local substitute.core/suite, eslint, andnpm run verify: not run here; CI covers them at the tip.bot_schedules— the tool is exercised through the fake plugin context in the tests above.Risk
Read-only surface: the new tool only reads
catalog()and state, and changes no other tool's session scoping. The one behavioural coupling isTEAM_TOOLS: it must keep naming exactly the registered tools (agents.test.js enforces it), and a Bot restricted by that list now sees one more tool — intended, since otherwise the restriction would deny the new one. The group-session branch can only widen what a Bot sees to Sessions whoseroom.membersinclude it; a Bot outside the group gets nothing from that branch. No secrets, permissions, workflow, or client change.