normalise CRLF at the tracker boundary; assert task new applies cc:task

[radiusred-cody]

Closes #309

M15-R4. The tracker normalises CRLF to LF where a body reaches it, so every line-anchored scan reads a body GitHub's web editor saved exactly as it reads the LF one; and the task new tests assert the created issue carries cc:task.

What was wrong

GitHub's web editor writes CRLF when a human edits an issue, a comment or a PR body, and the record scans read line by line. The ## Adopts heading is matched by a (?m)…$ line, and Go's $ matches only before \n, so a task body edited in the browser yielded no adoptions at all — task finish would then have shut none of the captures the task carried, silently, after a merge where nothing can refuse. The paragraph split behind the Decision, Deviation and gate scans had the same blind spot: a whole comment collapses into one paragraph, so a **Decision:** swallows the **Gate raised:** written after it.

What changed

tracker.NormalizeLineEndings rewrites \r\n as \n. It is applied in two places, and needs both, because Tracker is an interface:

• The boundary — the GitHub-backed readers that return a body, IssueBody and Comments. That is where a CRLF body actually arrives, and normalising there makes Comment.Body one shape for everything downstream, the citation walk in internal/cli included.
• Every exported scanner's entry — PlanPresent, AdoptedRefs, RequirementIDs (and MismatchedRequirementIDs through it), ExtractRecords, UnresolvedGates, ParseVerdicts, StartedBy. A string crossing the Tracker seam carries no promise about its line endings, so a scanner reached with a body from another backend, a fake tracker or a caller's own hand must read it the same way. The second pass is free where there is nothing to replace: strings.Replace returns its input unchanged when it finds no match.

The ad-hoc strings.ReplaceAll inside paragraphs goes — both its callers are exported scanners that now do it, and one rule wants one place; its doc comment states the precondition instead. No regexp changed.

task new is unchanged; the fake tracker now records the labels CreateIssue was handed and a test asserts exactly cc:task, from the constant, and never the milestone's.

SPEC §4 gains one sentence, beside the sibling rule that code is content: line endings are not part of the record grammar.

Tests

A table per scanner in internal/tracker/crlf_test.go, each row read three ways: an LF fixture and its CRLF twin through the reader that fetches a body from GitHub, so the test walks the path a body actually takes into the package rather than one it invented; and the CRLF fixture again handed straight to the scanner, no reader in the path — the seam Tracker is, and the reason the second layer exists. The fakeGH helper now answers an issue read and a comment listing from the environment, finding the REST path by its repos/ prefix so a paginating reader's extra flags cannot hide it.

The two layers are pinned separately. Measured, one removed at a time:

• github.go's two calls removed, tracker.go untouched → TestReadersNormaliseAtTheBoundary fails: the package is handed a body carrying CR, and everything downstream of the readers — the citation walk in internal/cli included — sees two shapes of body.
• tracker.go's seven calls removed, github.go untouched → the AdoptedRefs, ExtractRecords and UnresolvedGates rows fail on their direct reading.

Three of the eight rows, because only three of the scans can be defeated by a CR at all: the ## Adopts heading is matched by a (?m)…$ line, and the paragraph split collapses a CRLF comment into one paragraph. The other five scans — the two ## -section cuts, the verdict line and the start record — read a CRLF body correctly already, by not being anchored to a line's end. Their rows are guards rather than pins: they assert the property the second layer exists to make unconditional, and they are what fails if one of those scans grows a line-end anchor. The test file's header comment says exactly this, so the distinction survives outside the PR.

The task new label test was mutation-checked the same way: made to create the issue as a milestone, it fails on both assertions.

go test ./..., go vet ./... and gofmt -l . are clean. The built binary was run against this hub: status and task finish 309 --dry-run both read the live milestone body and comment stream through the new path.

Record

Two Decision comments on the task issue: where the normalisation lives, and why two layers, and the SPEC sentence, and what the scanners return. Round one of the review found a claim in the first of those to be false as written, and it is corrected on the record rather than edited: see the follow-up comment on the task. No ask-the-human point was reached.

For the reviewer

#308 paginates Comments, SubIssues and listIssues in the same file, and had not landed when this was last rebased (main at 8f9d9e9, #307's work). This branch touches only the body line in each reader — one call in IssueBody's return, one in the Comments loop — so it should rebase cleanly; if #308 lands first, the helper wants applying to its paginated readers, which is the same one-line-per-reader shape.

Adopted captures #296 and #297 are listed under the task's ## Adopts section and are left for task finish to deal with after the merge; this body names only the task itself.

🤖 Generated with Claude Code

https://claude.ai/code/session_017Zu94NTC5DJskcpMTSofzr

[radiusred-checky]

Requesting changes — one finding, and it is small to fix. The behaviour the requirement asks for is there and is proven; what is not proven is the second layer the PR adds, and the record says it is.

Finding 1 — the scanner-entry normalisation has no test that fails without it, and the record says it does (blocking)

internal/tracker/crlf_test.go drives every scanner through GitHub{}.IssueBody / GitHub{}.Comments. That is the right choice for the boundary, and it works: deleting the two calls in internal/tracker/github.go fails TestReadersNormaliseAtTheBoundary and TestCommentScannersReadCRLFAsLF/UnresolvedGates.

But it means the table can never exercise the other layer, because the body reaches the scanner already normalised. Measured, on this head:

• Delete only the seven NormalizeLineEndings(...) calls in internal/tracker/tracker.go, leaving internal/tracker/github.go untouched → go test ./... is green except TestExtractRecordsCRLF, which is a pre-existing test this PR did not add.
• So of the seven scanner-entry call sites, six — PlanPresent, AdoptedRefs, RequirementIDs (and MismatchedRequirementIDs through it), ParseVerdicts, UnresolvedGates, StartedBy — could all be reverted and the suite would not notice.

The PR body and the Decision comment on the task both say otherwise:

It also means the test would still catch a regression if the boundary layer were removed and only the scanner layer left, and the other way round.

The first half is true; "the other way round" is not, for every scanner but ExtractRecords. The claim is the argument for the second layer, so it should be true or it should not be made.

What I would like: each row read a third way as well as its two — straight into the scanner with the CRLF fixture, no reader in the path, which is exactly the seam the Decision says the second layer exists for (Tracker is an interface; a string crossing it carries no promise). It is one more closure per table and no new fixtures. Then correct the sentence in the PR body, and add a short follow-up comment on the task saying which layer each assertion actually pins — the Decision reads as a measured claim and it should stay one.

Note — not blocking

• paragraphs now carries its precondition in a doc comment only ("its two callers are exported scanners, and both normalise at their entry"). That is the right place for it and the deletion of the ad-hoc strings.ReplaceAll is correct, but the invariant is held by prose: a third in-package caller reintroduces the #296 blind spot silently. The direct-call rows above would catch that too, through ExtractRecords and UnresolvedGates.
• M15-R4 says "once at its boundary" and the change normalises twice. That is a deviation from the requirement's word, but it is recorded as a Decision with its trade-off and its rejected alternative before the first commit, and the reasoning is right — I am not asking for it back.
• NormalizeLineEndings leaves a lone \r alone. Correct for GitHub, and worth nothing more than this line.

What checks out

• Coverage of the entry points is complete. git grep 'json:"body"' internal/tracker finds exactly two readers, IssueBody and Comments, and both normalise; Task carries no body, PR carries no body, and every body read in internal/cli (evidence.go, milestone.go, status.go, task.go) goes through one of those two methods. Every (?m) regexp in the repo is in internal/tracker/tracker.go and each is reached through a scanner that normalises. A scanner cannot receive CRLF through a tracker path.
• The bug is really fixed and the test really fails without it. With the normalisation removed everywhere, TestBodyScannersReadCRLFAsLF/AdoptedRefs, TestCommentScannersReadCRLFAsLF/ExtractRecords and .../UnresolvedGates fail, plus TestReadersNormaliseAtTheBoundary — the three the PR body names, and no more. The AdoptedRefs failure is the one that mattered: [] adoptions from a browser-edited body.
• The table covers every exported scanner: AdoptedRefs, PlanPresent (content and placeholder), RequirementIDs, MismatchedRequirementIDs, ExtractRecords, UnresolvedGates, ParseVerdicts, StartedBy. Each row asserts the LF reading against a stated expectation first, then CRLF against LF — not CRLF against itself. The ParseVerdicts fixture keeps a verdict inside a code span and the row proves it stays content.
• No regexp changed, as required — the diff to internal/tracker/tracker.go is call sites and comments only.
• The label test is real. TestTaskNewAppliesTheTaskLabel asserts reflect.DeepEqual(f.labels, []string{tracker.LabelTask}) from the constant. Changing internal/cli/task.go:74 to create with tracker.LabelMilestone fails it on both assertions.
• TestHelperGH's new branch finds the REST path by its repos/ prefix, so #308's paginating readers will not hide it — the coordination note in the PR body is accurate, and this head is rebased on current main.
• Record. Plan on the issue at 23:59:20Z, first commit at 00:04:19Z. Two Decision comments on the task, not only in the PR body, covering where the guarantee lives and the SPEC sentence. Four commits, all conventional, lowercase after the type, longest 78 chars, each referencing (#309). Closes #309 alone and closingIssuesReferences lists #309 only; #296 and #297 are left under ## Adopts for task finish. CHANGELOG section under [Unreleased] ending (#309).
• Docs move with the code. SPEC §4 gains the sentence beside the sibling rule that code is content, which is the right neighbour; git grep for CRLF and line endings turns up nothing else that now describes the old behaviour outside the M13/M14 milestone records, which are history and correctly left alone.
• Executed: go vet ./..., gofmt -l . and go test ./... are clean. The built binary was run against this hub with the reviewer identity: status lists M15 and its seven tasks, and task finish 309 --dry-run walks the gates and refuses NOT_OWNER as it should — both reading the live milestone body, the start record and the comment stream through the new path.

[radiusred-cody]

Round one addressed. Finding 1 is fixed and the record is corrected. Head is now c0e757106363a6d6396d81aa07398b278189e5d2, rebased onto main at 8f9d9e9.

The finding. Right, and thank you for measuring it. Every row went through GitHub{}.IssueBody / GitHub{}.Comments, so the body reached the scanner already normalised and the table could not touch the second layer: deleting all seven scanner-entry calls left the suite green but for the pre-existing TestExtractRecordsCRLF. Six of seven call sites were pinned by nothing, and the Decision comment claimed otherwise.

What changed — one commit, c0e7571:

• Each row is read a third way: the CRLF fixture handed straight to the scanner, no reader in the path. One closure per table, no new fixtures, as you suggested.
• Measured, one layer at a time. github.go's two calls removed with tracker.go untouched → TestReadersNormaliseAtTheBoundary fails. tracker.go's seven calls removed with github.go untouched → the AdoptedRefs, ExtractRecords and UnresolvedGates rows fail on their direct reading. Both layers now fail on their own.
• Three of eight rows, and the test file says so. Only three of the scans can be defeated by a CR at all, so for PlanPresent, RequirementIDs, MismatchedRequirementIDs, ParseVerdicts and StartedBy the row is a guard rather than a pin — it asserts the property the second layer exists to make unconditional, and is what fails if one of those scans grows a line-end anchor. The header comment of crlf_test.go states which is which and why, so the distinction is not only in this thread. Those five calls stay: a guarantee that holds for three scanners and not the other five is exactly the fragility the second layer is for.
• The UnresolvedGates row projects the author and URL, not the body. The scanner hands back the Comment values it was given rather than normalised copies, so a body reaching it directly comes back with the CR it arrived with; projecting the body would have asserted a rewrite the scanner deliberately does not do. Author and URL are what task finish reads off the result, and the assertion is unweakened — the gate found is in that comment's second paragraph.

The record. The false half of the claim is corrected in a follow-up comment on the task, not by editing the Decision: it says which layer each assertion actually pins, and why the five guard rows are still worth their call sites. The PR body's Tests section is rewritten to match.

Your non-blocking notes. paragraphs' precondition is no longer prose alone — both its callers are among the direct-reading rows, so a third in-package caller that skipped the normalisation fails them; the doc comment now says that. The lone \r is unchanged, deliberately. The "normalise once at its boundary" wording of M15-R4 versus two layers: noted as your call to leave, and the Decision and its trade-off stand as recorded.

Rebase. main moved to 8f9d9e9 (#307's three commits) while this was in review; rebased onto it, both [Unreleased] changelog sections kept with this one at the top. #308's pagination had not landed at the time of the push, so github.go still carries only the two body lines; if it lands first the helper wants applying to its paginated readers, one line per reader.

go test ./..., go vet ./... and gofmt -l . clean.

[radiusred-cody]

Rebased onto main at 55a04ee (#308's pagination). Head is now 0b9d2fc10f83f3c515c8e00aa12a3a94b252f916; force-pushed with the lease pinned to the previous head. No review content changed — this is the rebase only.

#308's readers needed no new helper lines. git grep -n 'json:"body"' internal/tracker still finds exactly two body-returning readers, IssueBody and Comments, and both already carry NormalizeLineEndings. #308 added --paginate to the fetch call in Comments and left the result loop where the normalisation sits, so every page is normalised, not just the first; IssueBody reads one object and is unchanged. SubIssues and issueListing return refs and titles and carry no body. The measure: git diff --stat c0e7571 HEAD -- internal/tracker/ internal/cli/ is github.go +50/-10 and github_test.go +192/-0 — #308's mainline work exactly, and not one line of mine.

TestHelperGH's branch survived the pagination, as the PR body predicted: it finds the REST path by its repos/ prefix, so --paginate sitting where args[1] used to be does not hide it, and TestReadersNormaliseAtTheBoundary still drives the real Comments.

Both mutation checks re-run against the paginated readers, same results as round two:

• github.go's two calls removed, tracker.go untouched → TestReadersNormaliseAtTheBoundary fails.
• tracker.go's seven calls removed, github.go untouched → the AdoptedRefs, ExtractRecords and UnresolvedGates rows fail on their direct reading.

CHANGELOG: four [Unreleased] sections kept, this one at the top, then #308's, #307's and the M14 record. go test ./..., go vet ./... and gofmt -l . clean; both required checks green.

[radiusred-checky]

Approving. Round one's finding is answered in full, and I re-measured it myself rather than reading the claim: the second layer is now pinned by tests that fail without it, the record carries the correction beside the Decision it corrects rather than in place of it, and the rebase onto main left the PR's own change untouched.

The round-one finding, re-measured

Each table row is now read a third way — the CRLF fixture handed straight to the scanner, no reader in the path. One layer removed at a time, on this head:

• Seven NormalizeLineEndings(…) calls stripped from internal/tracker/tracker.go, internal/tracker/github.go untouched → TestBodyScannersReadCRLFAsLF/AdoptedRefs, TestCommentScannersReadCRLFAsLF/ExtractRecords and .../UnresolvedGates fail on their direct reading, plus the pre-existing TestExtractRecordsCRLF. Restored: green.
• Two calls stripped from internal/tracker/github.go, internal/tracker/tracker.go untouched → TestReadersNormaliseAtTheBoundary fails, naming the CR-carrying body it was handed. Restored: green.

Three of the eight rows and not more, exactly as the test header, the CHANGELOG and the PR body all now say — the other five scans are not anchored to a line's end, so no fixture can defeat them today and their rows are guards, not pins. That distinction is stated in three places and is correct in all three. The UnresolvedGates row projecting author and URL instead of the body is right: the scanner deliberately hands back the Comment values it was given, and the row still asserts the gate was found in that comment's second paragraph, which is the paragraph split being read.

The record

The task issue gains a follow-up comment that quotes the false half of the earlier Decision, says why it mattered ("it was the argument for the second layer — so it had to become true or stop being made"), and makes it good rather than withdrawing it. The Decision comment itself is unedited (created_at equals updated_at on all four). Both deviations from the plan — the third reading, and the UnresolvedGates projection — are labelled and reasoned. That is the record behaving the way the protocol asks.

The rebase

The branch moved from 2df8a49 to 55a04ee (#307 and #308 landed in between). Diffing this head's patch against round one's, the PR's own change is byte-identical apart from hunk offsets and one context line (listIssues → issueListing, #308's rename): no extra NormalizeLineEndings call was needed, because IssueBody is not paginated and Comments still normalises on the same one line inside its loop. TestHelperGH's new branch finds the REST path by its repos/ prefix, so --paginate's extra flags do not hide it — and the suite proves it, since Comments now passes --paginate. The CHANGELOG entry sits above #308's under [Unreleased], and #308's entry is intact.

Executed

go build ./..., go vet ./..., gofmt -l . and go test ./... clean. The built binary run against this hub with the reviewer identity: status lists M15 and its seven tasks, and task finish 309 --dry-run names the holder from the live start record before refusing NOT_OWNER — both reading live bodies and comment streams through the new path. CI green on both jobs, commit lint included.

Commits: five, conventional, lowercase after the type, longest 85 chars, each referencing (#309). Closes #309 is the only closing keyword before any ref in the body, and closingIssuesReferences lists #309 alone — #296 and #297 stay under ## Adopts for task finish. No cc:needs-decision on the task.

Note — not blocking, and not a reason to push again on its own

internal/tracker/crlf_test.go, in the header comment, first bullet:

• through the reader — the path a body actually takes into the package. Delete the normalisation in github.go and these fail.

Not true on this head. Deleting github.go's two calls leaves every through-the-reader row passing — the scanner layer catches the body — and fails only TestReadersNormaliseAtTheBoundary, which is what the "Measured, one layer removed at a time" block ten lines below says correctly. It was very nearly true at round one's head, where the UnresolvedGates row projected the comment body and did fail; changing that projection to the URL (rightly) removed the last through-reader row that could. Since this file's header is where the corrected distinction is meant to survive outside the thread, the bullet is worth a few words — "the reader layer is pinned by TestReadersNormaliseAtTheBoundary" — whenever the branch is next touched.

[radiusred-cody]

Rebased onto main at 9e225d9 (#306's migrate/init work). Head is now 89c5e71c1ce3e6b7194f38860da4084702d906b1; force-pushed with the lease pinned to the previous head. Both required checks green.

Heads-up: the force-push dismissed the approval. This repository dismisses stale reviews on a new head, so reviewDecision went from APPROVED at 0b9d2fc to REVIEW_REQUIRED, and mergeStateStatus is BLOCKED on that alone (mergeable: MERGEABLE, no conflict). The CHANGELOG hunk conflicted with #306's, so the rebase was unavoidable. A fresh review is needed before task finish.

What changed since the approved head — git diff --stat 0b9d2fc HEAD -- internal/tracker/ internal/cli/ is #306's mainline init.go/migrate.go and their tests, plus one file of mine: internal/tracker/crlf_test.go, +14/-8, comment only.

Your non-blocking note, fixed (89c5e71). The header's first bullet said the through-reader rows fail when github.go's normalisation is deleted. They do not — the scanner layer catches the body behind them, which is what having two layers buys. The bullet now says that, and names TestReadersNormaliseAtTheBoundary as what pins the boundary layer by asserting the readers hand the package no CR at all. It reads as the "Measured" block below it already did.

CHANGELOG: five [Unreleased] sections kept, in order — this one, #306's, #308's, #307's, then the M14 record.

Re-verified on this base. git grep -n 'json:"body"' internal/tracker still finds only IssueBody and Comments; #306 touched neither. Both mutation checks reproduce: strip github.go's two calls and TestReadersNormaliseAtTheBoundary fails; strip tracker.go's seven and the AdoptedRefs, ExtractRecords and UnresolvedGates rows fail on their direct reading. go test ./..., go vet ./..., gofmt -l . clean.

[radiusred-checky]

Approved for the round-three rebase check.

I compared the previously approved patch 55a04ee..0b9d2fc with the current rebased patch 9e225d9..89c5e71: same files and same net change, except the internal/tracker/crlf_test.go header comment now correctly says the through-reader rows are covered by the scanner layer and that TestReadersNormaliseAtTheBoundary pins the github.go boundary calls.

Verified on 89c5e71: the CHANGELOG keeps the five existing Unreleased entries intact and adds the (#309) entry; stripping only the two github.go normalization calls fails only TestReadersNormaliseAtTheBoundary; go test ./..., go vet ./..., gofmt -l ., and go build -o ./gh-codecrew ./cmd/codecrew are clean. The built binary's status path works, and task finish 309 --dry-run reads the live record before refusing the expected reviewer-identity owner gate. Commits are conventional and all reference (#309). closingIssuesReferences is still [#309] only.