diff --git a/.claude/rules/delivery.md b/.claude/rules/delivery.md index c9b110b..20b3e88 100644 --- a/.claude/rules/delivery.md +++ b/.claude/rules/delivery.md @@ -37,10 +37,10 @@ Once the last task is done: 3. **Commit and push.** Conventional Commits, written from the diff and the spec. 4. **Open the PR.** Body: what changed, which spec or plan, how it was verified. -**The work is the user's, and the record says so.** No `Co-Authored-By` for an assistant, -no session link, no "generated with" footer or badge, no naming of a model, vendor or -harness — not in a commit message, not in a PR title or body. The `commit-msg` hook -rejects it as you write it, `SCC_SKIP_HOOKS=1` is not the fix, `--pr` reads the PR body. +**The work is the user's, and the record says so.** No `Co-Authored-By` for an assistant, no +vendor link, no "generated with" footer or badge, no crediting a tool ("thanks to", "with the +help of", "used X to"), no naming a model, vendor or harness — not in a commit, a PR title, +body or comment. `commit-msg` rejects a commit, `--pr` the forge; `SCC_SKIP_HOOKS=1` is no fix. **The gates are commands, not claims.** What this workspace recorded for build, format, lint and test; the last reads back `{"total": N, "coverage": P}` and is held to a floor. diff --git a/.claude/scc-manifest.json b/.claude/scc-manifest.json index a2ec126..75a5780 100644 --- a/.claude/scc-manifest.json +++ b/.claude/scc-manifest.json @@ -10,62 +10,62 @@ { "hash": "08141d3f57fbe10f46ea557de9969fe06f96e4aceca90cd14079aea0162e4a73", "path": ".claude/agents/scc-code-review.md", - "version": "32" + "version": "35" }, { "hash": "86ae07c5864d0c38932cf9781e5b0ac5d6f6e36925577efdef646e779abbe65b", "path": ".claude/agents/scc-security-review.md", - "version": "32" + "version": "35" }, { "hash": "f3132fd6be0b859b5ffb063e22ba3b840226c5bfdde2613cd21993ff12afa477", "path": ".claude/rules/artifacts.md", - "version": "32" + "version": "35" }, { "hash": "2eb97e13f6d088379c51870ff7338ddd36d3e377486408d5a2a2d236fe05a1e1", "path": ".claude/rules/autonomy.md", - "version": "32" + "version": "35" }, { "hash": "8eee2d450faada83295a4a04e5a1267bf5f717df9e7ac7da5fddd7649c666499", "path": ".claude/rules/caveman.md", - "version": "32" + "version": "35" }, { "hash": "cfe46c6623c20b6da46c9f18ef9b35e0f554c6c4568791ff190648dda3524706", "path": ".claude/rules/code-search.md", - "version": "32" + "version": "35" }, { - "hash": "3ad4d9513a212bcced1b99db171a9ed53ccb4be38063f345ae722e6c8f36fa20", + "hash": "3bb9d1336dd9478c76f1957167bbfddba5aa5adf97908b0791caaf75e96c446f", "path": ".claude/rules/delivery.md", - "version": "32" + "version": "35" }, { "hash": "ffca716ea25746d8e100ba3a7a7b884907736adebf3a7131b989fa4e9fcd0d80", "path": ".claude/rules/knowledge-base.md", - "version": "32" + "version": "35" }, { "hash": "59bb916a2002d4176353463b017c366868192cf95c80fd1dce74265188b738c3", "path": ".claude/rules/ladder.md", - "version": "32" + "version": "35" }, { "hash": "79d5cbf1ab7fd09d26e9ea9119f3f283615467dbc35d3a4b1d4a030044b3598f", "path": ".claude/rules/methodology.md", - "version": "32" + "version": "35" }, { "hash": "7dcbf5cd9d2e9e8d08aed4eeaa96ee28af85da1d6de51fc35870b8430e30f331", "path": ".claude/rules/notes.md", - "version": "32" + "version": "35" }, { "hash": "52c5033fc427445b4e669c7865dbd8d5c2fc27d57f7200aeeb2cc48bc8e050d3", "path": ".claude/rules/prior-art.md", - "version": "32" + "version": "35" }, { "hash": "ae0030df2e1f053642a84953648e1d6038b0f0fbae831edf1c349badfb00485c", @@ -75,62 +75,62 @@ { "hash": "c8502ace5978c7b9c0543c49ebb5c9550584b1a3405e12c0217b595647e27722", "path": ".claude/rules/routing.md", - "version": "32" + "version": "35" }, { "hash": "0b86b1a73769c6ebd37c338c5ec3cbee8dda70b21d43b27a1c95eebc5ee70a9f", "path": ".claude/rules/specs.md", - "version": "32" + "version": "35" }, { "hash": "d64a118d493424b10ad8af24861083c34541f7946a92dae92fb153b0ab739c26", "path": ".claude/rules/tasks.md", - "version": "32" + "version": "35" }, { "hash": "403174a8133018b54adc69c3047c46f8ff603b8a91759c62da1e7ca6b7f45463", "path": ".claude/rules/verification.md", - "version": "32" + "version": "35" }, { "hash": "cce99457f203766837ec2b8165a9370924178d9ee377c7848dba65880461cf3f", "path": ".claude/skills/scc-adr/SKILL.md", - "version": "32" + "version": "35" }, { "hash": "fc2ecb43e7e3c157082dc359d1d0fb20c99fdb6fb774cfcead3c8c0961fef922", "path": ".claude/skills/scc-codewiki/SKILL.md", - "version": "32" + "version": "35" }, { "hash": "60e80dcee7e68498679735ac40812ac90641340fef6bd75d3e7de64d209f8716", "path": ".claude/skills/scc-glossary/SKILL.md", - "version": "32" + "version": "35" }, { "hash": "9d21e953cce950ccf495810c94411a98706780b2b8d38817e3ce13bfc0c74625", "path": ".claude/skills/scc-init/SKILL.md", - "version": "32" + "version": "35" }, { "hash": "2385f307f96cc4261e44d1a0d49bb39d6a5842a3205cee317e3228ced0f6aeb6", "path": ".claude/skills/scc-plan-run/SKILL.md", - "version": "32" + "version": "35" }, { "hash": "3d3773f869d73beeab4c2061397c9ad81faf6d4c11d1ef423470aca432c5a9e1", "path": ".claude/skills/scc-prd/SKILL.md", - "version": "32" + "version": "35" }, { "hash": "42dff0d55f15c60da604f62fc1dfe8af5b73de2e2497553411447aae57698e1b", "path": ".claude/skills/scc-stack/SKILL.md", - "version": "32" + "version": "35" }, { "hash": "aa53b7ddf701ea495d68ea12aa23a6d352c202edc0e308b7218a1bf685b09f21", "path": ".claude/skills/scc-wiki/SKILL.md", - "version": "32" + "version": "35" }, { "hash": "9dc2f234756bfdc79aa5697e8470f80766e68a0e72b6cda98107e25ea8a9427a", diff --git a/docs/notes.md b/docs/notes.md index ce667af..0c910b9 100644 --- a/docs/notes.md +++ b/docs/notes.md @@ -49,3 +49,6 @@ over this file answers with the example above as well as with the notes. --> - n-0009 2026-09-15 #gotcha @internal/mdblock/mdblock.go @internal/rtk/rtk.go @internal/mdscan/mdscan.go — a marker written in prose or a code span is not a block — mdblock and rtk.ForeignBlock search mdscan.Mask, and the entry file that documents the markers is why - n-0010 2026-09-15 #ceiling @internal/mdscan/mdscan.go — mdscan.Mask hides fenced blocks and inline code spans only — a marker in a 4-space-indented block or a blockquote still reads as a live block; lift it by teaching Mask those two constructs - n-0011 2026-09-15 #gotcha @internal/artifact/artifact.go — artifact.Within must resolve both sides the same way — resolving only the root made a not-yet-created path inside the workspace read as an escape on macOS and on Windows 8.3 paths + +- n-0013 2026-09-17 #ceiling @internal/finding/finding.go @internal/render/render.go — finding.Set.Report prints every Message to stderr unescaped - only validate/attribution clip strips control runes, so any validator embedding untrusted text inherits terminal-escape injection; lift it by escaping in render +- n-0014 2026-09-17 #gotcha @internal/scaffold @internal/manifest — scc update writes the manifest only when it applies a file, so a managed file already byte-identical to the next template keeps its old hash - and at the version after that it compares as edited and is never updated again; deleting the file and re-running update is the way back diff --git a/internal/assets/assets.go b/internal/assets/assets.go index ecf149f..a7a2617 100644 --- a/internal/assets/assets.go +++ b/internal/assets/assets.go @@ -283,7 +283,7 @@ import ( // `/home/node/.claude` is created and chowned in the image: without it the first // write of every session was `EACCES ... mkdir '/home/node/.claude/session-env'` and // the login it could not store read back as "not logged in". -const Version = "33" +const Version = "35" // The embedded tree. "all:" so nothing is silently dropped for having a name the // default embed pattern skips. diff --git a/internal/assets/assets_test.go b/internal/assets/assets_test.go index f5a016d..c5f386b 100644 --- a/internal/assets/assets_test.go +++ b/internal/assets/assets_test.go @@ -1075,7 +1075,7 @@ func (e errNoSuchRule) Error() string { // uses wherever a change is cheap to make and expensive to make by accident. func TestTheTemplateVersionMovesWithTheTemplates(t *testing.T) { // Bump Version, then replace this with the digest the failure prints. - const fingerprint = "5f4a989994ef836fd039f1f188f1027ee4924164a0ae7a6ebd30ce345f6f6353" + const fingerprint = "ca004345d7bd327f81c7361b2ceca205d8e2e1609e4b527118b1e1a7e9c65a05" sum := sha256.New() // Version goes into the hash, and without it this test does not do the job its diff --git a/internal/assets/templates/rules/delivery.md b/internal/assets/templates/rules/delivery.md index c9b110b..20b3e88 100644 --- a/internal/assets/templates/rules/delivery.md +++ b/internal/assets/templates/rules/delivery.md @@ -37,10 +37,10 @@ Once the last task is done: 3. **Commit and push.** Conventional Commits, written from the diff and the spec. 4. **Open the PR.** Body: what changed, which spec or plan, how it was verified. -**The work is the user's, and the record says so.** No `Co-Authored-By` for an assistant, -no session link, no "generated with" footer or badge, no naming of a model, vendor or -harness — not in a commit message, not in a PR title or body. The `commit-msg` hook -rejects it as you write it, `SCC_SKIP_HOOKS=1` is not the fix, `--pr` reads the PR body. +**The work is the user's, and the record says so.** No `Co-Authored-By` for an assistant, no +vendor link, no "generated with" footer or badge, no crediting a tool ("thanks to", "with the +help of", "used X to"), no naming a model, vendor or harness — not in a commit, a PR title, +body or comment. `commit-msg` rejects a commit, `--pr` the forge; `SCC_SKIP_HOOKS=1` is no fix. **The gates are commands, not claims.** What this workspace recorded for build, format, lint and test; the last reads back `{"total": N, "coverage": P}` and is held to a floor. diff --git a/internal/attribution/attribution.go b/internal/attribution/attribution.go index bc73813..182951a 100644 --- a/internal/attribution/attribution.go +++ b/internal/attribution/attribution.go @@ -51,12 +51,18 @@ const ( // address. It is separate from the other two because it survives them: a // footer rewritten by hand usually keeps the URL. // - // A session link and a no-reply address fire wherever they sit — nobody cites - // either to explain a change. Any other vendor URL has to be alone on its line, - // because documentation lives on those hosts and an argument that cannot cite - // the vendor's own docs is one nobody can check. That gate costs a real miss: - // a bare host inside a sentence is no longer reported, which is this package's - // standing trade. + // It fires wherever the URL sits — inside prose, inside a parenthesis, inside + // an HTML comment — and the one thing that buys an exemption is being + // *documentation*. That split replaces an earlier one that asked whether the + // URL was alone on its line, and it was measured rather than argued: a harness + // told not to sign its work wrote the product link into a sentence, and a rule + // that only looked at lines standing by themselves read it as prose. A product + // page is nobody's citation, so the position it sits in says nothing. + // + // Documentation keeps the old gate. `code.claude.com/docs/…` is a source, and + // a technical argument that cannot cite the vendor's own documentation is one + // nobody can check — so a docs URL is reported only when it stands alone, + // where it is the footer's surviving half rather than a reference. RuleLink = "assistant-link" // RuleBadge is a Markdown link or image whose label or target names an // assistant — the `[Claude Code](…)` half of the footer the harnesses append. @@ -84,17 +90,40 @@ const ( var ( // A git trailer: a known key at the start of a line, with a value. trailer = regexp.MustCompile(`(?i)^[ \t]*(co-?authored-by|assisted-by|generated-by|signed-off-by)[ \t]*:[ \t]*(\S.*?)[ \t]*$`) - // The footer phrase. It is only half a shape: the caller requires the name to + // The footer phrase, and every other way a line hands this change to whoever + // it names next. It is only half a shape: the caller requires the name to // follow it on the same line, so a sentence that happens to say "written by" // about something else stays prose. - footer = regexp.MustCompile(`(?i)\b(?:generated|created|authored|written|made|built|produced|drafted)\s+(?:with|by|using|via)\b`) - // A session link, a vendor's own page, or a vendor no-reply address, which is - // a signature on its own: nobody cites any of these to explain a change. + // + // Three families, and the second and third are what a session writes once it + // has been told not to write the first. "Generated with" is the harness + // default; "with the help of" and "thanks to" are what it collapses to when + // somebody rephrases it in their own words; and the verb list is wider than + // the footer's because the sentence being rewritten is about this work, so any + // verb for making it will do. + footer = regexp.MustCompile(`(?i)\b(?:` + + `(?:generated|created|authored|written|made|built|produced|drafted|implemented|developed|refactored|scaffolded|assembled|composed|prepared|completed|reviewed)\s+(?:with|by|using|via)` + + `|(?:with|using)\s+(?:the\s+)?(?:help|assistance|support|aid)\s+(?:of|from)` + + `|thanks\s+to|powered\s+by|driven\s+by|came\s+from|courtesy\s+of` + + `|pair(?:ed|[- ]programmed)?\s+with|in\s+(?:collaboration|partnership)\s+with` + + `)\b`) + // The other half of crediting: the assistant as the instrument rather than the + // agent — "used Claude Code to draft the tests". The name has to be what + // follows the verb, not merely somewhere after it, because the passive reading + // of the same words is ordinary technical prose: "the settings format used by + // Claude Code" describes a product, it does not hand it the work. + useVerb = regexp.MustCompile(`(?i)\b(?:used|using|ran)\s+(?:the\s+|a\s+|an\s+|my\s+|our\s+)?`) + // A session link, a vendor's own page, or a vendor no-reply address. // // Whole hosts, rather than the one path today's footer happens to use. // claude.com/claude-code is the spelling that walked past a rule written // around claude.ai, and the next rewording moves the path again. link = regexp.MustCompile(`(?i)\b(?:https?://)?(?:[\w-]+\.)*(?:claude\.(?:ai|com)|anthropic\.com|chatgpt\.com|openai\.com|cursor\.(?:com|sh)|copilot\.microsoft\.com|codeium\.com|windsurf\.com|sourcegraph\.com/cody)(?:/\S*)?|\b[\w.+-]+@(?:anthropic|openai)\.com\b`) + // What makes a vendor URL citable: a documentation host, or a path whose first + // meaningful segment is documentation. Everything else on a vendor host is a + // product page, and nobody cites a product page to explain a change. + docHost = regexp.MustCompile(`(?i)^(?:https?://)?(?:docs?|developers?|platform|api|help|support|code)\.`) + docPath = regexp.MustCompile(`(?i)^(?:https?://)?[^/\s]+/(?:[\w-]+/){0,3}?(?:docs?|reference|api|guides?|blog|news|engineering|research|help|support|changelog|papers?)(?:[/#?]|$)`) // A Markdown link or image: a label in brackets and a target in parentheses, // with an optional leading bang. Both halves are measured, so a badge survives // having either one of them rewritten. @@ -201,17 +230,13 @@ func scanLine(line string, last bool) (Hit, bool) { if m := trailer.FindStringSubmatch(line); m != nil && Names(m[2]) { return Hit{Rule: RuleTrailer, Match: strings.TrimSpace(line)}, true } - // The name has to come *after* the phrase, not merely somewhere on the line. - // Measured on this project's own history: "opencode has one AGENTS.md written - // by whichever ran first" carries the phrase and the vocabulary and is a - // sentence about opencode, not a line signed by it. - if at := footer.FindStringIndex(line); at != nil && Names(line[at[1]:]) { + if credited(line) || instrumental(line) { return Hit{Rule: RuleFooter, Match: strings.TrimSpace(line)}, true } if m := badge.FindStringSubmatch(line); m != nil && (Names(m[1]) || Names(m[2])) && signsAlone(line, m[0]) { return Hit{Rule: RuleBadge, Match: strings.TrimSpace(m[0])}, true } - if m := link.FindString(line); m != "" && (session.MatchString(m) || strings.Contains(m, "@") || signsAlone(line, m)) { + if m := link.FindString(line); m != "" && linkSigns(line, m) { return Hit{Rule: RuleLink, Match: m}, true } if m, ok := mention(line, last); ok { @@ -220,6 +245,104 @@ func scanLine(line string, last bool) (Hit, bool) { return Hit{}, false } +// credited reports a line that hands this change to whoever it names: the footer +// phrase, or one of the ways somebody rephrases it. +// +// The name has to come *after* the phrase, not merely somewhere on the line. +// Measured on this project's own history: "opencode has one AGENTS.md written by +// whichever ran first" carries the phrase and the vocabulary and is a sentence +// about opencode, not a line signed by it. +// +// And the occurrence must not be quoted, for the reason instrumental gives below: +// a commit explaining this rule quotes the phrases it catches, and the rule that +// reported that would fire on the fix for its own finding. +func credited(line string) bool { + for _, at := range footer.FindAllStringIndex(line, -1) { + if Names(line[at[1]:]) && !quoted(line[:at[0]]) { + return true + } + } + return false +} + +// instrumental reports a line that uses an assistant to do this work — "used +// Claude Code to draft the tests", "ran Codex over the diff". +// +// It is the footer rule's other voice, and it is separated from it because it +// needs a stricter test. The footer phrase can look anywhere after itself for the +// name, since "generated with" is already about this change. "Used" is not: the +// same verb in the passive describes a product rather than crediting one, and +// this repository's own history writes that sentence — "the settings format used +// by Claude Code" is prose about a harness scc supports. +// +// So the name has to be the verb's own object — the very next word, article +// aside — and it must not be possessive. "Used by Claude Code" puts a preposition +// there and is the passive; "using Claude Code's hook surface" is a sentence +// about a hook surface; "using Claude Code to write this" hands over the work. +// Those two tests are cheap, and they stand in for a distinction that would +// otherwise need a parser. +// A quoted occurrence is skipped, which is the anchoring the other rules get from +// their shape and this one has nowhere else to get. Measured on this package's own +// history: a commit explaining this very rule quotes the line it now catches, and +// a check that reported that would fire on the fix for its own finding — the +// failure the package comment names at the top of this file. +func instrumental(line string) bool { + for _, at := range useVerb.FindAllStringIndex(line, -1) { + rest := strings.Fields(line[at[1]:]) + switch { + case len(rest) == 0, !Names(rest[0]): + case strings.ContainsAny(firstWords(rest, 3), "'’"): + case quoted(line[:at[0]]): + default: + return true + } + } + return false +} + +// quoted reports whether what follows this much of a line sits inside a +// quotation: an odd number of straight quotes or backticks behind it, or a +// typographic quotation still open. +// +// The apostrophe is deliberately not counted. It is a possessive in ordinary +// English far more often than it opens anything, and instrumental already reads +// one three words along to tell a credit from a description. +func quoted(before string) bool { + return strings.Count(before, `"`)%2 == 1 || + strings.Count(before, "`")%2 == 1 || + strings.Count(before, "“") > strings.Count(before, "”") +} + +// firstWords is the opening of a fragment, punctuation kept. Kept rather than +// stripped because the punctuation is what instrumental reads: an apostrophe +// three words in is the difference between a credit and a description. +func firstWords(f []string, n int) string { + if len(f) > n { + f = f[:n] + } + return strings.Join(f, " ") +} + +// linkSigns decides whether a vendor URL on this line is a signature. +// +// A session link and a no-reply address always are: nobody cites either to +// explain a change. Documentation is the one exemption, and it keeps the older +// "alone on its line" gate, because a docs URL in a sentence is a source and the +// same URL by itself at the end of a message is the footer with the words taken +// off. Everything else on a vendor host is a product page, and a product page is +// a signature wherever it sits — in prose, in a parenthesis, or inside an HTML +// comment, which is where one was measured getting through. +func linkSigns(line, m string) bool { + switch { + case session.MatchString(m), strings.Contains(m, "@"): + return true + case docHost.MatchString(m), docPath.MatchString(m): + return signsAlone(line, m) + default: + return true + } +} + // mention reports a line that is nothing but an assistant's name, in the place a // signature goes. // @@ -247,6 +370,13 @@ func mention(line string, last bool) (string, bool) { for _, w := range words { low := strings.ToLower(w) switch { + // A file is not a name, and this repository is the reason the distinction + // had to be drawn: `CLAUDE.md` is a path the vocabulary matches on, so a + // last line naming the file — which a review bot writes and a commit body + // listing what it touched writes too — was read as somebody signing it. + // The line says something else, so it is not a bare mention. + case isFilename(low): + return "", false case Names(low): named = true case filler[low], isVersion(low): @@ -268,13 +398,46 @@ func mention(line string, last bool) (string, bool) { // of harnesses starts. A signature's decoration and a list's bullet look alike // enough that guessing between them is how this rule would earn its first false // positive. +// +// Saying that was not enough to make it true, and the gap was measured rather +// than argued. `unicode.IsSymbol` returns true for `>`, which is Sm — so every +// blockquoted line opened a signature, and `> Claude Code, Codex, and opencode` +// was reported: the very sentence the rule above promises never to report, quoted +// back. A review bot's comment is written entirely in blockquotes, and `--pr` now +// reads comments, so the paragraph and the code had to be made to agree. +// +// The symbol has to be non-ASCII. That is what a signature's decoration actually +// is — the robot, a sparkle, a trademark — while ASCII's symbol block is Markdown +// punctuation: `>`, `|`, `=`, `+`, `^`, the backtick, `~`. The one ASCII +// character kept is the tilde, named above and never a Markdown opener at the +// start of a line. func signed(line string) bool { for _, r := range strings.TrimLeft(line, " \t") { - return unicode.IsSymbol(r) || r == '—' || r == '–' || r == '~' + return (unicode.IsSymbol(r) && r > unicode.MaxASCII) || r == '—' || r == '–' || r == '~' } return false } +// isFilename reports a token that is a file rather than a name: a dot with a +// short alphabetic tail, which is an extension. +// +// It sits beside isVersion because it is the same kind of test — a shape rather +// than a word — and it is deliberately narrow. A model name keeps its dots +// ("claude-3.5-sonnet" ends in "5-sonnet", which is not an extension), and a +// vendor host ending in one ("claude.ai") is the link rule's to report and is +// reached before this. +func isFilename(w string) bool { + i := strings.LastIndexByte(w, '.') + if i <= 0 || i == len(w)-1 { + return false + } + ext := w[i+1:] + if len(ext) > 4 { + return false + } + return strings.IndexFunc(ext, func(r rune) bool { return !unicode.IsLetter(r) }) < 0 +} + // isVersion accepts the scraps of a model name that are not words: "5", "4.5", // "v2", "1m". It sits beside filler rather than in it because it is a shape too. func isVersion(w string) bool { diff --git a/internal/attribution/attribution_test.go b/internal/attribution/attribution_test.go index 06b3f4d..e8a0328 100644 --- a/internal/attribution/attribution_test.go +++ b/internal/attribution/attribution_test.go @@ -115,10 +115,6 @@ func TestTheFooterRewrittenIsStillCaught(t *testing.T) { {"badge named by target", "[the tool](https://claude.ai/code)", RuleBadge}, {"image badge", "![Claude](https://img.example/badge.svg)", RuleBadge}, {"vendor page", "https://www.anthropic.com/claude-code", RuleLink}, - // Alone on the line, which is where a footer's surviving URL sits. A vendor - // host *inside* a sentence is now a deliberate miss: documentation lives on - // these domains, and this package trades a miss for a false positive every - // time — one wrong finding teaches the reader to disbelieve the other ten. {"vendor host alone", "claude.com/claude-code", RuleLink}, {"session link mid-sentence", "picked up from https://claude.ai/code/session_01abc in passing", RuleLink}, {"bare name under a symbol", "\U0001F916 Claude Code", RuleMention}, @@ -196,3 +192,193 @@ func TestBadgeTellsACitationFromASignature(t *testing.T) { } } } + +// TestAProductPageIsASignatureWhereverItSits is the measured escape this rule was +// widened for. A harness told not to sign its work kept the link and moved it into +// a sentence, and the gate that asked whether the line stood alone read that as +// prose. A product page is nobody's citation, so where it sits says nothing about +// what it is. +func TestAProductPageIsASignatureWhereverItSits(t *testing.T) { + for _, tc := range []struct { + name string + line string + }{ + {"in prose", "See https://claude.com/claude-code for the tool that helped here."}, + {"in an HTML comment", ""}, + {"in a parenthesis", "Part of the work happened elsewhere (claude.com/claude-code)."}, + {"bare host mid-sentence", "Some of the tests came out of claude.com and were tidied by hand."}, + {"trailing punctuation", "Tooling: https://claude.com/claude-code."}, + {"a vendor page that is not the product", "written up at https://www.anthropic.com/claude in more detail"}, + } { + t.Run(tc.name, func(t *testing.T) { + hits := Scan("fix: thing\n\n" + tc.line) + if len(hits) != 1 || hits[0].Rule != RuleLink { + t.Fatalf("got %+v, want one %s hit", hits, RuleLink) + } + }) + } +} + +// TestDocumentationStaysCitable is the other side of that trade, and the more +// important one. An argument that cannot cite the vendor's own documentation is +// one nobody can check, and a validator that fires on scc's own pull requests is +// the worst bug in this product. +func TestDocumentationStaysCitable(t *testing.T) { + for _, line := range []string{ + "Anthropic's own guidance at https://code.claude.com/docs/en/devcontainer warns about exfiltration.", + "The event list is at code.claude.com/docs/en/hooks, which is where the stage names come from.", + "Measured against https://docs.anthropic.com/en/api/messages and the SDK's own defaults.", + "The post at https://www.anthropic.com/news/claude-opus-5 is where the figure comes from.", + "Rate limits: platform.openai.com/docs/guides/rate-limits, for the comparison.", + } { + if hits := Scan("fix: thing\n\n" + line); len(hits) != 0 { + t.Errorf("citation reported as a signature: %q → %+v", line, hits) + } + } + + // It is still reported standing by itself, which is where a footer's surviving + // URL lands once the words in front of it are gone. + if hits := Scan("fix: thing\n\nhttps://code.claude.com/docs/en/overview"); len(hits) != 1 { + t.Errorf("a docs URL alone on its line is the footer with the words off: got %+v", hits) + } +} + +// TestCreditingAToolIsAttribution covers the phrasings a session reaches for once +// it has been told not to write "Generated with" — the same claim in somebody's +// own words. +func TestCreditingAToolIsAttribution(t *testing.T) { + for _, line := range []string{ + "Part of the work used Claude Code to draft the tests.", + "Some of this came from Claude Code.", + "Written with the help of Claude Code.", + "Thanks to Claude for the refactor.", + "Powered by Codex.", + "Pair-programmed with Claude Opus 5.", + "Scaffolded with opencode, then finished by hand.", + "Reviewed by Claude before it was pushed.", + "Using Claude Code, the tests were rewritten from the requirement.", + } { + hits := Scan("fix: thing\n\n" + line) + if len(hits) != 1 || hits[0].Rule != RuleFooter { + t.Errorf("credit not reported: %q → %+v", line, hits) + } + } +} + +// TestTalkingAboutAHarnessIsNotCreditingIt is where the widened rule is weakest, +// and the bar is this repository's own history: fifteen of its commits discuss +// Claude Code as the harness scc scaffolds for, and not one of them credits it +// with the work. Every line here is taken from that history or written in its +// register. +func TestTalkingAboutAHarnessIsNotCreditingIt(t *testing.T) { + for _, text := range []string{ + "refactor(assets): drop the command files on Claude Code, move the prefix onto the skill", + "feat(assets): scaffold the methodology into Codex and opencode, not only Claude Code", + "fix: x\n\nClaude Code registers a skill at skills//SKILL.md as / by itself.", + "fix: x\n\nThe settings format used by Claude Code is JSON, and scc splices into it.", + "fix: x\n\nUsing Claude Code's hook surface, the Stop stage can report findings.", + "fix: x\n\nThe rules are loaded by Claude Code at launch with the same priority as CLAUDE.md.", + "feat(launch): sandbox by default, with a dev container for Windows", + "fix: x\n\nThe image carries Claude Code, scc, codegraph and rtk, pinned to build ARGs.", + "fix: x\n\nMeasured on a freshly scaffolded Claude Code workspace: 47KB in every request.", + } { + if hits := Scan(text); len(hits) != 0 { + t.Errorf("Scan(%q) reported %+v, want nothing", text, hits) + } + } +} + +// TestMarkdownStructureIsNotASignaturePosition is the measured regression, and +// the corpus is this repository's own pull requests: every body, every issue +// comment and every review comment. Two lines were reported before the position +// gate was made to mean what its own paragraph claimed — a quoted bullet in a +// review bot's file list, and a table row from PR #6 comparing the three +// harnesses. `unicode.IsSymbol` is true for `>` and `|`, so Markdown's own +// structure opened a signature. +// +// Reading comments is what made this live traffic: a review bot writes entirely +// in blockquotes. +func TestMarkdownStructureIsNotASignaturePosition(t *testing.T) { + for _, line := range []string{ + "> * `CLAUDE.md`", + "> Claude Code registers a skill as a command by itself.", + "| | Claude Code | Codex | opencode |", + "| Claude Code | yes |", + "= Claude Code", + "+ Claude Code", + "^ Claude Code", + } { + // Not the last line, which is a signature's other position and tested + // below: this is about the decoration in front of it. + if hits := Scan("fix: thing\n\n" + line + "\n\nand then some prose."); len(hits) != 0 { + t.Errorf("Markdown structure read as a signature: %q → %+v", line, hits) + } + } +} + +// And the decoration that really does open a signature still does. Every one of +// these is non-ASCII or the typographic dash the rule always named. +func TestRealDecorationStillOpensASignature(t *testing.T) { + for _, line := range []string{ + "🤖 Claude Code", + "✨ Claude Opus 5", + "— Claude Code", + "– via Codex", + "~ Claude", + } { + hits := Scan("fix: thing\n\n" + line + "\n\nand then some prose.") + if len(hits) != 1 || hits[0].Rule != RuleMention { + t.Errorf("signature not reported: %q → %+v", line, hits) + } + } +} + +// TestAFileIsNotAName is the other half of the same corpus result. `CLAUDE.md` is +// a path the vocabulary matches on, so a last line naming the file — which a +// review bot writes, and so does a commit body listing what it touched — read as +// somebody signing the work. +func TestAFileIsNotAName(t *testing.T) { + for _, line := range []string{ + "CLAUDE.md", + "🤖 CLAUDE.md", + "claude.md", + "— AGENTS.md and CLAUDE.md", + } { + if hits := Scan("fix: thing\n\n" + line); len(hits) != 0 { + t.Errorf("a filename read as a signature: %q → %+v", line, hits) + } + } + // A model name keeps its dots and is still a name: the gate is an extension, + // not a full stop. + for _, line := range []string{ + "🤖 claude-3.5-sonnet", + "— Claude Opus 5", + } { + if hits := Scan("fix: thing\n\n" + line); len(hits) != 1 { + t.Errorf("model name not reported: %q → %+v", line, hits) + } + } +} + +// TestQuotingTheShapeIsNotWearingIt is the failure the package comment names at +// the top of the file: a commit message that merely quotes one of these lines, +// which is exactly what a fix for this finding looks like. Measured on this +// change's own commit and pull request, both of which were reported by the rule +// they were explaining. +func TestQuotingTheShapeIsNotWearingIt(t *testing.T) { + for _, line := range []string{ + `voice, "used Claude Code to draft the tests". The last needs the stricter test`, + "the instrumental voice, `used Claude Code to draft the tests`, needs a stricter test", + `"thanks to", "with the help of", and "used Claude Code to" are the phrasings it catches`, + } { + if hits := Scan("fix: thing\n\n" + line); len(hits) != 0 { + t.Errorf("a quoted shape reported as a signature: %q → %+v", line, hits) + } + } + // And an unquoted credit on the same line as a quotation is still caught, so + // the gate is per occurrence rather than per line. + line := `The footer said "Generated with X", and I used Claude Code to rewrite it.` + if hits := Scan("fix: thing\n\n" + line); len(hits) != 1 { + t.Errorf("the unquoted credit went unreported: %q → %+v", line, hits) + } +} diff --git a/internal/git/git.go b/internal/git/git.go index 914e214..2413e1d 100644 --- a/internal/git/git.go +++ b/internal/git/git.go @@ -278,6 +278,71 @@ func viewPR(dir, arg string) (PR, error) { }, nil } +// PRComment is one thing somebody wrote on a pull request after it was opened — +// an issue comment or the body of a review. +// +// It is separate from PR.Body because it is the one part of the record nothing +// else can reach. A commit message passes through commit-msg on the machine that +// wrote it; a pull request body is at least typed once by whoever opened it; a +// comment is posted straight into the forge at any point afterwards, by a tool as +// readily as by a person. Whatever rule binds the first two binds this. +// +// Author is the login, which is how a reader finds the comment again — the URL is +// exact but unreadable in a terminal, so both are kept. +type PRComment struct { + Author string `json:"author"` + URL string `json:"url"` + Body string `json:"body"` +} + +// PRComments is everything written on a pull request after it was opened: the +// issue comments and the bodies of the reviews, in that order. +// +// A number of 0 means the pull request for the checked-out branch, which is gh's +// own default and the same convention CurrentPR follows. +// +// It is a second call rather than another field on PR, and that is deliberate: +// spec sync asks LookPR about every spec in the workspace, and it has no use for +// comment bodies. Only the attribution check pays for these, and only under +// `scc validate --pr`. +func PRComments(dir string, number int) ([]PRComment, error) { + args := []string{"pr", "view"} + if number > 0 { + args = append(args, strconv.Itoa(number)) + } + out, err := run(GHBin, dir, append(args, "--json", "comments,reviews")...) + if err != nil { + return nil, err + } + var raw struct { + Comments []rawComment `json:"comments"` + Reviews []rawComment `json:"reviews"` + } + if err := json.Unmarshal([]byte(out), &raw); err != nil { + return nil, err + } + var all []PRComment + for _, c := range append(append([]rawComment{}, raw.Comments...), raw.Reviews...) { + // A review with no body is an approval click, and there is nothing in it to + // read. Dropping it here keeps the caller from reporting an empty record. + if strings.TrimSpace(c.Body) == "" { + continue + } + all = append(all, PRComment{Author: c.Author.Login, URL: c.URL, Body: c.Body}) + } + return all, nil +} + +// rawComment is gh's shape for both lists, which differ only in fields neither +// caller reads. +type rawComment struct { + Author struct { + Login string `json:"login"` + } `json:"author"` + URL string `json:"url"` + Body string `json:"body"` +} + // Commit is one commit's identity and its message in full. // // Message is the whole thing — subject, body and trailers — because the trailers diff --git a/internal/git/repo_test.go b/internal/git/repo_test.go index 689d51e..a3be3e6 100644 --- a/internal/git/repo_test.go +++ b/internal/git/repo_test.go @@ -503,3 +503,47 @@ func TestViewPRReadsWhatTheForgeSays(t *testing.T) { t.Error("LookPR accepted output that is not JSON") } } + +// A comment is the half of the pull request that nothing else can reach: no hook +// runs when a tool posts one. The query reads the issue comments and the review +// bodies as one list, and drops a review that is an approval click with nothing +// written in it. +func TestPRCommentsReadsWhatWasWrittenAfterwards(t *testing.T) { + const body = `{"comments":[{"author":{"login":"someone"},"url":"https://example.invalid/pr/7#c1","body":"looks good"}],` + + `"reviews":[{"author":{"login":"other"},"url":"https://example.invalid/pr/7#r1","body":"one nit"},` + + `{"author":{"login":"third"},"url":"https://example.invalid/pr/7#r2","body":" "}]}` + t.Setenv("PATH", fakeBin(t, GHBin, body, 0)) + + got, err := PRComments(t.TempDir(), 7) + if err != nil { + t.Fatalf("PRComments: %v", err) + } + if len(got) != 2 { + t.Fatalf("got %d comments, want 2 (the empty review is an approval click): %+v", len(got), got) + } + // Issue comments first, then reviews, so a reader meets them in the order the + // pull request page shows them. + if got[0].Author != "someone" || got[0].Body != "looks good" { + t.Errorf("first = %+v", got[0]) + } + if got[1].Author != "other" || got[1].URL != "https://example.invalid/pr/7#r1" { + t.Errorf("second = %+v", got[1]) + } + + // The current branch's pull request is the same query with no number, which is + // gh's own default. + if _, err := PRComments(t.TempDir(), 0); err != nil { + t.Errorf("PRComments(0): %v", err) + } + + // And every way of having no answer is an error the caller turns into silence, + // never a half-read list. + t.Setenv("PATH", fakeBin(t, GHBin, "not json", 0)) + if _, err := PRComments(t.TempDir(), 7); err == nil { + t.Error("PRComments accepted output that is not JSON") + } + t.Setenv("PATH", t.TempDir()) + if _, err := PRComments(t.TempDir(), 7); err != ErrUnavailable { + t.Errorf("err = %v, want ErrUnavailable when gh is not on PATH", err) + } +} diff --git a/internal/validate/attribution.go b/internal/validate/attribution.go index dc680f1..3c8b3c7 100644 --- a/internal/validate/attribution.go +++ b/internal/validate/attribution.go @@ -3,6 +3,7 @@ package validate import ( "strconv" "strings" + "unicode" "github.com/protonspy/spec-claude-code/internal/attribution" "github.com/protonspy/spec-claude-code/internal/finding" @@ -51,7 +52,8 @@ func Attribution(root string) (*finding.Set, error) { } // AttributionPR reports the same signatures in the pull request this branch has -// open — its title and its body. +// open — its title, its body, and everything written on it since: the issue +// comments and the bodies of the reviews. // // It is the other half of the record and the half no hook can reach. A commit // message passes through `commit-msg` on the machine that wrote it; a PR body is @@ -66,7 +68,9 @@ func Attribution(root string) (*finding.Set, error) { // // Like its sibling it never returns an error. No gh, no authentication, no // network, no pull request for this branch: all of them mean there is nothing to -// check, and none of them is this validator failing. +// check, and none of them is this validator failing. The comment query degrades +// the same way and on its own — a forge that answers about the pull request and +// not about its comments still gets the title and body checked. func AttributionPR(root string) (*finding.Set, error) { set := &finding.Set{} if !git.IsRepo(root) || !git.Found(git.GHBin) { @@ -79,9 +83,29 @@ func AttributionPR(root string) (*finding.Set, error) { // Title first and body after, as one text, so a finding's line number reads // against what a person sees on the pull request page: line 1 is the title. report(set, prName(pr), clipSubject(pr.Title), pr.Title+"\n"+pr.Body) + // And then what was written on it afterwards. A comment is the one part of + // the record nothing else can reach — no hook runs when a tool posts one — and + // it is the same text under the same rule, so it is checked the same way. + // Each comment is its own text, because its line numbers are its own. + comments, err := git.PRComments(root, pr.Number) + if err != nil { + return set, nil + } + for _, c := range comments { + report(set, prName(pr)+" comment", commentBy(c), c.Body) + } return set, nil } +// commentBy names a comment the way its finding is read: by who wrote it, since a +// comment has no title and the number is already on the left of the line. +func commentBy(c git.PRComment) string { + if c.Author == "" { + return "a comment" + } + return "a comment by " + clipSubject(c.Author) +} + // prName is what a pull-request finding is filed under: the number, because it is // what `gh pr edit ` takes and the report is read in a terminal. func prName(pr git.PR) string { return "PR #" + strconv.Itoa(pr.Number) } @@ -107,16 +131,41 @@ var advice = map[string]string{ attribution.RuleMention: "a line that says nothing but the name is a signature", } -// clip keeps a finding to one readable line. A trailer is short, but a footer can -// carry a full URL and a message that wrapped the terminal would defeat the -// grouping the report is built around. +// clip keeps a finding to one readable line, and makes that line safe to print. +// +// Two jobs, and the second is not cosmetic. What is being clipped is the +// offending line itself, and under `--pr` that line arrives from a pull request +// comment — written by anybody with an account who can comment, rather than by +// whoever opened the pull request or by anybody with push access. It reaches a +// real terminal through finding.Set.Report on every `git push`, because that is +// where pre-push runs this check. A raw ESC surviving the trip is a control +// sequence the reader's emulator obeys: enough to scroll a finding off the screen +// and paint a clean report over the top of it, which is exactly the lie this +// validator exists to prevent. The subject printed beside the match goes out +// through %q and is escaped by the verb; the match goes out through %s, so it is +// escaped here. +// +// Every non-graphic rune becomes a space before the collapse — the C0 and C1 +// controls, and the format category, which is where the bidirectional overrides +// live. strings.Fields then folds the result back into the single line this +// function was always for. +// +// And the cut is by rune rather than by byte, for the same reason the escaping is +// here at all: halving a multi-byte rune puts invalid UTF-8 in the report, and +// the text is now chosen by somebody who would like it to. func clip(s string) string { const max = 64 + s = strings.Map(func(r rune) rune { + if unicode.IsGraphic(r) { + return r + } + return ' ' + }, s) s = strings.Join(strings.Fields(s), " ") - if len(s) <= max { - return s + if r := []rune(s); len(r) > max { + return string(r[:max-1]) + "…" } - return s[:max-1] + "…" + return s } // clipSubject names the commit in words, next to the short sha the finding is diff --git a/internal/validate/attribution_pr_test.go b/internal/validate/attribution_pr_test.go new file mode 100644 index 0000000..8c05a7c --- /dev/null +++ b/internal/validate/attribution_pr_test.go @@ -0,0 +1,171 @@ +package validate + +import ( + "encoding/json" + "os" + "os/exec" + "path/filepath" + "runtime" + "strings" + "testing" + "unicode" + "unicode/utf8" +) + +// A pull request comment is the one part of the record nothing else reaches: no +// hook runs when a tool posts one, and the body was written long after the branch +// was pushed. `scc validate --pr` reads them for the same signatures it reads in +// the title and the body. +func TestAttributionPRReadsTheCommentsToo(t *testing.T) { + root := gitRepo(t) + const pr = `{"number":7,"state":"OPEN","headRefName":"feat/x","url":"https://example.invalid/pr/7","title":"feat: x","body":"why"}` + const comments = `{"comments":[{"author":{"login":"someone"},"url":"https://example.invalid/pr/7#c1",` + + `"body":"Part of this came from Claude Code."}],"reviews":[]}` + stubGH(t, pr, comments) + + set, err := AttributionPR(root) + if err != nil { + t.Fatalf("AttributionPR: %v", err) + } + found := set.Sorted() + if len(found) != 1 { + t.Fatalf("got %d findings, want 1: %+v", len(found), found) + } + // Filed under the pull request it is on, and named by who wrote it: a comment + // has no title, and "PR #7" alone does not say which of ten comments to edit. + if !strings.Contains(found[0].File, "#7") || !strings.Contains(found[0].Message, "someone") { + t.Errorf("finding does not say where to look: %+v", found[0]) + } +} + +// And a forge that answers about the pull request but not about its comments +// still gets the title and the body checked. The comment query degrades on its +// own, because half the record is better than none. +func TestAttributionPRSurvivesAForgeWithNoComments(t *testing.T) { + root := gitRepo(t) + const pr = `{"number":7,"state":"OPEN","headRefName":"feat/x","url":"https://example.invalid/pr/7",` + + `"title":"feat: x","body":"Generated with Claude Code"}` + stubGH(t, pr, "") + + set, err := AttributionPR(root) + if err != nil { + t.Fatalf("AttributionPR: %v", err) + } + if len(set.Sorted()) != 1 { + t.Fatalf("the body's own footer went unreported: %+v", set.Sorted()) + } +} + +// gitRepo is an empty repository, which is all IsRepo asks for. +func gitRepo(t *testing.T) string { + t.Helper() + root := t.TempDir() + cmd := exec.Command("git", "init") + cmd.Dir = root + if err := cmd.Run(); err != nil { + t.Skip("git is not on PATH") + } + return root +} + +// stubGH puts a gh on the front of PATH that answers the two queries this +// validator makes: one about the pull request, one about its comments. It tells +// them apart by the field list, which is the only thing that differs. An empty +// comments answer is a gh that exits non-zero, which is how a forge that cannot +// answer that question reports it. +func stubGH(t *testing.T, pr, comments string) { + t.Helper() + dir := t.TempDir() + if runtime.GOOS == "windows" { + fail := "exit /b 1" + if comments != "" { + fail = "echo " + comments + } + body := "@echo off\r\n" + + "echo %*| findstr /C:\"comments\" >nul\r\n" + + "if errorlevel 1 (echo " + pr + ") else (" + fail + ")\r\n" + writeStub(t, filepath.Join(dir, "gh.cmd"), body, 0o644) + } else { + fail := "exit 1" + if comments != "" { + fail = "printf '%s\n' " + quote(comments) + } + body := "#!/bin/sh\ncase \"$*\" in\n *comments*) " + fail + " ;;\n" + + " *) printf '%s\n' " + quote(pr) + " ;;\nesac\n" + writeStub(t, filepath.Join(dir, "gh"), body, 0o755) + } + t.Setenv("PATH", dir+string(os.PathListSeparator)+os.Getenv("PATH")) +} + +func writeStub(t *testing.T, path, body string, mode os.FileMode) { + t.Helper() + if err := os.WriteFile(path, []byte(body), mode); err != nil { + t.Fatalf("WriteFile %s: %v", path, err) + } +} + +func quote(s string) string { return "'" + strings.ReplaceAll(s, "'", `'\''`) + "'" } + +// A comment body is the least trusted text this validator reads: anybody with an +// account can post one, on somebody else's pull request, and pre-push prints what +// it finds to a real terminal. A raw ESC surviving that trip is a control sequence +// the reader's emulator obeys — enough to scroll the finding away and paint a +// clean report over the top of it, which is the one lie this check exists to +// prevent. +func TestAPRCommentCannotWriteControlSequencesToTheTerminal(t *testing.T) { + root := gitRepo(t) + const pr = `{"number":7,"state":"OPEN","headRefName":"feat/x","url":"https://example.invalid/pr/7","title":"feat: x","body":"why"}` + // A screen-clearing sequence, a bell, and a right-to-left override, on a line + // that is a signature by any reading. Built from code points rather than typed, + // so the bytes under test survive every editor between here and the run — and + // marshalled, because that is how the forge hands them over. + esc, bel, rlo := string(rune(0x1b)), string(rune(0x07)), string(rune(0x202e)) + body := esc + "[2J" + esc + "[Hno findings" + bel + " generated with Claude Code" + rlo + comments, err := json.Marshal(map[string]any{ + "comments": []map[string]any{{ + "author": map[string]string{"login": "someone"}, + "url": "https://example.invalid/pr/7#c1", + "body": body, + }}, + "reviews": []any{}, + }) + if err != nil { + t.Fatalf("Marshal: %v", err) + } + stubGH(t, pr, string(comments)) + + set, err := AttributionPR(root) + if err != nil { + t.Fatalf("AttributionPR: %v", err) + } + found := set.Sorted() + if len(found) != 1 { + t.Fatalf("got %d findings, want 1: %+v", len(found), found) + } + for _, r := range found[0].Message { + if !unicode.IsGraphic(r) && r != ' ' { + t.Fatalf("finding carries %U to the terminal: %q", r, found[0].Message) + } + } + // And the line is still reported. Escaping it must not cost the reader the + // evidence it was written to show them. + if !strings.Contains(found[0].Message, "Claude Code") { + t.Errorf("the offending line went missing: %q", found[0].Message) + } +} + +// clip is the one place that escaping happens, so the two edges it owns are +// pinned here: a cut on a rune boundary rather than a byte one, and text that was +// already clean coming back the way it went in. +func TestClipCutsRunesAndLeavesCleanTextAlone(t *testing.T) { + if got := clip(" feat: a line "); got != "feat: a line" { + t.Errorf("clip collapsed to %q", got) + } + long := clip(strings.Repeat("é", 200)) + if !utf8.ValidString(long) { + t.Errorf("clip cut a rune in half: %q", long) + } + if n := utf8.RuneCountInString(long); n != 64 { + t.Errorf("clip returned %d runes, want 64", n) + } +}