diff --git a/.project/agents/workflows/discovered-work.md b/.project/agents/workflows/discovered-work.md index 9dd23f5..354647e 100644 --- a/.project/agents/workflows/discovered-work.md +++ b/.project/agents/workflows/discovered-work.md @@ -1,4 +1,4 @@ - + # Discovered work When actionable pending work appears during another task: @@ -6,6 +6,7 @@ When actionable pending work appears during another task: 1. Do not leave it only in comments, agent memory or an informal TODO. 2. Create a card in the same session with enough context and a source reference. 3. Relate it through `parent`, `depends`, `source` or record IDs. -4. Use `idea` only for unvalidated proposals; use a committed work type when a decision already exists. -5. Do not change owner priorities without explicit authorization. +4. Set `raised`: `reported` when a person asked for it, `derived` when you inferred it from the repository. Work you found is `derived` — say so rather than leaving it blank, because the difference is unrecoverable once the session ends and a reported card is a commitment to somebody where a derived one is a proposal. +5. Use `idea` only for unvalidated proposals; use a committed work type when a decision already exists. +6. Do not change owner priorities without explicit authorization. diff --git a/.project/cards/T-0210-nothing-on-a-card-says-whether-a-person-reported-i.md b/.project/cards/T-0210-nothing-on-a-card-says-whether-a-person-reported-i.md index 8b23397..062f8c6 100644 --- a/.project/cards/T-0210-nothing-on-a-card-says-whether-a-person-reported-i.md +++ b/.project/cards/T-0210-nothing-on-a-card-says-whether-a-person-reported-i.md @@ -1,7 +1,7 @@ --- id: T-0210 title: Nothing on a card says whether a person reported it or an agent inferred it -status: backlog +status: done type: feature priority: medium area: core @@ -9,7 +9,12 @@ tags: [protocol] effort: S scope: [packages/workfile/src/modules/cards] created: 2026-08-05 -updated: 2026-08-05 +updated: 2026-08-07 +verified: + at: "2026-08-07T23:51:17.103Z" + method: local + commit: eb12a11b8e26cedc67bcaf0b279e80542b40b4d7 + digest: "sha256:c57cb42ef79f929b2aed0bfa82975ba65b870920f19de81b85a792540ae5238d" --- Asked where one of eight cards came from, I could not answer from the record. I @@ -53,9 +58,21 @@ would reproduce the exact error that prompted this. They stay unmarked. ## Acceptance criteria -- [ ] A card records whether it was reported or derived, in a form that can be counted without parsing prose. -- [ ] The vocabulary is small, declared, and validated on write. -- [ ] Every surface that creates a card can set it: CLI, MCP and HTTP. -- [ ] `doctor` reports a card that carries none, as a warning rather than an error. -- [ ] Existing cards are left unmarked rather than guessed at, and that is stated. -- [ ] The agent protocol says to set it, in the workflow that files a card. +- [x] A card records whether it was reported or derived, in a form that can be counted without parsing prose. +- [x] The vocabulary is small, declared, and validated on write. +- [x] Every surface that creates a card can set it: CLI, MCP and HTTP. +- [x] `doctor` reports a card that carries none, as a warning rather than an error. +- [x] Existing cards are left unmarked rather than guessed at, and that is stated. +- [x] The agent protocol says to set it, in the workflow that files a card. + +## Activity + +- 2026-08-07 23:36Z illodev@local#42eb42f5 · claimed +- 2026-08-07 23:51Z illodev@local#42eb42f5 · released + +## Notes + +- 2026-08-07 23:51Z illodev@local#42eb42f5 — One thing the card left open, settled here: the doctor rule is bounded by date. Warning on every card that carries none would have reported 223 in this repository on the day the field shipped, and backfilling is not available — guessing which of them were reported reproduces the exact error that prompted the card. So the rule speaks about cards filed from 2026-08-08 and says nothing about the ones that could not answer. A date rather than a config value: a project does not choose when the field became available in the package it installed, and a knob would only be used to switch the rule off, which `--severity` and the baseline already do per project with a record of the decision. + +The field is `raised`, values `reported` and `derived`. Two existing guards caught what a new card field needs beyond the field itself: it has to be reserved against being declared an axis, and it has to be reachable from a `card create` flag. Both were failing tests, not things I remembered. +- 2026-08-07 23:51Z illodev@local#42eb42f5 — local verification: `raised` reaches a card through the CLI, MCP and HTTP, validated to two values with `CARD_RAISED_INVALID` on anything else, verified end to end with the shipped binary. No default: a card filed without it is one nobody classified, which is the fact worth keeping. The doctor rule fires for a card filed after the field existed and stays silent for one filed before, proven against a card dated past the cutoff rather than by waiting for the clock; answering it silences the rule. The generated `discovered-work` workflow now tells an agent to set it and says why the difference is unrecoverable. Reserved against being declared an axis and covered by the create-flag parity test, both of which were failing tests rather than things I remembered. Full gate green at 488 + 10. diff --git a/.project/cards/T-0224-a-trailing-separator-strip-is-written-five-times-a.md b/.project/cards/T-0224-a-trailing-separator-strip-is-written-five-times-a.md index f720eae..15357ec 100644 --- a/.project/cards/T-0224-a-trailing-separator-strip-is-written-five-times-a.md +++ b/.project/cards/T-0224-a-trailing-separator-strip-is-written-five-times-a.md @@ -1,7 +1,7 @@ --- id: T-0224 title: A trailing-separator strip is written five times, as a regex -status: backlog +status: done type: chore priority: low area: core @@ -9,6 +9,12 @@ tags: [security] effort: S created: 2026-08-07 updated: 2026-08-07 +scope: [packages/workfile/test] +verified: + at: "2026-08-07T23:51:17.943Z" + method: local + commit: eb12a11b8e26cedc67bcaf0b279e80542b40b4d7 + digest: "sha256:d21ebdd6298ef0abe47e0f7d4cc14beb7b991b70e7ff2be4f781a5dec60f916f" --- CodeQL reported `js/polynomial-redos` against `routeRoots` in `docs/validation.ts`: `replace(/\/+$/, "")` retries the anchored `+` from every start position, so a value of N slashes costs O(N²). The alert was high and it blocked a pull request that had nothing to do with it. @@ -19,6 +25,20 @@ Filed to record what is not done: nothing stops the sixth copy. The suite has no ## Acceptance criteria -- [ ] A test or lint rule fails on an anchored `+`/`*` applied to a config or record value. -- [ ] It names the linear alternative rather than only refusing. -- [ ] The existing six sites pass it. +- [x] A test or lint rule fails on an anchored `+`/`*` applied to a config or record value. +- [x] It names the linear alternative rather than only refusing. +- [x] The existing six sites pass it. + +## Activity + +- 2026-08-07 23:37Z illodev@local#42eb42f5 · claimed +- 2026-08-07 23:51Z illodev@local#42eb42f5 · released + +## Notes + +- 2026-08-07 23:51Z illodev@local#42eb42f5 — The card said nothing stops the sixth copy. Writing the rule found that there were already six more, and one of them mattered: `replace(/\n+$/, "")` in `core/frontmatter.ts`, applied to a record *body*, which nothing caps — unlike a title, refused past 80 characters on write. A body of half a million newlines followed by one other character is O(N²) with no bound but the disk. Fixed with a linear strip. CodeQL never reported it; it reported the one copy whose taint it could follow from a declared value. + +The five slug helpers keep the shape and are allowlisted with the bound that makes them safe rather than because they look fine: they run on a title, and the 80-character cap is the argument. Take the cap away and they are the next finding, which the allowlist says out loud. + +The rule caught two of its own faults on the way. It reported the comment that explains the shape — a rule that reports prose teaches people to stop reading it — and its `g`-flagged matcher carried `lastIndex` between files, so the staleness half silently skipped whichever file followed a match. Both mutation-proven now, along with a seventh copy and a stale allowlist entry. +- 2026-08-07 23:51Z illodev@local#42eb42f5 — local verification: A rule over `src` and `bin` refusing a `replace` whose pattern ends in an anchored `+$`/`*$`, naming `stripTrailingSlashes` as the alternative, with an allowlist that carries the bound making each entry safe and fails when an entry stops applying. It found a seventh site the card did not know about — `\n+$` over an uncapped record body — which is fixed. Mutation-proven four ways: a new copy in an unbounded file, a stale allowlist entry, and the two faults the rule had itself (reporting its own documentation, and a `g` matcher carrying lastIndex between files). diff --git a/.project/changelog/unreleased/CHG-0159-a-card-says-whether-a-person-reported-it-or-an-agent-derived-it.md b/.project/changelog/unreleased/CHG-0159-a-card-says-whether-a-person-reported-it-or-an-agent-derived-it.md new file mode 100644 index 0000000..c72bb7a --- /dev/null +++ b/.project/changelog/unreleased/CHG-0159-a-card-says-whether-a-person-reported-it-or-an-agent-derived-it.md @@ -0,0 +1,12 @@ +--- +id: CHG-0159 +title: A card says whether a person reported it or an agent derived it +type: added +area: core +visibility: public +cards: [T-0210] +created: 2026-08-07 +updated: 2026-08-07 +--- + +`raised: reported | derived`, on every surface that creates a card. Neither `origin` nor `source` answered this: `origin` takes record ids, which is the provenance of discovered work, and `source` is a path checked on disk, so a report made in conversation had nowhere to go. The distinction is what changes priority — a reported card is a commitment to somebody and a derived one is a proposal that costs nothing to discard — and it is unrecoverable once the session ends. Cards filed before the field existed are left unmarked rather than guessed at, and `doctor` asks only about cards filed since. diff --git a/.project/changelog/unreleased/CHG-0160-a-trailing-strip-over-an-uncapped-body-no-longer-backtracks.md b/.project/changelog/unreleased/CHG-0160-a-trailing-strip-over-an-uncapped-body-no-longer-backtracks.md new file mode 100644 index 0000000..9f3f488 --- /dev/null +++ b/.project/changelog/unreleased/CHG-0160-a-trailing-strip-over-an-uncapped-body-no-longer-backtracks.md @@ -0,0 +1,12 @@ +--- +id: CHG-0160 +title: A trailing strip over an uncapped body no longer backtracks +type: security +area: core +visibility: public +cards: [T-0224] +created: 2026-08-07 +updated: 2026-08-07 +--- + +`replace(/\n+$/, "")` in the frontmatter reader ran over a record body, which nothing caps, and retries the anchored quantifier from every start position — O(N²) with no bound but the disk. Replaced by a linear strip, and a test now refuses the shape across the package with an allowlist that has to state the bound making each remaining site safe. diff --git a/packages/workfile/bin/workfile.ts b/packages/workfile/bin/workfile.ts index e627d93..430e55c 100644 --- a/packages/workfile/bin/workfile.ts +++ b/packages/workfile/bin/workfile.ts @@ -147,6 +147,7 @@ const USAGE: Record = { "workfile card list [--json] [--axis context=treasury] # repeatable, once per axis", "workfile card show ID [--json]", "workfile card create --title TITLE [--area AREA] [--type TYPE] [--priority PRIORITY]", + "workfile card create --title TITLE --raised reported|derived # a person asked, or you inferred it", "workfile card create --json-input FILE # recommended: body, parent, source, tags in one call", "workfile card create --title TITLE --axis context=treasury # repeatable; see `workfile schema`", "workfile card patch ID --json-input FILE [--expected-revision REV]", @@ -333,6 +334,7 @@ const COMMAND_FLAGS: Record = { ], "card create": [ "--area", + "--raised", "--axis", "--body", "--depends", @@ -1532,6 +1534,7 @@ async function cardCommand(workspace, action) { ...(option("--body") ? { body: option("--body") } : {}), ...(option("--parent") ? { parent: option("--parent") } : {}), ...(option("--source") ? { source: option("--source") } : {}), + ...(option("--raised") ? { raised: option("--raised") } : {}), ...(option("--milestone") ? { milestone: option("--milestone") } : {}), diff --git a/packages/workfile/docs/SPEC.md b/packages/workfile/docs/SPEC.md index dbd0470..e0be4c4 100644 --- a/packages/workfile/docs/SPEC.md +++ b/packages/workfile/docs/SPEC.md @@ -672,6 +672,7 @@ A card carries five relationship fields, and they are not interchangeable: | `parent` | one card ID | this card is **part of** that one | | `depends` | card IDs | those must close **before** this one is actionable | | `origin` | record IDs, any kind | this card was **discovered while working on** those | +| `raised` | `reported` \| `derived` | who put it on the board: a person asked, or an agent inferred it from the repository | | `related` | record IDs, any kind | worth reading alongside; no direction, no claim | | `source` | a repository-relative path | the file the work came from, checked on disk | diff --git a/packages/workfile/docs/cli.md b/packages/workfile/docs/cli.md index c0c99f5..116e3a3 100644 --- a/packages/workfile/docs/cli.md +++ b/packages/workfile/docs/cli.md @@ -210,6 +210,7 @@ workfile card list [--status S] [--area A] [--type T] [--priority P] [--parent I [--with-body] [--json] workfile card show ID [--json] workfile card create --title TITLE [--area AREA] [--type TYPE] [--priority PRIORITY] +workfile card create --title TITLE --raised reported|derived [--parent ID] [--source PATH] [--tags a,b] [--scope PATH,PATH] [--depends ID,ID] [--related ID,ID] [--origin ID,ID] [--milestone M] [--effort S|M|L] diff --git a/packages/workfile/src/config/defaults.ts b/packages/workfile/src/config/defaults.ts index 907c5ac..36c84c8 100644 --- a/packages/workfile/src/config/defaults.ts +++ b/packages/workfile/src/config/defaults.ts @@ -62,6 +62,7 @@ export const CARD_RESERVED_KEYS = Object.freeze([ "origin", "parent", "priority", + "raised", "related", "revision", "scope", diff --git a/packages/workfile/src/core/frontmatter.ts b/packages/workfile/src/core/frontmatter.ts index a5fceb0..1b9e4ec 100644 --- a/packages/workfile/src/core/frontmatter.ts +++ b/packages/workfile/src/core/frontmatter.ts @@ -119,6 +119,24 @@ export type FrontmatterStyle = const KEY_LINE = /^([A-Za-z_][\w.-]*):(.*)$/; const BLOCK_ITEM = /^(\s+)-\s?(.*)$/; + +/** + * Trailing newlines removed without a regex. + * + * `replace(/\n+$/, "")` retries the anchored `+` from every start position, so a + * value that ends in anything other than a newline costs O(N²) — and this one is + * applied to a *body*, which nothing caps. A card title is refused past 80 + * characters, so the same shape in the slug helpers is quadratic over a bounded + * input; a body read from `--body-file` is bounded by the disk. Found while + * writing the rule for T-0224, not by CodeQL, which reported only the copy whose + * taint it could follow. + */ +function stripTrailingNewlines(value: string): string { + let end = value.length; + while (end > 0 && value[end - 1] === "\n") end -= 1; + return end === value.length ? value : value.slice(0, end); +} + const BLOCK_SCALAR = /^([|>])([+-]?\d*)\s*$/; /** ` - id: gate-test` — the line that opens one record in a `records` block. */ const RECORD_ITEM = /^(\s+)-\s+([A-Za-z_][\w.-]*):\s*(.*)$/; @@ -289,7 +307,7 @@ function scanEntries(lines, listKeys) { const text = dedent(block); value = style === "literal" - ? text.join("\n").replace(/\n+$/, "") + ? stripTrailingNewlines(text.join("\n")) : text.join(" ").replace(/\s+/g, " ").trim(); } else if ( meaningful.length && diff --git a/packages/workfile/src/modules/agents/agents.ts b/packages/workfile/src/modules/agents/agents.ts index fc01bd2..e872bdb 100644 --- a/packages/workfile/src/modules/agents/agents.ts +++ b/packages/workfile/src/modules/agents/agents.ts @@ -153,8 +153,9 @@ When actionable pending work appears during another task: 1. Do not leave it only in comments, agent memory or an informal TODO. 2. Create a card in the same session with enough context and a source reference. 3. Relate it through \`parent\`, \`depends\`, \`source\` or record IDs. -4. Use \`idea\` only for unvalidated proposals; use a committed work type when a decision already exists. -5. Do not change owner priorities without explicit authorization.`, +4. Set \`raised\`: \`reported\` when a person asked for it, \`derived\` when you inferred it from the repository. Work you found is \`derived\` — say so rather than leaving it blank, because the difference is unrecoverable once the session ends and a reported card is a commitment to somebody where a derived one is a proposal. +5. Use \`idea\` only for unvalidated proposals; use a committed work type when a decision already exists. +6. Do not change owner priorities without explicit authorization.`, "record-knowledge": `# Record knowledge Choose the record first, then the collection: diff --git a/packages/workfile/src/modules/cards/cards.ts b/packages/workfile/src/modules/cards/cards.ts index b35d4a3..b353a2b 100644 --- a/packages/workfile/src/modules/cards/cards.ts +++ b/packages/workfile/src/modules/cards/cards.ts @@ -40,6 +40,17 @@ import { CARD_TYPES } from "../../config/defaults.js"; +/** + * The first day `raised` could be answered, so the rule below can be quiet about + * every card filed before it. + * + * A date rather than a config value: a project does not get to choose when this + * field became available in the package it installed, and a knob here would only + * be used to switch the rule off — which `doctor --severity` and the baseline + * already do, per project, with a record of the decision. + */ +const RAISED_EXPECTED_FROM = "2026-08-08"; + export const CARD_LIST_KEYS = new Set([ "tags", "depends", @@ -340,6 +351,37 @@ export async function diagnoseCards({ // and had no rule at all (T-0223). The same argument // `duplicate-record-id` makes — a module sees one kind, and this // question is about all of them. + /** + * A card that does not say whether a person asked for it. + * + * `warning`, not `error`, because it is a fact about how the card was + * filed and not a defect in the record — and because the repair is a + * judgement only the filer can make. + * + * Bounded by date, and that is the part the card did not settle. Every + * card written before the field existed carries none, and this repository + * alone holds 223 of them; reporting all of them would drown the doctor + * on the day the field shipped and teach everyone to ignore the rule. + * Backfilling is not available either — guessing which of them were + * reported would reproduce the exact error that prompted T-0210. So the + * rule speaks about cards filed from the day it could be answered, and + * says nothing about the ones that could not. + */ + if ( + !card.raised && + String(card.created || "") >= RAISED_EXPECTED_FROM && + !card.archived + ) { + issues.push( + issue( + "warning", + "raised-missing", + card, + "Does not say whether a person reported it or it was derived; " + + "set `raised: reported` or `raised: derived`" + ) + ); + } if ((card.title || "").length > 80) { issues.push( issue( diff --git a/packages/workfile/src/modules/cards/mutations.ts b/packages/workfile/src/modules/cards/mutations.ts index bba3799..ac27487 100644 --- a/packages/workfile/src/modules/cards/mutations.ts +++ b/packages/workfile/src/modules/cards/mutations.ts @@ -767,6 +767,9 @@ export async function createCard(workspace, input, { maxRetries = 32, now }: any ...(input.scope?.length ? { scope: input.scope } : {}), ...(input.related?.length ? { related: input.related } : {}), ...(input.origin?.length ? { origin: input.origin } : {}), + // Beside `origin` and `source`, because it answers the neighbouring + // question and a reader meets the three together. + ...(input.raised ? { raised: input.raised } : {}), ...(input.start ? { start: input.start } : {}), ...(input.due ? { due: input.due } : {}), ...(input.claimed_by ? { claimed_by: input.claimed_by } : {}), diff --git a/packages/workfile/src/modules/cards/validation.ts b/packages/workfile/src/modules/cards/validation.ts index 5be6da2..e902692 100644 --- a/packages/workfile/src/modules/cards/validation.ts +++ b/packages/workfile/src/modules/cards/validation.ts @@ -47,9 +47,31 @@ export const CARD_PATCHABLE_FIELDS = Object.freeze([ "due", "related", "origin", + "raised", "verify" ]); +/** + * How a card came to be on the board, which nothing recorded. + * + * Asked where one of eight cards came from, the record could not answer: the + * commit message that filed them was read, the grouping of its paragraphs was + * taken as evidence that the card was the agent's own, and it was wrong — it was + * item six of a list the owner had written out. The fields that look like they + * should carry this are both something else. `origin` takes record ids, which is + * the provenance of *discovered* work: what were you doing when you found this. + * `source` takes a repository-relative path and is checked on disk, so a report + * made in conversation has nothing to put there. + * + * Two values, and the smallness is the decision. A person reported it, or it was + * derived from reading the code. More than two and nobody picks correctly — and + * the distinction that actually changes behaviour is exactly this one: a reported + * card is a commitment to somebody, and a derived card is a proposal that costs + * nothing to discard. Six months of the two mixed together is a backlog nobody + * can prioritise (T-0210). + */ +export const CARD_RAISED_VALUES = Object.freeze(["reported", "derived"]); + const DATE_RE = /^\d{4}-\d{2}-\d{2}$/; const TIMESTAMP_RE = /^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}(?:\.\d+)?Z$/; @@ -561,6 +583,12 @@ export function validateCardCandidate(workspace, candidate, cards, currentId = n fail("CARD_SELF_ORIGIN", "A card cannot originate from itself."); } } + if (candidate.raised && !CARD_RAISED_VALUES.includes(candidate.raised)) { + fail( + "CARD_RAISED_INVALID", + `raised must be one of ${CARD_RAISED_VALUES.join(", ")}.` + ); + } const hasActor = Boolean(candidate.claimed_by); const hasTimestamp = Boolean(candidate.claimed_at); if (hasActor !== hasTimestamp) { diff --git a/packages/workfile/src/modules/mcp/tools.ts b/packages/workfile/src/modules/mcp/tools.ts index 30801d8..3462731 100644 --- a/packages/workfile/src/modules/mcp/tools.ts +++ b/packages/workfile/src/modules/mcp/tools.ts @@ -872,6 +872,16 @@ const TOOL_DEFINITIONS = [ "IDs of cards that must close first. Blocks this card from being ranked as actionable." ), source: text("Where the work came from: an issue URL, a person, a meeting."), + raised: { + type: "string", + enum: ["reported", "derived"], + description: + "Who put this on the board: `reported` when a person asked for " + + "it, `derived` when you inferred it from reading the repository. " + + "Set it — a reported card is a commitment to somebody and a " + + "derived one is a proposal, and the difference is unrecoverable " + + "once the session ends." + }, tags: strings("Free-form tags for filtering."), scope: strings( "Repository paths this work will change. Declared here, enforced when the card is claimed." diff --git a/packages/workfile/test/cli.test.ts b/packages/workfile/test/cli.test.ts index 967c1aa..d18e9ec 100644 --- a/packages/workfile/test/cli.test.ts +++ b/packages/workfile/test/cli.test.ts @@ -1123,7 +1123,10 @@ const CREATE_FLAG_COVERAGE = { // Any record id, not only a card: a decision spawns work as often as a // card does, and restricting the edge to cards throws away the half of the // provenance tree that explains why the work exists. - origin: ["--origin", "T-0001,ADR-0001"] + origin: ["--origin", "T-0001,ADR-0001"], + // Two values and no default. A card filed without it is a card nobody has + // classified, which is a fact worth keeping rather than one to invent. + raised: ["--raised", "reported"] }; test("card create reaches every field the mutation accepts", async () => { diff --git a/packages/workfile/test/raised.test.ts b/packages/workfile/test/raised.test.ts new file mode 100644 index 0000000..fbe0421 --- /dev/null +++ b/packages/workfile/test/raised.test.ts @@ -0,0 +1,120 @@ +import assert from "node:assert/strict"; +import test from "node:test"; +import { readFile, writeFile } from "node:fs/promises"; +import { join } from "node:path"; + +import { createTestWorkspace } from "./support/workspace.ts"; + +import { createCard, diagnoseCards, loadCards, patchCard } from "../dist/src/index.js"; + +/** + * A card says whether a person asked for it or an agent inferred it. + * + * Asked where one of eight cards came from, the record could not answer. The + * commit message that filed them was read, the grouping of its paragraphs was + * taken as evidence the card was the agent\'s own, and that was wrong — it was + * item six of a list the owner had written out. The inference was wrong and the + * record could not correct it, because it did not carry the fact at all (T-0210). + * + * `origin` and `source` both look like they should and are something else: + * `origin` takes record ids, which is the provenance of *discovered* work, and + * `source` is a repository path checked on disk, so a report made in conversation + * has nothing to put there. + */ + +test("the vocabulary is two values and anything else is refused", async () => { + const { workspace, cleanup } = await createTestWorkspace(); + try { + for (const raised of ["reported", "derived"]) { + const created = await createCard(workspace, { + title: `Filed as ${raised}`, + area: "api", + raised + }); + assert.equal(created.card.raised, raised); + } + // Two, and the smallness is the decision: more than two and nobody picks + // correctly, while this is the distinction that changes what you do — a + // reported card is a commitment to somebody and a derived one is a + // proposal that costs nothing to discard. + await assert.rejects( + () => createCard(workspace, { title: "Guessed", area: "api", raised: "guessed" }), + (error: any) => { + assert.equal(error.code, "CARD_RAISED_INVALID"); + assert.match(error.message, /reported, derived/); + return true; + } + ); + } finally { + await cleanup(); + } +}); + +test("it can be set after the fact, on a card that was filed without it", async () => { + const { workspace, cleanup } = await createTestWorkspace(); + try { + const created = await createCard(workspace, { title: "Filed blank", area: "api" }); + assert.equal(created.card.raised, undefined, "a card gets no default"); + // No default, deliberately: a value nobody chose would be the same wrong + // answer the card was filed about, written by the tool instead of guessed + // by a reader. + await patchCard(workspace, created.card.id, { raised: "reported" }); + const { cards } = await loadCards(workspace); + assert.equal( + cards.find((card: any) => card.id === created.card.id)?.raised, + "reported" + ); + } finally { + await cleanup(); + } +}); + +/** + * The rule is bounded by date, and that bound is the part the card left open. + * + * Every card written before the field existed carries none — 223 in this + * repository alone — so reporting all of them would drown the doctor on the day + * it shipped and teach everyone to ignore the rule. Backfilling is not available + * either: guessing which of them were reported reproduces the exact error that + * prompted the card. So the rule speaks about cards filed from the day it could + * be answered and says nothing about the ones that could not. + */ +test("doctor asks only about cards filed since the field existed", async () => { + const { root, workspace, cleanup } = await createTestWorkspace(); + try { + const report = async () => { + const loaded = await loadCards(workspace); + const result = await diagnoseCards({ + ...loaded, + workspace, + checkPaths: false, + checkGit: false + }); + return result.issues.filter((issue: any) => issue.code === "raised-missing"); + }; + + // The fixture predates the field, so it is left alone. + assert.deepEqual(await report(), [], "an old card was asked about"); + + const created = await createCard(workspace, { title: "Filed blank", area: "api" }); + const path = join(root, workspace.config.cards.path, created.card.file); + const before = await readFile(path, "utf8"); + + // Dated after the cutoff by hand rather than by waiting for the clock, + // which is the only way to test a date boundary without owning time. + await writeFile(path, before.replace(/^created:.*$/m, "created: 2099-01-01")); + const flagged = await report(); + assert.deepEqual( + flagged.map((issue: any) => issue.id), + [created.card.id], + "a card filed after the field existed was not asked about" + ); + assert.equal(flagged[0].severity, "warning"); + + // And answering it silences the rule. + await patchCard(workspace, created.card.id, { raised: "derived" }); + assert.deepEqual(await report(), []); + } finally { + await cleanup(); + } +}); diff --git a/packages/workfile/test/redos.test.ts b/packages/workfile/test/redos.test.ts new file mode 100644 index 0000000..98a76c7 --- /dev/null +++ b/packages/workfile/test/redos.test.ts @@ -0,0 +1,116 @@ +import assert from "node:assert/strict"; +import test from "node:test"; +import { readdir, readFile } from "node:fs/promises"; +import { join } from "node:path"; +import { fileURLToPath } from "node:url"; + +/** + * An anchored quantifier applied to something a caller supplies. + * + * CodeQL reported `js/polynomial-redos` as high against `routeRoots`: + * `replace(/\/+$/, "")` retries the anchored `+` from every start position, so a + * value of N separators costs O(N²). It reported exactly one of the six copies of + * that spelling, because it was the one whose taint it could follow from a + * declared value — so fixing what was reported would have left five, and the next + * report waiting on whichever grew an input first (T-0224). + * + * This refuses the shape by name instead. It is not taint analysis and does not + * pretend to be: what it does is make the seventh copy a decision somebody writes + * down here rather than an accident, and name the linear alternative. + */ + +const packageRoot = fileURLToPath(new URL("..", import.meta.url)); + +/** + * The shape: a `replace` whose pattern ends in `+$/` or `*$/`. + * + * Anchored at the end is what makes it quadratic — the engine cannot fail fast, + * because a match may still start further along. An unanchored `+` is linear and + * is not what this is about. + */ +const ANCHORED_QUANTIFIER_SOURCE = "replace\\(\\s*/[^/\\n]*[+*]\\$/"; + +/** A fresh matcher per use: a `g` regex carries `lastIndex` between calls, and + * sharing one across files made the staleness check below skip whichever file + * came after a match. */ +const anchoredQuantifier = (flags = "") => + new RegExp(ANCHORED_QUANTIFIER_SOURCE, flags); + +/** + * Where the shape survives, and why each one is allowed to. + * + * Every entry is a bounded input, and the bound is the argument — not "it looks + * fine". The five slug helpers run on a title, and a title is refused past 80 + * characters on write (`CARD_TITLE_TOO_LONG`), so the quadratic is over 80 + * characters at worst. Take the cap away and these become the next finding. + */ +const ALLOWED = new Map([ + ["src/modules/cards/slug.ts", "a card title, capped at 80 characters on write"], + ["src/modules/memory/memory.ts", "a memory record title, same cap"], + ["src/modules/docs/docs.ts", "a document title, same cap"], + ["src/modules/changelog/changelog.ts", "a fragment title or a version string"], + ["src/modules/init/initializer.ts", "a project name, supplied once at init"] +]); + +async function sourcesUnder(directory: string, found: string[] = []) { + for (const entry of await readdir(join(packageRoot, directory), { + withFileTypes: true + })) { + if (entry.name === "node_modules" || entry.name === "dist") continue; + const path = `${directory}/${entry.name}`; + if (entry.isDirectory()) await sourcesUnder(path, found); + else if (/\.(ts|mjs)$/.test(entry.name)) found.push(path); + } + return found; +} + +test("no new anchored quantifier is applied to a value a caller supplies", async () => { + const files = [ + ...(await sourcesUnder("src")), + ...(await sourcesUnder("bin")) + ]; + assert.ok(files.length > 40, `scanned only ${files.length} files; the walk broke`); + + const found: string[] = []; + for (const file of files) { + const source = (await readFile(join(packageRoot, file), "utf8")).replaceAll( + "\r\n", + "\n" + ); + const lines = source.split("\n"); + for (const match of source.matchAll(anchoredQuantifier("g"))) { + if (ALLOWED.has(file)) continue; + const number = source.slice(0, match.index).split("\n").length; + // A comment explaining the shape is not the shape. This caught its + // own documentation on the first run — the paragraph in + // `frontmatter.ts` that says why the loop is there quotes the regex + // it replaced — and a rule that reports prose teaches people to + // stop reading it. + const text = lines[number - 1].trimStart(); + if (text.startsWith("*") || text.startsWith("//") || text.startsWith("/*")) { + continue; + } + found.push(`${file}:${number} ${match[0]}`); + } + } + assert.deepEqual( + found, + [], + "These retry an anchored quantifier from every start position, which is " + + "O(N²) in the length of the value. Strip with a loop instead — " + + "`stripTrailingSlashes` in `core/glob.ts` is the one for separators — " + + "or add the file to ALLOWED with the bound that makes it safe:\n " + + found.join("\n ") + ); + + // And the allowlist is not a place things rot: an entry naming a file that no + // longer has the shape is an entry that stopped meaning anything. + for (const [file, reason] of ALLOWED) { + const source = await readFile(join(packageRoot, file), "utf8"); + assert.match( + source, + anchoredQuantifier(), + `${file} is allowed for "${reason}" and no longer has the shape; drop the entry` + ); + } +}); diff --git a/packages/workfile/ui/src/types.ts b/packages/workfile/ui/src/types.ts index 4495d99..9a9740e 100644 --- a/packages/workfile/ui/src/types.ts +++ b/packages/workfile/ui/src/types.ts @@ -54,6 +54,17 @@ export interface Task { depends?: string[]; /** Records this card was discovered while working on. Any record kind. */ origin?: string[]; + /** + * How the card came to be on the board: `reported` when a person asked for + * it, `derived` when an agent inferred it from the repository. + * + * Neither `origin` nor `source` answers this. `origin` is the provenance of + * discovered work — what you were doing when you found it — and `source` is a + * path checked on disk, so a report made in conversation has nothing to put + * there. Absent on every card filed before the field existed, and those are + * deliberately not guessed at (T-0210). + */ + raised?: "reported" | "derived"; related?: string[]; milestone?: string; source?: string;