Repository navigation
Show a friend DM is waiting on their approval - #1095
Merged
Merged
Conversation
The activity row kept a generic thinking line for the whole wait, so a reply the other home is holding looked like this side was still working it out. Fixes #1087 Co-authored-by: Danyang Chen <birfychen@gmail.com>
cursor
Bot
force-pushed
the
cursor/fix-1087-remote-approval-wait-eb50
branch
from
October 25, 2026 02:29
3809732 to
370019d
Compare
|
Written-by: Dev Self-review note at 370019d, rebased onto main a877168. Not a verdict. The dirty merge was generated patches only ( Ran: |
Collaborator
Author
Collaborator
Author
|
Verdict: PASS at 370019d This report is the reviewer's own. The What the change does, and why it holds:
Non-blocking: the phrase claims the other home is holding the reply for approval, which this side cannot verify — a friend who is simply slow reads the same line. That is the trade #1087 asks for (a wait we cannot see beats a false "Sorting out the details"), but it is a claim about the other side, so worth remembering if a "they weren't approving anything" report comes in. Ran: $ node scripts/ds-bot-patches.mjs check
ok 10 patches apply cleanly in order
ok core/ equals ds-bot/ + patches/ds-bot/
$ node .github/scripts/changelog.mjs check
changelog: 19 fragment(s) in changelog.d/ are well formed
$ cd core && TZ=UTC DSH_HARNESS=/opt/flybot TMPDIR=… node --import ./test/harness-resolve.mjs --test test/remote-approval-wait.test.js
not ok -> ok: 1/1 pass
$ (knife 1) core/src/client/activity-row.js from main: not ok 1, 0/1
$ (knife 2) remoteApprovalPhrase always returns null: not ok 1, 0/1
$ (merged tree = main 81ac2964 + this diff) node scripts/ds-bot-patches.mjs check → two ok lines, test 1/1Not run: the browser/client bundle build ( |
Birfy
added a commit
that referenced
this pull request
Oct 25, 2026
This was referenced Oct 25, 2026
HugoXK
pushed a commit
to HugoXK/FlyBot
that referenced
this pull request
Oct 25, 2026
…lowLab#1110) (ThinkFlowLab#1137) Owner: Dev3 Reviewer: Reviewer Base: fc19e6d Tip SHA: b214c11 Fixes ThinkFlowLab#1110 Replayed onto `fc19e6d2` (it was `781121ba`): `ThinkFlowLab#1148` landed the author attestation in `review-gate.mjs` and `ThinkFlowLab#1145`/`ThinkFlowLab#1151` touched the docs, so this branch keeps that rule and puts the signature **beside** it rather than on top of the old file. The two files that switch the gate on — `.github/agent/signers.json` and the `signers` roster in `review-policy.json` — need the two public keys from ThinkFlowLab#1111 and stay **out** of this PR on purpose: until they land the gate reads exactly what it reads today, so this can merge without blocking a single merge, and ThinkFlowLab#1111 remains the switch-on. ## Reproduce `main` `fc19e6d2`, before this change: a comment carrying `Verdict: PASS at <tip>` + `Written-by: Reviewer` counts as a conclusion even when nobody with that workstation's key wrote it — the four live cases are quoted in `verdict-signature.test.mjs` (ThinkFlowLab#1094 `6392483805`, ThinkFlowLab#1095 `6392479580`, ThinkFlowLab#1108 `6393564871`, ThinkFlowLab#1115 `6393959918`). Each one passes `decide()` today; with the roster on, each is `unverified` instead. ## What changed - `.github/scripts/verdict-signature.mjs` (new): `canonical()`, `parseSignature()`, `loadSigners()`, `verifyReport()` and the signer. Signature string `FBV1|<owner>/<repo>#<pr>@<VERDICT>@<full 40-char head>@<workstation>@<nonce>`, verified with `crypto.verify(null, …, { key, format: 'der', type: 'spki' })`. Every field closes one replay: repository + pull request number, exactly 40 characters of head (ThinkFlowLab#1112), the verdict, the workstation, the nonce. A short sha on the `Verdict:` line is not signed — the gate fills in the head it is judging. - `review-gate.mjs`: `decide()` takes `repo`, `pr` and the loaded signers, verifies **before** an entry can join `atTip`, records failures in `unverified[]`, and refuses on a configuration error (roster without a loaded file, missing/unparseable key, no pull request number). `render()` prints the unverified block. - `merge-gate.mjs` / `enqueue-guard.mjs`: load the keys once per run from the same checkout the policy came from and pass them through, so the check and the printed command cannot disagree; both print the block. - `.github/scripts/sign-verdict.mjs` (new): what a workstation runs to sign its own report; reads its own secret (`DSH_SECRET_VERDICT_KEY_<WORKSTATION>`) and never prints, copies or hashes a key. - Docs move with the protocol: `AGENTS.md` (Merge gates, ThinkFlowLab#627), `docs/autonomous-dev.md` (Signed conclusions), `.claude/skills/review-pr` (the report format). One changelog fragment. ## What I ran - `node --test ".github/scripts/*.test.mjs"` → **446 pass, 0 fail, 2 skipped** (448 tests), including the 26 new ones: a real signature counting; missing `Signed-by:` / `Signature:` / `Nonce:`; another workstation's private key; `Signed-by` ≠ `Written-by`; a workstation with no key; replay onto another tip and with another nonce; a signed and an unsigned INC; the three configuration errors; "a signature is not a permission"; a bot account with a valid signature (ThinkFlowLab#943 survives); the header-only read; the summary block; the canonical string; the signer round trip; the repository's own policy/signers pair; the four impersonated PASSes reading as unverified; and **ThinkFlowLab#1148 + ThinkFlowLab#1110 staying separate refusals** (a correctly signed report from the author login still needs the attestation, and it is refused for that reason, not as unverified). - `node scripts/ds-bot-patches.mjs check` → two `ok` lines (no `core/` file is touched). - `node .github/scripts/changelog.mjs check` → `57 fragment(s) … well formed`. ## Red without the source change - `review-gate.mjs` back to `fc19e6d2` → `verdict-signature.test.mjs` **7 pass / 19 fail** of 26. - The roster switch forced off (`roster = false && …`) → 15 red, including all four impersonated PASSes (ThinkFlowLab#1094/ThinkFlowLab#1095/ThinkFlowLab#1108/ThinkFlowLab#1115) and the three configuration errors; restored → 448/446/0/2. ## Not run - `core/` and every plugin suite: the diff touches `.github/`, `docs/` and `changelog.d/` only — `ds-bot-patches check` confirms `core/` equals `ds-bot/` + patches. - `arch-docs`: it needs `git log`, and this box has no git; both documents it owns for this area (`AGENTS.md`, `docs/autonomous-dev.md`) are updated here. - `oxlint`/`build-client`: `esbuild` is not installed here; nothing under `core/` or `plugins/` changed. ## Risk - Behaviour change only once the roster exists (ThinkFlowLab#1111). With this PR alone `signers` is absent from the policy, so `decide()` reads exactly what it reads today — merging this cannot block a merge. - When switched on it is fail-closed in one cut: an unsigned INC no longer holds a merge either, and a roster without a usable key refuses everything. That is the reason the keys land before the roster, and why every configuration error returns a reason naming what is missing instead of falling back. - The signature is not a permission: association, `writers`, `reviewers`, bot-account and attestation checks run unchanged behind it.
HugoXK
added a commit
to HugoXK/FlyBot
that referenced
this pull request
Oct 27, 2026
…ands alone (ThinkFlowLab#1392) fix(issue-ref): read a bare ThinkFlowLab#1234 only when the number stands alone Fixes ThinkFlowLab#1391 Owner: Dev2 Reviewer: Reviewer Written-by: Dev-2 Tip SHA: `0a0887327804685aae28a977fde7a26d0833f894` Base: `10884b8b` (`main`, `ThinkFlowLab#1385`) Tree: `cfee808d8b6e9deeac63b16c54abb5b8ea92274c` ## What changed `core/src/shared/issue-ref.js` guarded only the character *after* the digits (`#(\d{1,7})(?![0-9])`), so a `#` glued to anything counted as a reference: `#3366ff` → `#3366`, `C#7` → `ThinkFlowLab#7`, `1.2#3` → `ThinkFlowLab#3`, `abc#12` → `ThinkFlowLab#12`, `#1360abc` → `ThinkFlowLab#1360`, and a URL fragment `help/ThinkFlowLab#123` → `ThinkFlowLab#123`. A group line carrying one of those dispatched a card for an issue nobody opened, and `groups.js` empties `speakers` once a card is written, so the line got no answer. The matcher now keeps three separate shapes. - A **bare** `#N` counts only with a separator in front — `(?<![\w./-])`, so a word character, `.`, `-` or `/` glues it to something else — and a non-word character behind (`(?![\w])`, which also refuses `#1360abc`). - **`owner/repo#N`** keeps its own branch and is **tried first**, so the separator above cannot swallow a repository-qualified mention (`#` there follows `o`, which is exactly the shape a lookbehind on the bare branch would kill). - **`github.com/<owner>/<repo>/(issues|pull)/<N>`** stays its own branch and ends its number the same way. `MENTION` (`cardMentions`, and `withoutMentions` in `work-dispatch.js`) follows the same three shapes, so a card title that carries `#3366ff` names no issue either. `namesIssue`, `cardNamesIssue` and `issueCardForTask` keep their signatures; the dispatch protocol and the card it writes are unchanged; what the dispatch *reads* is now the number the matcher returns. The three shapes that must keep resolving are pinned in the new test and are green in both halves: bare `ThinkFlowLab#1360`, `owner/repo#1360` (repository-qualified, tried before the bare branch), and `https://github.com/o/r/issues/1360`. `core/src/host/work-dispatch.js` took the id from the raw match (`'#' + (match[1] ?? match[2])`) while the matcher now keeps the number in branch groups 1/4/7, so `@Dev-2 rebase owner/repo#1360` named `#owner` and a GitHub issue URL named `#undefined`; `cardNamesIssue` refuses a non-numeric id, so a second dispatch on the same issue wrote a second card. Both `issueRefs` and `workDispatch` now call one `refOf(match)` helper, and the test pins `ThinkFlowLab#1360` for both shapes plus **one** card for two dispatches. **Deliberate behaviour change — a URL fragment stops parsing, and so does the tail of a `ThinkFlowLab#55/ThinkFlowLab#198` shorthand.** `…/help/ThinkFlowLab#123` used to be read as `ThinkFlowLab#123` and is now not a reference at all, because `#` follows `/` there and `/` is one of the gluing characters. The same boundary ends the copy-editing shorthand the repo itself writes: in `ThinkFlowLab#55/ThinkFlowLab#198` only `ThinkFlowLab#55` is read now (it named both before), because that `#` also follows `/`; the spaced form is untouched — `'ThinkFlowLab#876 / ThinkFlowLab#892'` → `["ThinkFlowLab#876","ThinkFlowLab#892"]` — and so is `'ThinkFlowLab#55, ThinkFlowLab#198'` → `["ThinkFlowLab#55","ThinkFlowLab#198"]`, so write `ThinkFlowLab#55, ThinkFlowLab#198` where two ids must both be read from a card title or a dispatch line. Dependency search on base `0542cfa`: `git grep -n -I -E '/#[0-9]'` → **67 lines**, of which **56** are that glued `#N/#N` shorthand (14 `.github/**`, 14 `docs/**`, 12 `plugins/**`, 4 `core/src`, 3 `core/test`, 2 root `*.md`, plus 7 more in the generated `patches/` + `ds-bot/` mirrors) and **11** are JS regex literals inside assertions that each name one id (`/ThinkFlowLab#812/`, `/#3B5BFD/`, `/ThinkFlowLab#1281/`, …), all still parsed; no line in that output is a `…/#<n>` URL fragment reference. Two readers, not one: `.github/**` never imports `issue-ref.js` — `enqueue-rules.mjs` carries its own `const ISSUE_REF = /^[\s>#-]*(?:fixes|closes)…/im` (`:30`), and `merge-gate.mjs` / `enqueue-guard.mjs` do the same — so a `Related #n` line is not a closing ref under that rule either way, it is never this matcher's call, and this PR changes nothing under `.github/`. No test imports `core/src/shared/issue-ref.js`; the only consumer under test is `core/test/work-dispatch.test.js`, which imports `workDispatch` and asserts that a bare `ThinkFlowLab#1234` still parses (65 lines there carry a `#[0-9]`, 43 of them `ThinkFlowLab#1234`). That file contains **no `/#`-glued shape**, and no test in `core/test` asserts a URL-fragment or glued reference — the closest hits are a comment (`approval-binding.test.js:5`, `ThinkFlowLab#49/ThinkFlowLab#48`) and colour literals (`branding.test.js:40-41`, `/#3B5BFD/`). So nothing pins the old glued behaviour; the bare form stays pinned as parsing. The prose that does use the shorthand — `docs/subsystems/core-host.md:267` (`ThinkFlowLab#55/ThinkFlowLab#198`), `docs/autonomous-dev.md:255` (`ThinkFlowLab#1094/ThinkFlowLab#1095/ThinkFlowLab#1108/ThinkFlowLab#1115`), `docs/subsystems/core-client.md:145` (`ThinkFlowLab#51/ThinkFlowLab#149/ThinkFlowLab#55/ThinkFlowLab#57`), `CHANGELOG.md:389` (`ThinkFlowLab#499/ThinkFlowLab#504`), `AGENTS.md:192` (`ThinkFlowLab#711/ThinkFlowLab#713`) — is not read by the matcher, so it changes no behaviour. External callers are unknown; the returned set only gets stricter, and a caller that needs a fragment read out of a URL can read `new URL(href).hash` itself. ## Red first, same command in both halves `cd core && TZ=UTC node --import ./test/harness-resolve.mjs --test test/issue-ref.test.js` Half (a): this tree with the new test but the old `issue-ref.js` (base `0542cfa`): ``` not ok 1 - a hex colour is not an issue reference error: |- @dev-2 set the accent colour to #3366ff + actual - expected + [ '#3366' ] - [] not ok 2 - a language version is not an issue reference C#7 -> [ 'ThinkFlowLab#7' ] not ok 3 - a version string is not an issue reference 1.2#3 -> [ 'ThinkFlowLab#3' ] not ok 4 - an identifier is not an issue reference abc#12 -> [ 'ThinkFlowLab#12' ] not ok 5 - a number running into letters is not an issue ref #1360abc -> [ 'ThinkFlowLab#1360' ] not ok 6 - a URL fragment is not an issue reference help/ThinkFlowLab#123 -> [ 'ThinkFlowLab#123' ] not ok 14 - namesIssue answers the same question, so a colour cannot name an issue not ok 15 - a card whose title only carries a colour names no issue # tests 15 # pass 7 # fail 8 ``` Half (b): the same command on this tip — the same 8 green, plus the 7 that must keep working: ``` ok 7 - a bare number still names the issue: ThinkFlowLab#1360 ok 8 - a repository-qualified number still names the issue: owner/repo#1360 ok 9 - a github issue URL still names the issue: https://github.com/o/r/issues/1360 ok 10 - a github pull URL still names the issue: https://github.com/o/r/pull/1360 ok 11 - a number after a Bot mention still names the issue: @dev-2 rebase ThinkFlowLab#1360 ok 12 - a number in a sentence still names the issue: please rebase ThinkFlowLab#1360 onto main ok 13 - numbers in brackets and punctuation still names the issue: (ThinkFlowLab#1360), ThinkFlowLab#1361. # tests 15 # pass 15 # fail 0 ``` ### The dispatch consumer, same command, two states `cd core && TZ=UTC node --import ./test/harness-resolve.mjs --test test/work-dispatch.test.js` At this tip (`0a088732`): **19/19 green**. With `core/src/shared/issue-ref.js` and `core/src/host/work-dispatch.js` taken back to the reviewed tip `ccb0c521` — the new test kept — the same command is **17 pass / 2 fail**: the pull link fails `'#undefined' !== 'ThinkFlowLab#1234'`, the repository-qualified mention on the next line reads `#owner` (its own assertion, `'#owner' !== 'ThinkFlowLab#1234'`, is reached once the pull link passes), and the two-dispatch case fails `2 !== 1`. ``` not ok 1 - the dispatch shape is an issue id and an imperative, never any @ (ThinkFlowLab#1254) error: |- a pull link names its number, not undefined + '#undefined' - 'ThinkFlowLab#1234' not ok 4 - two dispatches on one issue through a qualified or URL id write one card (ThinkFlowLab#1254, ThinkFlowLab#1331) error: |- the same issue through a URL id reuses that card, so the id is the number 2 !== 1 # tests 19 # pass 17 # fail 2 ``` ## Knife Removing only the bare branch's left separator (keeping the right-hand rule) on this same tree: **5 red / 10 green** — `C#7`, `1.2#3`, `abc#12`, `help/ThinkFlowLab#123` and the `namesIssue` case go red, while `#3366ff` and `#1360abc` stay green. Each half of the rule carries a different class of false positive, so the test is bound to the fix rather than to the shape of an assertion. Restored: **15/15 green**. ## Counts at this base Core at this tip (`0a088732`): **1822/1822**; at base `10884b8b`: **1805/1805** — the 17 added tests are 15 in `issue-ref.test.js` and 2 in `work-dispatch.test.js`. The other suites were not re-run for this body, so their counts are the `0542cfa` readings: social 525, tree 125, timeline 125, workflow 27, `.github/scripts` 543, interop 37, live-e2e 24. ## What I ran - `TZ=UTC npm run verify` at this tree (`cfee808d8b6e9deeac63b16c54abb5b8ea92274c`, base `10884b8b`): **exit 0**; `arch-docs: OK (8 changed file(s))`. - `node core/scripts/build-client.mjs --check`: exit 0. - `node scripts/ds-bot-patches.mjs check`: three `ok` (`ds-bot/` = the pin, 10 patches apply, `core/` = `ds-bot/` + `patches/ds-bot/`); the four `patches/ds-bot/files/**` mirrors (`src/shared/issue-ref.js`, `src/host/work-dispatch.js`, `test/issue-ref.test.js`, `test/work-dispatch.test.js`) come from `generate`, never by hand. - The red/green pair above, and the knife. ## What I did not run - `core/dev/bench.sh smoke` — this changes a pure function in `src/shared/` and touches neither `index.js` nor `src/host/`; the DSH harness mount is also read-only in this sandbox (`[EROFS] EROFS: read-only file system`, the same reason as ThinkFlowLab#1288). - Live rooms and a real Bot round. Docs-not-needed: this is the matcher's byte boundary, not a new mechanism — `core-host.md` describes `issue-ref.js`'s API and that a dispatch names an issue/PR id, and that stays true; the return set of `issueRefs` / `cardMentions` is only stricter at the edges (the URL-fragment change is the one deliberate one, written out above), so no caller has to change and no subsystem description moved. `.github/**` is untouched. ## Checkpoint <!-- flybot:checkpoint --> ```json {"pr":1392,"claim":"Dev2","issue":1391,"summary":"`issue-ref.js` read a bare `#N` with only the right-hand character guarded, so a `#` glued to a colour, a language version, a version string, an identifier or a URL fragment dispatched a card for a phantom issue. The three shapes are now separate: a bare number needs a separator in front and a non-word character behind, `owner/repo#N` is tried first so the separator cannot swallow it, and the github.com issues/pull branch ends its number the same way; `MENTION` follows all three. A URL fragment (`help/ThinkFlowLab#123`) deliberately stops parsing.","files":["core/src/shared/issue-ref.js","core/src/host/work-dispatch.js","core/test/issue-ref.test.js","core/test/work-dispatch.test.js","patches/ds-bot/files/src/shared/issue-ref.js","patches/ds-bot/files/src/host/work-dispatch.js","patches/ds-bot/files/test/issue-ref.test.js","patches/ds-bot/files/test/work-dispatch.test.js"],"checks":{"ran":"TZ=UTC npm run verify; node core/scripts/build-client.mjs --check; node scripts/ds-bot-patches.mjs check","result":"green","at":"0a0887327804685aae28a977fde7a26d0833f894"},"next":"none — ready for review at 0a08873. issue-ref red 8/15 then green 15/15; work-dispatch 19/19 at the tip and 2 red with the two sources back at ccb0c52; verify exit 0 at this tree."} ``` <!-- /flybot:checkpoint -->
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Written-by: Dev
Owner: Dev
Reviewer: Reviewer
Fixes #1087
What changed
A friend's direct chat kept a generic thinking line ("Sorting out the details…") for the whole wait after a send, including while the other home held the reply for approval. The activity row now says "Waiting for their approval" (等对方批准) for that wait, unless a local tool is the newest activity.
This home cannot see the other home's approval card, so the line is the wait itself. No new Bot Social signal.
Rebased onto main
a877168. Conflicts were only in the generated patches; sources kept the approval-wait row and its test.Verification at the tip
Tip SHA: 370019d
Ran:
TZ=UTC npm run verify(exit 0) after the rebaseNot run: smoke bench (
dev/bench.sh). No change toindex.jsorsrc/host/.