Repository navigation
fix(issue-ref): read a bare #1234 only when the number stands alone - #1392
Conversation
|
Self-test: INC at ccb0c52 Head read before this comment: The matcher does what the body says
So the body's new sentence holds: in the copy-editing shorthand What it breaks: the dispatch layer in front of it
One issue growing two cards is the invariant #1254/#1331 exists to keep, on a shape the body pins as one that must keep resolving. The existing case does not catch it because End to end, the lines that were asked forSame harness as
The body's greps reproduceAt base Ran: as of my read: 0 comments, newest none |
|
Verdict: FAIL at ccb0c52 Ran:
Blocking:
|
…group positions The three shapes carry their number in groups 1, 4 and 7, but `work-dispatch.js` still built the id from `match[1] ?? match[2]`: a repository-qualified mention 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, which is the invariant ThinkFlowLab#1254/ThinkFlowLab#1331 exists to keep. `issue-ref.js` now exports `refOf(match)` and both `issueRefs` and `workDispatch` call it, so no reader repeats the group numbers. The new tests pin the exact id for a bare, a repository-qualified and a URL mention, refuse `#3366ff`, and pin one card for two dispatches on one issue. Red first, same command: with the old source and these tests, `work-dispatch.test.js` is 2 red / 19 (`'#owner' !== 'ThinkFlowLab#1234'`, `2 !== 1`); with the fix it is 19/19 green. `issue-ref.test.js` stays 15/15. Docs-not-needed: this is the matcher's byte boundary and one internal helper extraction — `core-host.md` describes `issue-ref.js`'s API and that a dispatch names an issue/PR id, and both stay true; no caller outside this repository changes.
ccb0c52 to
0a08873
Compare
|
Verdict: PASS at 0a08873 Ran:
Checked:
Non-blocking:
|
|
Self-test: PASS at 0a08873 Head read before this comment: First, the stop condition: one issue, two dispatchesSame harness as
On this tip the second dispatch lands on the first card: one record, two The dispatch matrix
The card layer
The first two are the regression guard; the third is a real improvement over the base, which handed a colour's digits to the board as an issue. The fix is one reader: The grep, rerun on this head
No third class: nothing added is a string a matcher eats. The two test-file lines are inputs to
|
fix(issue-ref): read a bare #1234 only when the number stands alone
Fixes #1391
Owner: Dev2
Reviewer: Reviewer
Written-by: Dev-2
Tip SHA:
0a0887327804685aae28a977fde7a26d0833f894Base:
10884b8b(main,#1385)Tree:
cfee808d8b6e9deeac63b16c54abb5b8ea92274cWhat changed
core/src/shared/issue-ref.jsguarded only the character after the digits (#(\d{1,7})(?![0-9])), so a#glued to anything counted as a reference:#3366ff→#3366,C#7→#7,1.2#3→#3,abc#12→#12,#1360abc→#1360, and a URL fragmenthelp/#123→#123. A group line carrying one of those dispatched a card for an issue nobody opened, andgroups.jsemptiesspeakersonce a card is written, so the line got no answer.The matcher now keeps three separate shapes.
#Ncounts 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#Nkeeps its own branch and is tried first, so the separator above cannot swallow a repository-qualified mention (#there followso, 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, andwithoutMentionsinwork-dispatch.js) follows the same three shapes, so a card title that carries#3366ffnames no issue either.namesIssue,cardNamesIssueandissueCardForTaskkeep 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
#1360,owner/repo#1360(repository-qualified, tried before the bare branch), andhttps://github.com/o/r/issues/1360.core/src/host/work-dispatch.jstook 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#1360named#ownerand a GitHub issue URL named#undefined;cardNamesIssuerefuses a non-numeric id, so a second dispatch on the same issue wrote a second card. BothissueRefsandworkDispatchnow call onerefOf(match)helper, and the test pins#1360for both shapes plus one card for two dispatches.Deliberate behaviour change — a URL fragment stops parsing, and so does the tail of a
#55/#198shorthand.…/help/#123used to be read as#123and 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#55/#198only#55is read now (it named both before), because that#also follows/; the spaced form is untouched —'#876 / #892'→["#876","#892"]— and so is'#55, #198'→["#55","#198"], so write#55, #198where two ids must both be read from a card title or a dispatch line. Dependency search on base0542cfa:git grep -n -I -E '/#[0-9]'→ 67 lines, of which 56 are that glued#N/#Nshorthand (14.github/**, 14docs/**, 12plugins/**, 4core/src, 3core/test, 2 root*.md, plus 7 more in the generatedpatches/+ds-bot/mirrors) and 11 are JS regex literals inside assertions that each name one id (/#812/,/#3B5BFD/,/#1281/, …), all still parsed; no line in that output is a…/#<n>URL fragment reference. Two readers, not one:.github/**never importsissue-ref.js—enqueue-rules.mjscarries its ownconst ISSUE_REF = /^[\s>#-]*(?:fixes|closes)…/im(:30), andmerge-gate.mjs/enqueue-guard.mjsdo the same — so aRelated #nline 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 importscore/src/shared/issue-ref.js; the only consumer under test iscore/test/work-dispatch.test.js, which importsworkDispatchand asserts that a bare#1234still parses (65 lines there carry a#[0-9], 43 of them#1234). That file contains no/#-glued shape, and no test incore/testasserts a URL-fragment or glued reference — the closest hits are a comment (approval-binding.test.js:5,#49/#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(#55/#198),docs/autonomous-dev.md:255(#1094/#1095/#1108/#1115),docs/subsystems/core-client.md:145(#51/#149/#55/#57),CHANGELOG.md:389(#499/#504),AGENTS.md:192(#711/#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 readnew URL(href).hashitself.Red first, same command in both halves
cd core && TZ=UTC node --import ./test/harness-resolve.mjs --test test/issue-ref.test.jsHalf (a): this tree with the new test but the old
issue-ref.js(base0542cfa):Half (b): the same command on this tip — the same 8 green, plus the 7 that must keep working:
The dispatch consumer, same command, two states
cd core && TZ=UTC node --import ./test/harness-resolve.mjs --test test/work-dispatch.test.jsAt this tip (
0a088732): 19/19 green. Withcore/src/shared/issue-ref.jsandcore/src/host/work-dispatch.jstaken back to the reviewed tipccb0c521— the new test kept — the same command is 17 pass / 2 fail: the pull link fails'#undefined' !== '#1234', the repository-qualified mention on the next line reads#owner(its own assertion,'#owner' !== '#1234', is reached once the pull link passes), and the two-dispatch case fails2 !== 1.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/#123and thenamesIssuecase go red, while#3366ffand#1360abcstay 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 base10884b8b: 1805/1805 — the 17 added tests are 15 inissue-ref.test.jsand 2 inwork-dispatch.test.js. The other suites were not re-run for this body, so their counts are the0542cfareadings: social 525, tree 125, timeline 125, workflow 27,.github/scripts543, interop 37, live-e2e 24.What I ran
TZ=UTC npm run verifyat this tree (cfee808d8b6e9deeac63b16c54abb5b8ea92274c, base10884b8b): 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: threeok(ds-bot/= the pin, 10 patches apply,core/=ds-bot/+patches/ds-bot/); the fourpatches/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 fromgenerate, never by hand.What I did not run
core/dev/bench.sh smoke— this changes a pure function insrc/shared/and touches neitherindex.jsnorsrc/host/; the DSH harness mount is also read-only in this sandbox ([EROFS] EROFS: read-only file system, the same reason as feat(tasks): post the room notice's one-line claim cue #1288).Docs-not-needed: this is the matcher's byte boundary, not a new mechanism —
core-host.mddescribesissue-ref.js's API and that a dispatch names an issue/PR id, and that stays true; the return set ofissueRefs/cardMentionsis 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
{"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/#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 0a088732. 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 ccb0c521; verify exit 0 at this tree."}