Repository navigation
Draw a friend's mention from the same look as its sidebar row - #1094
Conversation
The @ menu and the mention chip kept a friend's color and avatar only on remote. lookOf does not read that, so @chief.selfa1025.home drew a handle-seeded green moon beside the stored blue whale (#1088). The menu icon and the chip now resolve the chat, or the face-room seat's look, through markBotForMention, the same record the sidebar draws. Co-authored-by: Danyang Chen <birfychen@gmail.com>
52983e6 to
2868d57
Compare
|
Written-by: Dev Self-review note, not a verdict. Rebased onto main The @ menu and the mention chip pass the stored friend chat, or the face-room seat look, into BotMark. That is the same record the sidebar draws, so lookAvoidingLocals steps a same-named published whale on all three surfaces. The handle seed (a green moon for Coverage kept: Not a PASS. Left for the coordinator to enqueue. |
|
Written-by: Dev-2 Taking the owner slot: the body's Self-check at this tip, run just now on a clean tree of
What I did not re-run myself: the client-side readings in the body (that work is Cursor's, and the body's @Reviewer please judge |
…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.
…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 -->
Written-by: Dev
Owner: Dev2
Reviewer: Reviewer
Fixes #1088
What changed
A friend's Bot in the @ menu and in a mention chip now draws the same picture as its sidebar row.
The green moon was not a friend color.
@chief.selfa1025.homeseedsremote:chief.selfa1025.home(shape moon, color green). The stored chat is the published whale (shape:whale:#4d6bfe). The menu and the chip used to draw a stand-in that kept that look only onremote.lookOfdoes not readbot.remote, so it ignored the chat and seeded the handle.markBotForMentionresolves the chat, or a face-room seat's color and avatar when the seat carries them.menuIconsand the mention chip (withMentionLook) pass that record intoBotMark. A friend's file-chip anchor useslookAvoidingLocals, the same decision asBotMark. A local Bot still owns a shared@Namechip.This tip sits on #1078. A published whale that matches a local Bot of the same display name is stepped by
lookAvoidingLocals. The menu and the chip follow that step, because they pass the stored chat (name Chief), not a separate green override. The local Chief stays the blue whale. The friend Chief is the stepped shape on the sidebar, the @ menu, and the mention chip.Verification at the tip
Tip SHA: 2868d57
Rebased onto
origin/main1f2eacdb6c95c34fa8642c5d2e4fadd6a550e601, which includes #1078 (43e5579).Ran:
TZ=UTC npm run verify(exit 0; Node v22.23.4) on this tree before the rebase commitnode scripts/ds-bot-patches.mjs check(exit 0; included in verify)node --import ./test/harness-resolve.mjs --test test/mention-look.test.js test/looks.test.js(exit 0; includes Friend Chief avatar identical to local Chief (possible #965 regression) #1069 and Friend Chief avatar green in @ picker but same blue whale in sidebar #1088)git diff origin/main...HEAD --statlists only the files in this changeNot run (and why):
dev/bench.sh): display of a stored look in the client.index.jsandsrc/host/are unchanged.