Skip to content

Prune the waiting-notice ledger when a hold leaves - #660

Merged
Birfy merged 1 commit into
ThinkFlowLab:mainfrom
HugoXK:fix/social-notices-bound
Oct 22, 2026
Merged

Birfy merged 1 commit into
ThinkFlowLab:mainfrom
HugoXK:fix/social-notices-bound

Conversation

@HugoXK

@HugoXK HugoXK commented Oct 22, 2026

Copy link
Copy Markdown
Contributor

Prune the waiting-notice ledger when a hold leaves
settings.approvalNotices gets one entry each time Bot Social holds a send (syncApprovalNotices) or the owner rejects one (postRejectNote), and nothing ever deleted them, so the per-server flybot-social-settings*.json grew for the life of the install. Its only job is to keep one "Waiting for the owner's approval" note per hold; a hold that is no longer pending never needs its entry again.
syncApprovalNotices now drops the entry of every hold that is not in the current pending set, so the ledger stays as small as the pending queue. The sibling maps ownLineApprovals and holdRooms already cap at 200; this one is bounded by the live queue instead.
plugins/social/test/social.test.js lists one hold (entry written) and then an empty queue (entry must be gone) — it fails without the change (the entry stays in the settings file). docs/subsystems/social.md records the bound.
Ran at the tip: npm run verify (green, TZ=UTC). Not run: smoke (this macOS box has no setsid, so dev/bench.sh cannot start dsh web), Desktop, real-model steward.

settings.approvalNotices gained one persisted entry per hold ever seen
and nothing removed them, so the per-server settings file grew for the
life of the install. Its only job is one waiting notice per pending
hold, so a sync now drops the entry of every hold that is no longer
pending.
@Birfy Birfy added area:social Friends, invites, grants, Bot Social bug Something isn't working labels Oct 22, 2026
@Birfy

Birfy commented Oct 22, 2026

Copy link
Copy Markdown
Collaborator

Verdict: PASS at 3850867
Written-by: Reviewer
Posted-by: Reviewer

Tip SHA: 3850867d3b63c4d40ff423404e27136d66af760f
CI at tip: 9/9 success (tests-ok, package, plugins social/tree/timeline, merge-gate, node 24)
Tier (computed from the diff by risk-gate.mjs + risk-rules.json from the base commit): T3 — plugins/social/src/**
Changed files: 4 (+30 / -1); mergeable_state: clean

Ran at this tip (tarball of the tip, nothing patched by hand):

$ cd plugins/social && TZ=UTC DSH_HARNESS=/opt/flybot node --import ./test/resolve.mjs --test "test/*.test.js"
# tests 318 / pass 318 / fail 0

Gate ③, the source change reverted with the tests kept:

plugins/social/src/social.js -> c74e1207 (main):
  not ok 49 - the waiting-notice ledger drops a hold once it is no longer pending
  276 tests, 275 pass, 1 fail

The bound is at the right place. The prune runs in the same call as the add, against the same set the add loop consumes (held), so a hold that is still pending cannot lose its entry and cannot get a second waiting note. Three details I checked in the code rather than only in the test:

  • a failed list_approvals returns pendingCache.items from the catch before syncApprovalNotices runs, so a network error cannot empty the ledger and cause a re-post storm on recovery;
  • the deletion loop snapshots Object.keys before deleting, and changed is only set when something was actually removed, so the settings file is not rewritten on every 5 s poll;
  • the sibling bounds are untouched: ownLineApprovals/holdRooms still cap at 200, and this map is now bounded by the live pending queue, which is what the doc line claims.

Two non-blocking notes, for the record, not reasons to hold:

  1. The prune also drops kind: 'rejected' entries (postRejectNote), whose only reader is the same dedupe check. If an approval id that was rejected ever came back through list_approvals as pending (a server-side flap or a failed decide_approval whose reject note already went out), the waiting bubble could be posted a second time. The server does not normally do that, and the ledger's stated job is "one note per hold"; if it ever shows up, keep rejected entries under their own small cap instead of in the pending set.
  2. The new test asserts the waiting entry only; the rejected path has no case here. Fine for this diff, worth a line if the behaviour above ever changes.

Not run: the full npm run verify in this sandbox (the checkout has no node_modules); CI's tests-ok, package and the three plugin suites cover it.

Not enqueued: T3 needs the maintainer's approve together with this PASS. No earlier conclusion exists at this tip — this is the first and only one.

@Birfy Birfy added the awaiting-approval PR is green and waits for the maintainer's approval (T2/T3) or a review PASS at its tip label Oct 22, 2026
@Birfy
Birfy added this pull request to the merge queue Oct 22, 2026
Merged via the queue into ThinkFlowLab:main with commit ef7a29d Oct 22, 2026
9 checks passed
Birfy added a commit to Birfy/FlyBot that referenced this pull request Oct 22, 2026
Owner decision 2026-10-22 09:57 Asia/Shanghai: the gates no longer check for "Not run:". A PASS still
needs at least one Ran command, at the head; INC, hold and a wrong sha still refuse. The four real
reports ThinkFlowLab#660-ThinkFlowLab#663 now pass the queue entry as written.
@Birfy Birfy removed the awaiting-approval PR is green and waits for the maintainer's approval (T2/T3) or a review PASS at its tip label Oct 22, 2026
Birfy added a commit to Birfy/FlyBot that referenced this pull request Oct 23, 2026
…g Not run (ThinkFlowLab#668)

Fixes ThinkFlowLab#667

Every queue entry since ThinkFlowLab#658 refused `Birfy's PASS lists no command it
ran` (ThinkFlowLab#660–ThinkFlowLab#663). The reviewer writes `Ran at this tip (…):` over a
fenced block of `$ ` commands. `review-gate.mjs` read only a bare `Ran:`
(`:65`) plus `- ` bullets (`:72-77`), and it had stripped every backtick
first (`:32`), so the fence was invisible.

- A `Ran …:` heading now counts (`/^Ran\b[^:]*:/`). Its commands are the
`$ ` lines of the fenced block below it, or `- ` bullets as before.
Words after a longer heading's colon are not a command.
- Still refused: an empty `Ran:`, `Ran: -`, output-only blocks and a
bare `$`; a PASS at another sha; INC; `hold`. A quoted block's `Ran:`
still cannot lend the conclusion evidence (ThinkFlowLab#640).
- **Owner decision 2026-10-22 09:57 Asia/Shanghai: 「那还是不检测这个了吧」. The
gates no longer require a `Not run:` line.** `decide` drops the "has no
Not run line" refusal. `Not run` stays recommended in
`.claude/skills/review-pr`, docs/code-standards.md (Verifier honesty 3)
and docs/autonomous-dev.md:147. AGENTS.md rule 4 never named it, so it
is unchanged.
- Fixtures: the four real PASS comments, verbatim, in
`.github/scripts/fixtures/review-reports-660-663.json`. Through
`merge-gate.mjs` `main()` with the real `review-policy.json`, **all four
pass as written**. The same reports with a wrong head or with INC
refuse.

No changelog fragment: CI-only, as in ThinkFlowLab#640 and ThinkFlowLab#658.

## Verification at dfdc6ad
- `ARCH_DOCS_BASE=origin/main TZ=UTC npm run verify` → pass (core 1204,
social 319, tree 89, timeline 105, .github/scripts 149, interop 37,
live-e2e 16; 0 fail).
- review-gate + merge-gate tests → 59/59. Against main's
`review-gate.mjs` → 9 fail. Against this PR's previous tip f12fda7 (Not
run still required) → 3 fail.
- Not run: a real merge_group run; smoke (no host code).

## Landing (T3)
This PR's queue entry runs **main's** `review-gate.mjs` (from `base/`),
which still wants a bare `Ran:` with bullets **and** a `Not run:` line.
The self-review at this tip is written in exactly that shape, so the
entry is expected to pass `merge-gate` as long as that PASS stays the
newest conclusion at the tip. Any newer report has to keep that shape
too. If it doesn't, `merge-gate` reds once, but it isn't a required
check, and the queue merges on `tests-ok`: `gh pr merge 668 --auto
--squash --match-head-commit dfdc6ad`.
No admin-merge, no bypass.

---------

Co-authored-by: Birfy <15378933+Birfy@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:social Friends, invites, grants, Bot Social bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants