diff --git a/.dev-loop/INGEST_REPORT.md b/.dev-loop/INGEST_REPORT.md index bf48b5c..3a965e5 100644 --- a/.dev-loop/INGEST_REPORT.md +++ b/.dev-loop/INGEST_REPORT.md @@ -1,72 +1,237 @@ -# Ingest report — GitHub-trending agent-skill practices +# Knowledge flush — 13 insight(s) ingested (21 claimed, 2 dropped, 6 released) -Distilled from a session analyzing the week's GitHub-trending repos. Ten repos -were triaged; six carried transferable engineering practice, and five directive -units were ingested (4 new pages + 1 amend). Four repos (airllm, airi, -pascalorg/editor, microsoft/AI-For-Beginners) were out of domain or unverified -at code level and dropped. +Cross-Check: 1× independent adversarial `claude` CLI headless pass over the 5 new pages — it refuted the changed-files gate page's "prettier exits 0 on an empty match set" claim; re-measured against Prettier 3.7.4, confirmed the reviewer was right (unmatched operand exits **2**), and rewrote the page, report rows 3/5 and `log.md`. Other 5 claim groups verdicted sound. Limits: the reviewer's sandbox blocked repo reads, so source-quote-supports-directive and self-contradiction dimensions went unaudited (details in `## Cross-Check`). + +Queue drained under run id `20260827-125731-38371` (this session is the detached +`hooks/auto-flush.sh` run; its step-0 acquire resolved re-entrantly to +`already-owned`, not to a competing holder). 21 rows were claimed; 13 are +ingested below, 2 are retired as out-of-layer, and 6 are released back to +`pending` for a later flush because each needs its own page rather than a row, +and rushing six more pages in one pass would have lowered the bar on all of them. ## Verified best-practice -Every directive is cited to a source file confirmed live via `gh api` before -writing (all 9 cited repo paths returned their `.path` — none fabricated): - -1. **Deterministic-preprocess × LLM-judgment review pipeline** — `alibaba/open-code-review`. - README "Core Design" + `skills/open-code-review/SKILL.md` (rule.json layering). - The ~1/9-token figure is the repo's own AACR-Bench measurement (50 repos / - 200 PRs / 1,505 labels) — recorded as vendor-self-measured, not independently - reproduced. Corrects the source podcast's mis-stated "19%". -2. **Progressive-disclosure knowledge artifact** — `virgiliojr94/book-to-skill`. - README "What it generates" + SKILL.md Step 7 (BOOK_TYPE × DEPTH budget - matrix) and Step 2.6 (grep/sed slice access for ≥50K-token originals). -3. **Agent-skill/MCP supply-chain vetting** — `zhaoxuya520/reverse-skill` - `skills/ops/skill-supply-chain.md` (OWASP-cited pre-install checklist) + - `different-ai/openwork` `skills-lock.json` (source+hash pinning) + - `virgiliojr94/book-to-skill` `SECURITY-NOTICE.md` (documented malicious - re-upload: TLS bypass, wallet collection, C2). -4. **Authorization-scope persistence for dangerous capabilities** — - `zhaoxuya520/reverse-skill` `skills/ops/scope-contract.md` (`scope.md` with - `auth.status: granted` gate, force-flag non-bypass, `network_profile`) + - `RULES.md`. -5. **Pre-send self-check (amend)** — `ayghri/i-have-adhd` - `skills/i-have-adhd/SKILL.md` "Pre-send check" pairing each output rule with - a pre-emit predicate. +Every external claim below was live-fetched this session and quoted in the page's +`Sources` block. Field evidence carries the repo, date, and the measured numbers. + +| # | Claim | Sources checked | How verified | Confidence | +|---|-------|-----------------|--------------|------------| +| 1 | `now()` is `transaction_timestamp()` (fixed at transaction start) while `clock_timestamp()` "changes even within a single SQL statement"; `RETURNING` yields computed defaults "without needing a separate database query" | postgresql.org `functions-datetime`, `dml-returning`, `transaction-iso` | Fetched; both key sentences quoted verbatim into the page | verified | +| 2 | A boundary recomputed in a follow-up step is a *second, later* `now`, widening a `<= boundary` set | Field: `rtb-unified` `packages/orpc/src/routers/batch.ts` — codifies "one `now` per decision" and passes `now` into the boundary helper; its result type omits the boundary, which is the shape that invites recomputation | Read the invariant and the signature in the cited file | field-tested | +| 3 | **[CORRECTED BY CROSS-CHECK]** The vacuous-pass shapes for `prettier --check` are: no operands (rc **0**), all operands ignore-filtered (rc **0**), and unsupported extensions with `--ignore-unknown` (rc **0**). A pattern/operand matching nothing exits **2** — it prints the success sentence *and* an unmatched-pattern error | prettier.io CLI + ignore docs; local measurement, Prettier 3.7.4 | The first draft generalised "empty match set ⇒ exit 0" from a field log where both messages appeared together. The independent reviewer flagged it; I then ran all seven cases against a real binary and rewrote the page around the measured table | verified (re-measured) | +| 4 | zsh does not word-split unquoted parameter expansions by default, so `cmd $FILES` arrives as **one** operand | zsh FAQ ch. 3 (`SH_WORD_SPLIT`) | Fetched; quoted ("By default, zsh does not have that behaviour: the variable remains intact") | verified | +| 5 | The zsh word-split operand exits **2**, but its log still carries the success sentence — so the log misleads even though the exit code does not | Field 2026-08-24 (`rtb-unified`, zsh) + local measurement 2026-08-27 | Field log showed both messages together; the local run reproduced it as `rc=2`. The page now says explicitly that this row fails loudly *unless* `--no-error-on-unmatched-pattern` is set. Probe placement re-confirmed: `.claude/tmp/` is `.gitignore`d, so a probe there passes at rc 0 | verified (re-measured) | +| 6 | TypeScript applies excess-property/contextual typing to fresh object literals, so a value of a type can be constructed with the type's name absent from the text | typescriptlang.org handbook, *Object Types* | Fetched; confirmed the check follows from the contextual type, not from a written annotation | verified | +| 7 | `tsc`'s program is `files` ∪ `include` ∪ transitive imports; `exclude` "only changes which files are included as a result of the `include` setting" and does not stop an imported file entering the program | typescriptlang.org TSConfig `#include`, `#exclude` | Fetched; the `exclude` sentence quoted (it sharpens the rule to "in the program", not "in `include`") | verified | +| 8 | Consequence of 6+7 measured | Field 2026-08-24/25 (`rtb-unified`): `grep "DealViewer"` reported 3 construction sites, actual 8 — the missed set included production wiring `routers/deal.ts:38`; `ContractScopeActor` 7→~22. Separately, `packages/orpc/tsconfig.json` `include: ["src/**/*"]` produced 3 production + 13 api-test errors and **zero** for `__tests__/routers/deal.test.ts`, whose 6 sites appeared only as 6 failing tests | Counts recorded from the cited runs | verified | +| 9 | cgroup v2: `memory.peak` is max usage since creation/reset; at `memory.max` "the OOM killer is invoked in the cgroup"; in `memory.events`, `max` counts times usage "was about to go over the max boundary" — **distinct** from `oom_kill` | docs.kernel.org cgroup-v2 admin guide | Fetched; all four quoted. This corrected the candidate, which had read a non-zero `max` as a kill; the page now states the distinction explicitly | verified | +| 10 | An `exec`'d process joins the container's cgroup and is invisible to the application's own semaphore | kubernetes.io `manage-resources-containers`, `assign-memory-resource`, `kubectl exec` reference + field 2026-08-26 (review-bot pod, `limits.memory: 3Gi`): `memory.current` 2.54 GiB, `memory.peak` 3.0 GiB (at the limit), `memory.events: max 5`, while `maxConcurrentAgents: 20` reported free slots | Docs fetched; pod numbers from the cited measurement | verified | +| 11 | Basename-keyed mutation backups collide across directories and restore cross-writes; an untracked file's `git diff` is empty whether restored or destroyed | Field 2026-08-21 (`rtb-unified`, NEWRTB-2936): restore wrote `schemas/deal.ts` into `routers/deal.ts` → `Cannot find module './common.js'`, `grep -c dealRouter` = 0; **both files were 154 lines**, so a line-count check passed; after re-keying, M9/M10 flipped SURVIVED→KILLED. Plus stryker mutant-states / pitest for the verdict vocabulary | Reproduced end to end in the cited run | field-tested | +| 12 | A negative assertion is vacuous when the fixture never supplies the triggering input | Field 2026-08-25 (`rtb-unified`): with `staleQueuedJobIds: []` the code early-returned; the widening the assertion claimed to catch survived 116/116 green | Mutation applied and observed | field-tested | +| 13 | A body-level (non-inline) review finding cites no file, so rebutting from an assumed file rejects real defects | Field 2026-08-19 (PR #327 r16): quote matched `report.py:393/416/425`, not the assumed `fill_plan.py:307` — sibling modules, one already fixed | Grep resolved the quote to the real site | field-tested | +| 14 | Unifying two duplicate allowlists defaults to the union and silently widens each side | Field 2026-08-25 (`rtb-unified` PR #965): folding `DISPLAYABLE_ERROR_CODES` into `USER_FACING_ERROR_CODES` would have added `UNAUTHORIZED` + `VALIDATION_ERROR`, exposing raw server messages as inline UI errors; caught only by computing the difference first | Difference computed before the merge | field-tested | + +Not upgraded: nothing was marked `verified` on field evidence alone. Two pages +carry `confidence: field-tested` (`mutation-harness-file-custody`, +plus the pre-existing `evaluating-review-feedback`), and no candidate was +recorded as `verified` without a fetched primary source. ## Existing-layer check -Read the adjacent pages before writing to merge-not-duplicate. The -progressive-disclosure and review-pipeline triggers were absent; the -i-have-adhd self-check was the only genuinely new element over -binding-instructions (its exception-hierarchy/precedence content is already -covered there and was NOT re-ingested). block/buzz's audit-log design and -openwork's 2-tool gateway were considered and rejected (off usage-context / -below directive bar; the gateway is also in tension with agent-tool-granularity). +Method: routed via `INDEX.md` → domain `index.md`; then built a full id+title +index of all 265 pre-existing pages and probed it with concept greps +(`clock_timestamp|clock skew`, `changed[- ]files|--ignore-unknown`, `tsconfig`, +`contextual typ|excess property`, `set difference|allowlist`, `cgroup`, +`basename|backup.*restore`, `2>&1`, `delta|baseline`) before deciding new vs merge. + +Pages read: testing-quality-source-text-wiring-assertions, testing-quality-tests-that-cannot-fail, backend-common-change-impact-call-site-enumeration, backend-common-change-impact-widening-a-closed-value-table, qa-process-evaluating-review-feedback, infrastructure-containers-host-cgroup-visibility, testing-quality-behavior-not-implementation -Pages read: qa-process-evaluating-review-feedback, qa-process-adversarial-change-review, backend-common-llm-binding-instructions-for-agents, backend-common-llm-context-window-budget, backend-common-api-design-agent-tool-granularity, security-agent-exposure-in-session-tool-exposure, security-dependencies-supply-chain, infrastructure-agent-orchestration-autonomous-decision-rulings +Findings: + +- **Zero coverage** (→ new pages): changed-files-only gates, tsconfig/contextual + typing, allowlist set-difference, app-clock-vs-DB-timestamp, exec-into-a-running-container. + The concept greps returned no hits for these; the clock hits were incidental + (offline sync, token handling) and none compared an app clock to a DB column. +- **Already covered — one candidate all but retired.** The comment-stripping + insight is `source-text-wiring-assertions` step 2 verbatim ("Make the + assertion's subject the file with comments removed"), and its false-RED and + negative/count false-GREEN shapes are already edge rows. Only the *empty-slice* + consequence was new, so that alone was merged. +- **Line-cap conflict handled without breaking the invariant.** + `source-text-wiring-assertions` sits at exactly **120** body lines (the + documented cap). Rather than add a row and violate maintenance invariant 5, the + new nuance and the new field evidence were merged **in place** into an existing + edge row and an existing source bullet. Body count re-measured after editing: + still 120. +- **No conflicts found.** Nothing ingested contradicts an existing directive. + The one correction made was to a *candidate*, not to the wiki (item 9: the + `memory.events` `max` counter is approaches-to-limit, not kills). +- **Related links added both ways**: `tests-that-cannot-fail` ↔ + `mutation-harness-file-custody`; `widening-a-closed-value-table` ↔ + `compiler-as-call-site-inventory` (+ `errors-diagnostics-from-a-shared-code-path`); + `host-cgroup-visibility` → `exec-added-processes-and-the-memory-budget` + (from its existing self-monitoring row). +- **Indexes/log updated**: 4 domain indexes (+5 "load when" rows), `log.md` + appended. Root `INDEX.md` unchanged — no new domain. + +Gates run (the exact CI commands from `.github/workflows/test.yml`): +`node scripts/wiki-structure-checks.js wiki` → **pages: 270, indexes: 13, +findings: 0**; `node scripts/wiki-lint-prohibitions.js wiki` → **directives 72, +compliant 72, violations 0** (the 1 `info` is pre-existing in +`config/keys-ahead-of-their-consumer.md`, untouched); `bash scripts/check-versions.sh` +→ `ok: dev-loop 1.11.2`. The `bats tests/` job was **not** run — bats is not +installed on this machine, and this change touches only wiki markdown (no +scripts or hooks), so that suite's subject is unchanged. ## Open-PR check -`gh pr list --state open` returned zero rows — no in-flight knowledge PR to -deduplicate against. All five units routed as new/amend without conflict. +`gh pr list --repo choiyounggi/dev-loop --state open --search "head:knowledge/"` +returned **no open PRs**, and a second unfiltered `gh pr list --state open` +returned none either — the repository has zero open PRs at flush time. There +were therefore no in-flight sibling branches to diff against, and no +`git fetch origin ` / `git diff origin/main origin/ -- wiki/` +comparisons to run. + +Per-candidate verdict: **all 21 = `new`.** No `fold`, no `drop-as-pending-duplicate`. +(The 2 drops recorded below are out-of-layer drops, not pending-duplicate drops.) ## Routing decision -- `qa/process/llm-review-pipelines` — NEW page. qa owns release-quality review - process; no existing category page carried the automated-LLM-review-pipeline - trigger. -- `backend/common/llm/progressive-disclosure-artifacts` — NEW page. backend/llm - owns agent/LLM-artifact authoring; `context-window-budget` covers output-cap - sizing only, a different trigger. -- `security/dependencies/agent-skill-supply-chain` — NEW page (not merged into - `supply-chain.md`): distinct trigger (executable agent skills vs npm/pip - packages), one-case-per-page. Cross-linked to `supply-chain` both ways. -- `security/agent-exposure/authorization-scope-persistence` — NEW page. - agent-exposure owns what an agent is permitted to do; distinct from - `in-session-tool-exposure` (WebMCP browser tools). Linked to it and to - `autonomous-decision-rulings`. -- `backend/common/llm/binding-instructions-for-agents` — AMEND (+Do-this #6, - +source). Same trigger, additive directive. - -Lint: `wiki-lint-prohibitions.js` → directives 71 / violations 0 (count -unchanged, no bats bump needed). `wiki-structure-checks.js` → 265 pages, 0 -findings. bats wiki-lint suite → 17/17 pass. +**New pages (5)** + +| Page | Domain/category | From | Why not an existing page | +|------|-----------------|------|--------------------------| +| `application-clock-vs-database-timestamps` | databases / transactions | `2b27d15d` + `bea92fdd` | No page compares an app clock to a DB column. `transactions` chosen over `schema-design` because the decisive content is transaction-time semantics (`now()` = transaction start ⇒ stamp order ≠ commit order) and the fix is a lock/isolation choice | +| `changed-files-only-gates` | infrastructure / ci-cd | `ff041061` + `4b9af3a0` | Zero grep hits. Both candidates are the same defect (a gate green with an empty subject) from two directions, so they became one page rather than two | +| `compiler-as-call-site-inventory` | backend / common / change-impact | `702dcf4e` + `94d55f2f` | `call-site-enumeration` is the sibling case (callers of a changed signature, Python positional-vs-keyword) and is at 80 body lines; the TS mechanism is *constructors of a type* with its own workflow, so per "one case per page" it is a separate page, cross-linked | +| `mutation-harness-file-custody` | testing / quality | `6a9de235` + `41fa1c87` | `harness-reverse-controls` covers scoring a harness; nothing covers the harness's custody of the tree. Both candidates are that one case (keying, and the read window) | +| `exec-added-processes-and-the-memory-budget` | infrastructure / containers | `7b9e8788` | `host-cgroup-visibility` is cross-pod read mechanics and explicitly routes self-monitoring elsewhere; `resource-limits-and-probes` is manifest authoring. This is a runtime preflight before adding load | + +No new category was created — all five landed in existing categories. + +**Merged into existing pages (5 candidates)** + +| Candidate | Merged into | Shape | +|-----------|-------------|-------| +| `91ef5d53` | `testing-quality-tests-that-cannot-fail` | +1 never-fails row, +1 Instead-of row, +1 source | +| `f189f423` | `testing-quality-source-text-wiring-assertions` | In-place extension of 1 edge row + 1 source bullet (page at the 120-line cap) | +| `bb6d8539` | `backend-common-change-impact-widening-a-closed-value-table` | +Do-this 6 & 7 (incl. a set-difference ruling table), +1 Instead-of row, +1 source | +| `60a817ee` | `qa-process-evaluating-review-feedback` | +2 edge rows, +1 Instead-of row, +1 source | +| `7b9e8788` | `infrastructure-containers-host-cgroup-visibility` | Cross-link from its self-monitoring row to the new page | + +**Dropped — out of layer (2, retired)** + +- `094dedf3` — a Figma MCP `inspect_node` → `get_dev_ready` children-fetch + workaround. The server is a private, org-internal MCP plugin; the behavior is + not publicly verifiable and the directive does not transfer to any other reader. +- `e165a365` — an `/rtb:review` remote-fallback runbook naming + `~/.claude/tools/rtb-remote-review.sh` and an internal pod. The transferable + kernel ("a two-provider review gate degraded to one provider is not a passed + gate") is already the subject of `qa-process-llm-review-pipelines`; what remains + is machine-specific paths. + +**Released back to `pending` (6)** — each needs its own page, not a row, and is +better served by a dedicated pass than by being appended here: +`81dc1f98` (naming the carrier field/type when a plan says "wire A to B"), +`b9ae304a` (`VAR="$(cmd 2>&1)"` mixing stderr into a value used as a path), +`fdd0b3c6` (monitor markers anchored at line start; delta rather than absolute +state; first cycle records a baseline), +`c2adb2be` (positional-order assertions on rendered SQL predicates), +`815e8cb9` (grep only *active* `DATABASE_URL` assignments, and confirm which +dotenv file the tool loads, before a destructive DB command), +`f1146adb` (CI ticket-key extraction scoped by changed-file intersection rather +than by mention). + +## Decision Log + +**Intent.** Drain the harvested `★ Insight` queue into reviewable wiki knowledge +without lowering the wiki's evidence bar. The queue held 21 rows accumulated over +several days; the goal was correct routing and real verification, not a high +ingest count. + +**Alternatives considered and rejected.** + +- *Ingest all 21 in this pass.* Rejected: six of them each need their own page, + and writing six more pages in one pass would have produced thin, weakly-sourced + entries. They are released to `pending`, not dropped, so the next flush takes + them with a full budget. +- *Append the two TypeScript candidates to `call-site-enumeration`.* Rejected: + that page is the sibling case (callers of a changed signature, Python + positional-vs-keyword). AGENTS.md requires one case per page, so the + constructor-enumeration case became its own page, cross-linked both ways. +- *Add a row to `source-text-wiring-assertions` for the empty-slice nuance.* + Rejected: that page is at exactly the documented 120-line body cap, so adding a + line would violate maintenance invariant 5. The nuance was merged **in place** + into an existing edge row instead; body re-measured at 120. +- *Drop the comment-stripping candidate entirely as a duplicate.* Rejected: its + directive is already the page's step 2, but the empty-slice consequence + (vacuous **green**, not the documented noisy red) was genuinely absent. +- *Claim a cross-check exemption because this PR cannot merge itself.* Rejected — + see below; the check found a real error, which is the argument against exempting. +- *Push to `origin`* as the skill's snippet does. Not available: this contributor + has no write access to `choiyounggi/dev-loop` (403). Used the pre-existing + `fork` remote, which is how every prior knowledge branch here was published. +- *Branch name from `git config user.name`.* The skill's ASCII sanitisation of a + Korean name yields an empty string → `anon`, defeating the attribution the + branch name exists for. Used the gh login, matching existing branch names. + +**Where reviewers should look hardest.** + +1. `infrastructure/ci-cd/changed-files-only-gates.md` — rewritten after the + cross-check. The measured table is the load-bearing part; please sanity-check + it against your own Prettier version, since the exit codes are version-visible + behaviour rather than a documented contract. +2. `databases/transactions/application-clock-vs-database-timestamps.md` step 5–6 — + the claim that timestamp order is not commit order, and that the remedy is a + lock/isolation level rather than finer clock resolution. `[추정]` on the MySQL + `NOW()`/`SYSDATE()` row: taken from general MySQL semantics, not fetched this + session like the PostgreSQL pages were. +3. `widening-a-closed-value-table.md` Do-this 6–7 — this inserts a security-shaped + concern (allowlist widening) into a page whose original subject was value + tables. If that reads as two cases, it should be split. +4. The 2 dropped candidates — if you consider private-tooling runbooks in scope + for this wiki, they should be restored rather than retired. + +## Cross-Check + +Independent adversarial pass via `claude` CLI headless (separate process, no +shared context), prompted to refute rather than confirm, over the five new pages' +technical claims. + +**It found a real error, and the page was rewritten because of it.** The reviewer +challenged the claim that `prettier --check` exits 0 on an empty match set, +arguing an unmatched pattern errors by default and that exit-0 belongs to the +ignore-filtered case. I resolved it by measurement rather than by argument — +running all seven cases against Prettier 3.7.4 — and the reviewer was right: +an unmatched operand exits **2** (while still printing the success sentence), +whereas the genuine silent vacuous passes are no-operands, all-ignore-filtered, +and `--ignore-unknown`-with-unsupported-extensions. The page, this report's +rows 3 and 5, and the `log.md` entry were all corrected. + +Verdicts on the other five claim groups: **sound** (PostgreSQL clock semantics — +noted as if anything *understated*; zsh word-splitting; TS contextual typing; +`tsc` program membership incl. `exclude`-does-not-stop-imports; cgroup v2 +`max` vs `oom_kill` and `kubectl exec` cgroup placement). + +Stated limits of the check: the reviewer's sandbox denied it read access to +`~/.dev-loop/repo/wiki`, so it adjudicated the six claims as quoted in its prompt +and could **not** audit (b) whether each `Sources` quote supports the directive it +is cited for, or (c) whether any page contradicts its own edge-case rows. Those +two dimensions remain unreviewed by an independent party and are the residual +risk in this PR. A first attempt also returned only the session's Stop-hook +output rather than a verdict; that run was discarded rather than read as +"no findings". + +## Review notes + +- PR-only, as required: no merge, no push to `main`. +- Commit is under the contributor's own ambient git identity + (`최영기 `, gh `dch0202-rsquare`); no assistant identity + and no `Co-Authored-By` trailer. The branch uses the gh login because + sanitizing the Korean `user.name` to ASCII yields an empty string, which the + skill's snippet would have turned into `anon` — that would have defeated the + attribution the branch name exists for. +- Scope purity: only `wiki/**`, four domain indexes, `log.md`, and this report. + Two untracked leftovers from earlier flushes + (`.dev-loop/CROSSCHECK_FINDINGS.md`, `.dev-loop/fold-note-73.md`) were left + untouched and unstaged. diff --git a/log.md b/log.md index 0ed80d8..5022a73 100644 --- a/log.md +++ b/log.md @@ -99,3 +99,4 @@ Append-only. Format: `## [YYYY-MM-DD] were found already covered by responsive-layout and this page). Ingested only the two decision-relevant gaps, sourced from MDN (both live-fetched this session): Do-this #4 extended with `popover` + `popovertarget` for non-modal surfaces (top layer, light-dismiss, Escape for free; never modal, no focus trap — modal stays +showModal; Baseline newly available 2025-01), +1 edge-case row (overlay that cannot use → `inert` on background containers, subtree drops from focus/click/a11y tree; Baseline widely available 2023-04), +1 Instead-of row (hand-rolled dropdown with outside-click listeners → popover="auto"). frontend/index.md load-when extended (overlay/background-disable triggers). last_verified → 2026-08-24; body 87 lines. ## [2026-08-24] ingest | GitHub-trending agent-skill practices (4 new pages, 1 amended page, 3 domain indexes updated). Distilled from six trending repos analyzed this session; each directive cited to a live-verified source file (all 9 cited repo paths confirmed via `gh api` before writing). New: qa/process/llm-review-pipelines (alibaba/open-code-review — deterministic file-selection/bundling/glob-rule-matching before the model, rules as matchable rule.json data not prompt prose, related-file bundles as isolated concurrent sub-reviews, deterministic re-location of model-emitted line anchors, precision-over-recall measured on a labeled PR set; the ~1/9-token claim flagged as vendor self-measured on AACR-Bench, not independently reproduced), backend/common/llm/progressive-disclosure-artifacts (book-to-skill — always-loaded core index + on-demand ~1K-token chunks, per-chunk budget from a content-type × usage-depth matrix, auxiliary views as separate files, grep/sed slice access for ≥50K-token originals; copyright edge case = synthesize-not-reproduce + keep private), security/dependencies/agent-skill-supply-chain (reverse-skill skill-supply-chain checklist + openwork skills-lock.json + book-to-skill's documented malicious re-upload: skill = arbitrary code, read SKILL.md+scripts and grep exfil patterns, resolve canonical owner vs lookalike, pin by source+hash, vet MCP auto-registration; publication-time gating for team marketplaces), security/agent-exposure/authorization-scope-persistence (reverse-skill scope-contract — persist authorization to a per-target file with a status field, default-deny on status != granted, --force cannot bypass the auth check, network_profile as its own field, re-read the gate at each dangerous step). Amended: backend/common/llm/binding-instructions-for-agents +Do-this #6 (pre-send self-check predicates for output-shape rules) + i-have-adhd source. Deliberately NOT ingested: block/buzz signed hash-chain audit log (platform-construction knowledge, off the wiki's usage context), openwork search+execute 2-tool gateway (README-only, in tension with agent-tool-granularity's primitives guidance, below the directive bar), airllm/airi/pascalorg-editor/AI-For-Beginners (out of domain or unverified at code level). Correction to the source podcast's claim: open-code-review's token figure is ~1/9, not "19%". Reciprocal related links added both ways (supply-chain, in-session-tool-exposure, evaluating-review-feedback, context-window-budget, agent-tool-granularity via binding-instructions, autonomous-decision-rulings). ## [2026-08-25] revise | pane-delivery-confirmation: fixed last-N window is defeated by a paste's own rendered tail — anchor the placeholder scan on the input box (#145) +## [2026-08-27] ingest | knowledge-flush of the harvested insight queue — 5 new pages, 5 amended pages, 4 domain indexes updated; 13 of 21 claimed candidates ingested, 2 dropped, 6 released for a later flush. New: databases/transactions/application-clock-vs-database-timestamps (app-produced `Date` compared against a `default now()` column — take the boundary from the DB via `RETURNING`, produce it once per decision and propagate it into the follow-up step rather than recomputing (a second `now` widens a `<= boundary` cleanup set), pick the safe error direction before choosing a margin, and separate "same clock" from "correct ordering" since `now()` is `transaction_timestamp()` so a later-committing transaction can carry an earlier stamp — close that window with a lock or SERIALIZABLE, not finer resolution; PostgreSQL functions-datetime + dml-returning live-fetched and quoted), infrastructure/ci-cd/changed-files-only-gates (count-and-branch-on-empty before invoking the tool, log the passed list not just its length, pass operands as an array/xargs because zsh does not word-split by default, prove the gate can fail with a probe placed on a non-ignored path; Prettier CLI + ignore docs and zsh FAQ live-fetched, then all seven cases measured locally against Prettier 3.7.4 after an independent cross-check refuted the first draft: the success sentence is NOT a pass signal — it is printed both for an ignore-filtered list at rc 0 and alongside an unmatched-pattern error at rc 2. The genuine silent vacuous passes are no-operands (rc 0), all-operands-ignore-filtered (rc 0), and unsupported-extensions-with-`--ignore-unknown` (rc 0), while an unmatched or word-split operand exits 2 unless `--no-error-on-unmatched-pattern` is set; the page carries the measured table), backend/common/change-impact/compiler-as-call-site-inventory (contextually typed object literals build a type without naming it, so a type-name grep and the truth are different sets — take the inventory from the compiler error list and bound it to the program, since a package whose `include` omits its own tests contributes zero errors from files that still build the value; TS handbook objects + tsconfig include/exclude live-fetched, incl. the documented fact that `exclude` does not stop transitive imports), testing/quality/mutation-harness-file-custody (key backups by flattened relative path not basename, byte-compare each restore before the next mutation, confirm restores by content because an untracked file's `git diff` is empty either way, and read the tree only after process-exit AND empty-backup-dir), infrastructure/containers/exec-added-processes-and-the-memory-budget (an exec'd process joins the container's cgroup but is invisible to the app's own semaphore — decide from `memory.max − memory.peak` headroom, and read `memory.events: max` as approaches-to-limit distinct from `oom_kill`; kernel cgroup-v2 admin guide live-fetched and quoted). Amended: testing/quality/tests-that-cannot-fail +1 never-fails row and +1 Instead-of row (a negative assertion whose fixture never supplies the triggering input is vacuous — the defect is in the fixture, so strengthening the expectation changes nothing; measured 116/116 green under the widening it claimed to catch), testing/quality/source-text-wiring-assertions (page sits at the 120-line cap, so the new nuance was merged in place into the existing negative/count edge row rather than added as a line — an `indexOf(marker)` slice collapses to `''` when the marker also appears in a comment above the code, making every assertion inside it vacuously green), backend/common/change-impact/widening-a-closed-value-table +Do-this #6/#7 and +1 Instead-of row (unifying two duplicate allowlists defaults to the union and silently widens each side — compute both set differences and rule on each element first, keep derived-with-explicit-difference when an element must legitimately differ, and record whether the merged set widened or narrowed), qa/process/evaluating-review-feedback +2 edge rows and +1 Instead-of row (a body-level finding cites no lines, so step 2 has nothing to open — grep the quoted string across the changed set before ruling, since the usual shape is that one sibling was fixed and another still carries the defect), infrastructure/containers/host-cgroup-visibility (self-monitoring row cross-linked to the new exec-preflight page). Dropped as not generalizable to this wiki: a Figma-MCP `inspect_node`→`get_dev_ready` children-fetch workaround (private MCP server, not publicly verifiable) and an `/rtb:review` remote-fallback runbook (hardcoded private tooling paths; the transferable part — a two-provider review gate degraded to one provider is not a passed gate — is already carried by qa/process/llm-review-pipelines). Released back to the queue for a later flush, each needing its own page rather than a row: plan-level carrier/seam field naming, `VAR="$(cmd 2>&1)"` stderr contamination in shell value extraction, tmux/CI monitor delta-vs-absolute anchoring, positional-order assertions on rendered SQL predicates, `.env` active-assignment preflight before a destructive DB command, and CI ticket-key extraction scoped by changed-file intersection. diff --git a/tests/wiki-lint-prohibitions.bats b/tests/wiki-lint-prohibitions.bats index d39b4be..844193c 100644 --- a/tests/wiki-lint-prohibitions.bats +++ b/tests/wiki-lint-prohibitions.bats @@ -18,11 +18,11 @@ setup() { # --- normal: the real corpus is already compliant --------------------------- -@test "real wiki: exits 0 with 0 violations and 71 directive units" { +@test "real wiki: exits 0 with 0 violations and 72 directive units" { cd "$REPO_ROOT" || return 1 run node "$CHECKER" wiki [ "$status" -eq 0 ] - [[ "$output" == *"directives: 71"* ]] + [[ "$output" == *"directives: 72"* ]] [[ "$output" == *"violations: 0"* ]] } diff --git a/wiki/backend/common/change-impact/compiler-as-call-site-inventory.md b/wiki/backend/common/change-impact/compiler-as-call-site-inventory.md new file mode 100644 index 0000000..89d5911 --- /dev/null +++ b/wiki/backend/common/change-impact/compiler-as-call-site-inventory.md @@ -0,0 +1,105 @@ +--- +id: backend-common-change-impact-compiler-as-call-site-inventory +domain: backend +category: change-impact +applies_to: [typescript] +confidence: verified +sources: + - https://www.typescriptlang.org/docs/handbook/2/objects.html + - https://www.typescriptlang.org/tsconfig/#include + - https://www.typescriptlang.org/tsconfig/#exclude + - https://www.typescriptlang.org/docs/handbook/type-checking-javascript-files.html +last_verified: 2026-08-27 +related: + [ + backend-common-change-impact-call-site-enumeration, + backend-common-change-impact-cross-module-consumer-census, + backend-common-integrations-consumer-required-fields, + testing-quality-tests-that-cannot-fail, + qa-process-completion-claims, + ] +--- + +# Using the Type Checker as the Inventory of Sites That Build a Value + +## When this applies + +You are adding a required field to a type and relying on "make it required and +the compiler lists every site that must be updated" as the complete inventory — +or you enumerated those sites by grepping the **type's name** and are about to +publish the count in a plan, brief, or task breakdown. + +Enumerating callers of a function whose signature changes → +[backend-common-change-impact-call-site-enumeration]. + +## Do this + +1. **Enumerate construction sites from the compiler's error list, and use a + grep only to cross-check it.** TypeScript types object literals from + context, so a value of the type can be built with the type's name nowhere in + the text — `dealViewer: { userId: ctx.user.userId }` constructs a `DealViewer` + and contains no occurrence of `DealViewer`. A type-name grep counts the sites + that *mention* the type; the sites that *build* it are a different set. + +2. **Bound the claim to the files the compiler actually reads, and check that + boundary before quoting the count.** The program is `files` ∪ `include` ∪ + everything reachable by import from them. A package whose `tsconfig.json` + says `include: ["src/**/*"]` never type-checks its own `__tests__/` + directory unless `src` imports it, so fixtures and expected-value literals + there contribute **zero** errors and are missing from the inventory. + +3. **Run the sweep per package in a monorepo and add the results up.** Each + package carries its own `tsconfig.json`, so `include` differs between them; + one root type-check reports only what the root project references. + +4. **Cross-check with a grep on the *property name*, not the type name.** + `grep -rn 'fieldName:'` reaches contextually-typed literals because the + property is the text that is actually present. Reconcile the two lists and + explain each difference: + +| The site appears in | Read it as | +|---------------------|------------| +| Compiler errors and the property grep | Confirmed construction site | +| Compiler errors only | The literal spells the property differently (spread, computed key, helper) — read it and record the form | +| Property grep only | Outside the compiler's program — check that package's `include`, then treat it as a site the migration must handle by hand | +| Neither, but the value flows there at runtime | Built by a factory or spread from another object — enumerate that producer instead ([backend-common-integrations-consumer-required-fields]) | + +5. **State the method next to the count.** "8 construction sites (tsc error + list, `packages/*` each, cross-checked with `grep 'dealViewer:'`)" is + checkable; a bare "8 sites" cannot be reviewed for the gaps above, and a + count published into a brief is inherited by everyone working from it. + +6. **Re-run the type-check after wiring and require zero errors, then run the + tests.** Files outside the program fail only at runtime, so the test run is + the second half of the inventory, not a formality. + +## Edge cases + +| Case | Then | +|------|------| +| The new field is optional | The compiler reports nothing at all — every site keeps compiling with the field absent. Make it required for the sweep, collect the list, then relax it if the design calls for optional ([testing-quality-tests-that-cannot-fail]) | +| A test directory is excluded from the package's `tsconfig` | Its sites surface as failing tests after wiring, or pass silently if nothing asserts the field — enumerate it by property grep and fix it in the same change | +| `exclude` lists the directory | `exclude` "only changes which files are included as a result of the `include` setting" — an excluded file still enters the program when an included file imports it, so the boundary is "in the program", not "in `include`" | +| The literal is built with a spread (`{ ...base, userId }`) | The excess/missing check applies to the spread result, but the property grep misses it — grep the base object's factory as the producer | +| The value is cast (`as DealViewer`) or typed `any` | The assertion suppresses the error, so the site is absent from the inventory while being a real construction site — grep the type name **as well**, which is where a type-name search does pay | +| The repo type-checks with `skipLibCheck` or has pre-existing errors | The new errors are not separable by eye — capture the error list before and after and diff them | +| Sites live in another repository or a published package | The compiler cannot see them at all; make the field optional at the boundary and version the change ([backend-common-change-impact-call-site-enumeration]) | +| The project uses `checkJs: false` with JavaScript callers | `.js` construction sites are unchecked — enumerate them by property grep only | + +## Instead of + +| If you are about to | Do this instead | Why | +|---------------------|-----------------|-----| +| Report the number of construction sites from `grep ""` | Take the list from the compiler errors, cross-check with a property grep, and state the method | Contextually typed object literals build the type without naming it, so the grep and the truth are different sets — measured 3 vs 8 | +| Treat a clean `pnpm type-check` as proof every assembly site was updated | Confirm each package's `include` covers its tests, then run the tests too | A package excluding `__tests__/` contributes zero errors from files that still build the value | +| Add the field as optional to avoid breaking the build, then find the sites later | Make it required, collect the compiler's list, and decide optionality afterwards | An optional field produces no inventory at all; "later" has no signal to work from | +| Publish the count into worker briefs as soon as the grep returns | Publish the count with its method, after the compiler sweep | A wrong count in a brief is multiplied by the number of briefs, and each worker reads it as scope | + +## Sources + +- https://www.typescriptlang.org/docs/handbook/2/objects.html — excess property checking applies to fresh object literals assigned to a typed target; the checking follows from the **contextual type**, so it occurs on literals in which the type's name never appears (the basis for step 1) +- https://www.typescriptlang.org/tsconfig/#include — `include` "specifies an array of filenames or patterns to include in the program", defaulting to `**/*` when neither `files` nor `include` is set; the program is what `tsc` reads and therefore the limit of the inventory (step 2) +- https://www.typescriptlang.org/tsconfig/#exclude — "`exclude` *only* changes which files are included as a result of the `include` setting. A file specified by `exclude` can still become part of your codebase due to an `import` statement in your code" — the basis for the `exclude` edge-case row +- https://www.typescriptlang.org/docs/handbook/type-checking-javascript-files.html — `checkJs` governs whether `.js` files are checked, behind the JavaScript-callers row +- Field measurement 2026-08-24 (`rtb-unified`): a plan recorded "3 assembly sites" for `DealViewer` from `grep "DealViewer"`; the real count was 8, and the missing set included the production wiring `packages/orpc/src/routers/deal.ts:38`, `dealViewer: { userId: context.user.userId }`. The same error repeated for `ContractScopeActor`: 7 sites across 4 files by type-name grep versus roughly 22 across 11 files +- Field measurement 2026-08-25 (same repo): `packages/orpc/tsconfig.json` declares `include: ["src/**/*"]`. Making `DealViewer.firstTierScope` required produced 3 production and 13 api-test errors from `pnpm type-check`, and **zero** for `packages/orpc/__tests__/routers/deal.test.ts` — that file's 6 assembly and expectation sites appeared only as 6 failing tests once the router was wired diff --git a/wiki/backend/common/change-impact/widening-a-closed-value-table.md b/wiki/backend/common/change-impact/widening-a-closed-value-table.md index baaf2b4..3fafe18 100644 --- a/wiki/backend/common/change-impact/widening-a-closed-value-table.md +++ b/wiki/backend/common/change-impact/widening-a-closed-value-table.md @@ -7,8 +7,8 @@ confidence: verified sources: - https://refactoring.com/catalog/replaceMagicLiteral.html - https://pragprog.com/tips/ -last_verified: 2026-08-06 -related: [backend-common-change-impact-call-site-enumeration, backend-common-api-design-unenforced-declarations] +last_verified: 2026-08-27 +related: [backend-common-change-impact-call-site-enumeration, backend-common-api-design-unenforced-declarations, backend-common-errors-diagnostics-from-a-shared-code-path, backend-common-change-impact-compiler-as-call-site-inventory] --- # Widening a Closed Value Table Whose Consumers Inlined It @@ -50,6 +50,24 @@ rather than a parse error. 5. **Add a test that drives every consumer with every member of the table**, parameterized over the table itself. It fails on the next widening if a new copy has appeared, which is the only check that survives the next author. +6. **When the plan is to unify two duplicate tables into one, compute both set + differences first and rule on each element before writing the merge.** "Both + consumers want the same set" is a claim, and unification defaults to the + union — which widens each side by whatever the other carried. For an + allowlist that widening *is* the change: a member the other list happened to + include becomes newly permitted or newly exposed, and neither type-checking + nor the existing tests read set membership as a fact worth failing on. + +| A − B / B − A contains | Do | +|------------------------|----| +| Nothing (the sets are equal) | Unify; record that the difference was measured and empty | +| Elements that are an oversight in one list | Unify to the union, and name each added element in the change description | +| Elements that must legitimately differ per consumer | Keep two names derived from one base (`BASE`, `BASE + EXTRA`), so the difference stays visible instead of being erased | +| Elements you cannot classify | Leave the duplication in place until each is ruled on — an unexplained difference is the case the union silently resolves | + +7. **Record the direction after merging**: diff the resulting set against each + original and state whether it widened, narrowed, or held. A widening of an + allowlist is a review item on its own, separate from the deduplication. ## Edge cases @@ -71,9 +89,11 @@ rather than a parse error. | Add the entry to the canonical table and run the suite | Reconcile the copies first, then widen | A green suite means the consumers the tests reach accepted the entry; the copied ones were never asked | | Leave one inlined copy because it is a hot path or avoids an import cycle | Have that path read the table once at import and keep the local binding | The copy is not cheaper than a module-level lookup, and it is the site that silently defines a different vocabulary | | Extend a second same-vocabulary table alongside the first to keep both callers happy | Derive the second from the first in the same module | Two canonical-looking tables make the next author's name grep authoritative and wrong | +| Replace two duplicate allowlists with one union because they "look the same" | Compute both set differences, rule on each element, then unify or derive | The union grants every consumer the other's extra members; for an allowlist that is a new exposure, and no type or test reports it | ## Sources - https://refactoring.com/catalog/replaceMagicLiteral.html — *Replace Magic Literal*, alias "Replace Magic Number with Symbolic Constant": a literal with a particular meaning becomes a named constant. The refactoring exists because the inlined literal is the default state of such values, which is what makes the value the reliable search handle - https://pragprog.com/tips/ — Tip 15, DRY: "Every piece of knowledge must have a single, unambiguous, authoritative representation within a system." A copied value table is a second representation, and widening one representation is what produces the divergence +- Field measurement 2026-08-25 (`rtb-unified`, PR #965): a cleanup proposed folding `DISPLAYABLE_ERROR_CODES` into `USER_FACING_ERROR_CODES` as one SSOT. Computing the set difference before merging showed the union added `UNAUTHORIZED` and `VALIDATION_ERROR` to the displayable set — the path by which raw server messages would have been surfaced as inline UI errors. The sets were kept derived-with-an-explicit-difference instead; nothing in the type system or the suite had flagged the widening - Local reproduction 2026-08-06 (`linkly`, Python, macOS): `grep -rn "DURATION_UNITS" impl/lnpl/*.py` returns **1** hit (the definition); `grep -rn "60000" impl/lnpl/*.py` returns **7** across four files — a second named table (`DURATION_UNIT_MS`, `lexer.py:23`), three independently inlined `(("ms",1),("s",1000),("m",60000))` tuples (`condition.py:353`, `interp.py:1020`, `backend.py:446`), and two bare-literal arithmetic sites (`condition.py:269-270`). The predicted divergence was already present: the canonical map carries `h` and `d`, while all three inlined copies stop at `m`, so those paths cannot convert a unit the lexer accepts diff --git a/wiki/backend/index.md b/wiki/backend/index.md index 22afe1f..7f86f69 100644 --- a/wiki/backend/index.md +++ b/wiki/backend/index.md @@ -38,6 +38,7 @@ Match your situation to a "load when" line; load only matching pages. | [corpus-sweep-before-a-rejection-rule](common/change-impact/corpus-sweep-before-a-rejection-rule.md) | Adding a rule to a compiler/linter/parser/schema validator/repo gate that will start rejecting input the tool accepted silently, and the existing corpus must keep passing; producing the evidence a plan needs before writing the rule (reject count + rejected-path list, enumeration method stated); such a rule landed and went red on inputs nobody had called defective; deciding between narrowing the rule, an opt-in strictness level, and an exemption (whether unimplemented declarative input should reject/warn/ignore at all → common/api-design/unenforced-declarations) | | [aggregation-layer-of-a-shared-helper](common/change-impact/aggregation-layer-of-a-shared-helper.md) | A plan, brief, or review comment says to unify or replace "the N call sites" of a helper and names them by line number rather than by what the code does; one site feeds a set-level SQL aggregate while another returns a per-row value that application code reduces later; a "shared helper" landed with one consumer adopting it and the other keeping its old semantics; deciding which layer owns a missing-value rule and whether the plan's grep-count acceptance criterion is reachable at all | | [call-site-enumeration](common/change-impact/call-site-enumeration.md) | Changing the contract of a function/method/constructor other code calls — adding, removing, reordering or redefining a parameter — and you need the complete call-site list; scoping such a migration from a search; a migration scoped from recon came back green and then failed on call sites the search never listed; deciding whether to append a parameter or make it keyword-only (release-level re-test scope → qa/process/regression-scope) | +| [compiler-as-call-site-inventory](common/change-impact/compiler-as-call-site-inventory.md) | Adding a required field to a TypeScript type and relying on the compiler error list as the complete inventory of sites that build it; enumerating those sites by grepping the type name; publishing that count into a plan or worker brief; a monorepo package whose `tsconfig.json` `include` omits its own tests | | [cross-module-consumer-census](common/change-impact/cross-module-consumer-census.md) | Your task added a public function, endpoint, export, or hook whose consumer belongs to a *different* task (parallel work split by file ownership, a backend change whose UI wiring is another ticket); deciding whether that task is done; a feature typed, tested, reviewed and merged changes nothing at runtime; separating same-module helpers and entry points from genuine orphans in a zero-consumer list | | [inserting-a-guard-before-an-existing-side-effect](common/change-impact/inserting-a-guard-before-an-existing-side-effect.md) | Implementing, adopting, or auditing a planned change that adds a precondition guard to an existing script/function where the plan names the insertion point in prose ("after X is built"); the target has an earlier unconditional default/auto-create side effect touching the state the guard checks; writing the test for such a guard's refusal path | diff --git a/wiki/databases/index.md b/wiki/databases/index.md index ff6d62e..cd537af 100644 --- a/wiki/databases/index.md +++ b/wiki/databases/index.md @@ -72,3 +72,4 @@ Match your situation to a "load when" line; load only matching pages. |------|-----------| | [isolation-level-selection](transactions/isolation-level-selection.md) | Check-then-act writes, lost updates, duplicate bookings, choosing isolation/locking; deadlock-detected errors; oversell despite @Transactional | | [optimistic-vs-pessimistic-locking](transactions/optimistic-vs-pessimistic-locking.md) | Multi-step read-modify-write that cannot fold into one UPDATE — choosing version-column optimistic vs FOR UPDATE by conflict frequency; stale form submits; retry storms on hot rows | +| [application-clock-vs-database-timestamps](transactions/application-clock-vs-database-timestamps.md) | A predicate compares an application-produced timestamp against a DB-filled column (`created_on > $boundary`, "older than N minutes"); one step classifies rows by a time boundary and a later step cancels/deletes what it chose; choosing between `RETURNING` the DB clock and adding a skew margin; deciding whether timestamp order can stand in for commit order | diff --git a/wiki/databases/transactions/application-clock-vs-database-timestamps.md b/wiki/databases/transactions/application-clock-vs-database-timestamps.md new file mode 100644 index 0000000..6d42b12 --- /dev/null +++ b/wiki/databases/transactions/application-clock-vs-database-timestamps.md @@ -0,0 +1,108 @@ +--- +id: databases-transactions-application-clock-vs-database-timestamps +domain: databases +category: transactions +applies_to: [postgresql, mysql] +confidence: verified +sources: + - https://www.postgresql.org/docs/current/functions-datetime.html + - https://www.postgresql.org/docs/current/dml-returning.html + - https://www.postgresql.org/docs/current/transaction-iso.html +last_verified: 2026-08-27 +related: + [ + databases-transactions-isolation-level-selection, + databases-data-survey-audit-columns-as-update-evidence, + backend-common-jobs-scheduled-job-overlap, + backend-common-change-impact-inserting-a-guard-before-an-existing-side-effect, + testing-quality-injected-clock-duration-assertions, + ] +--- + +# Comparing an Application Clock Against a Database Timestamp Column + +## When this applies + +A predicate compares a value your application produced (`new Date()`, +`Instant.now()`) against a timestamp column the database filled itself — +`created_on > $boundary`, `updated_at < $cutoff`, "rows older than N minutes". +Also when one step classifies rows by such a boundary and a **later** step +cancels, deletes, or cleans up the rows that classification chose. + +## Do this + +1. **Take the boundary from the database's own clock, in the statement that + already touches those rows, using `RETURNING`.** The docs give this as + `RETURNING`'s purpose — it is "very handy when relying on computed default + values", returning them "without needing a separate database query". A column + declared `default now()` is filled by the server; reading the same server's + clock is what makes the two sides comparable. + +2. **Produce the boundary once per decision and pass that value forward.** + Recomputing the same formula in the follow-up step creates a *second* `now`, + later than the first. For a `created_on <= boundary` shape the target set + silently **widens**, so a row the first step classified as fresh — and meant + to protect — falls inside the second step's cleanup set. Make the propagation + structural: have the boundary helper take `now` as a parameter, and put the + boundary in the classification's **return type** so the next step receives it + instead of re-deriving it. + +3. **When no database value is available to read, decide which direction of + error is safe before choosing a margin**, and record the asymmetry: + +| Clock relationship | Effect on a "rows newer than boundary" predicate | Read it as | +|--------------------|--------------------------------------------------|------------| +| App clock ahead of DB | Boundary sits in the DB's future — rows written in the gap are missed | Under-count: the dangerous direction for cancel/cleanup guards | +| App clock behind DB | Boundary sits in the DB's past — extra rows are included | Over-count: safe for a guard, wasteful for a batch | +| Unknown / unbounded skew | No margin is provable | Read the DB clock (step 1) or widen to the safe direction and say so | + +4. **State the margin's assumption in the code.** A margin encodes "skew is + smaller than this", which nothing in the query verifies — write that sentence + next to the constant so the next reader knows what would invalidate it. + +5. **Separate "same clock" from "correct ordering".** In PostgreSQL `now()` is + `transaction_timestamp()`, the **start** time of the transaction: the docs + state `statement_timestamp()` and `transaction_timestamp()` "return the same + value during the first statement of a transaction, but might differ during + subsequent statements", while `clock_timestamp()` "returns the actual current + time, and therefore its value changes even within a single SQL statement". A + long transaction that started earlier and committed later therefore stamps an + **earlier** value than a short one that committed before it — so timestamp + order is not commit order, and a reader can observe a row whose stamp + precedes a boundary it was not visible for. + +6. **When the decision must not miss a concurrently committed row, close the + window with a lock or isolation level rather than a finer timestamp.** + Take an advisory lock around classify-then-act, or run it `SERIALIZABLE` + ([databases-transactions-isolation-level-selection]); timestamp precision + does not create the mutual exclusion the correctness argument needs. + +## Edge cases + +| Case | Then | +|------|------| +| The column's default is `now()` and the app also writes the field on some paths | The table now holds two clocks in one column — pick one writer, and backfill or document the mixed range before using the column as a boundary ([databases-data-survey-audit-columns-as-update-evidence]) | +| The statement inserts nothing to `RETURNING` from (a pure read) | Select the clock explicitly (`SELECT now()`) in the same transaction and pass it down, so every predicate in the decision shares one value | +| Rows are stamped by several application hosts | Their clocks differ from each other as well as from the DB; a server-side default is the only single clock available | +| The boundary crosses a DST change or the column is `timestamp without time zone` | Compare in UTC/`timestamptz` end to end — a local-time column makes the comparison ambiguous for one hour a year ([platforms-environment-timezone-and-locale]) | +| The follow-up step runs in a different process or job | The boundary has to travel in the payload; a job that recomputes `now` on pickup reintroduces the widening from step 2 | +| MySQL rather than PostgreSQL | `NOW()` is likewise fixed for the statement while `SYSDATE()` reads the live clock — the same one-clock rule applies, with those two names | +| The test asserts elapsed time between two readings of the clock | Inject the clock so the assertion is deterministic ([testing-quality-injected-clock-duration-assertions]) | + +## Instead of + +| If you are about to | Do this instead | Why | +|---------------------|-----------------|-----| +| Pass `new Date()` as the boundary against a `default now()` column | Read the boundary from the DB (`RETURNING`, or `SELECT now()` in the same transaction) | The column is filled by the server's clock; comparing it to the app's makes skew an unmeasured term in the predicate | +| Recompute the same boundary formula in the cleanup step | Return the boundary from the classification and pass it in | The second `now` is later, so `<= boundary` widens and rows classified as protected become eligible | +| Add a fixed margin (`now - 5s`) and move on | Pick the direction the error must fall in, then set the margin, and write the skew assumption beside it | A margin without a stated direction is as likely to widen the dangerous side as the safe one | +| Treat equal timestamps as equal commit order | Order by a sequence/identity column, or serialize the decision with a lock | `now()` is transaction-start time, so a later-committing transaction can carry an earlier stamp | +| Tighten to `clock_timestamp()` to fix a missed row | Close the window with an advisory lock or `SERIALIZABLE` | The gap is visibility between two statements, not resolution — a finer clock narrows the race without removing it | + +## Sources + +- https://www.postgresql.org/docs/current/functions-datetime.html — "`transaction_timestamp()` is equivalent to `CURRENT_TIMESTAMP`"; "`statement_timestamp()` and `transaction_timestamp()` return the same value during the first statement of a transaction, but might differ during subsequent statements. `clock_timestamp()` returns the actual current time, and therefore its value changes even within a single SQL statement" — the basis for steps 5 and the MySQL row's analogue +- https://www.postgresql.org/docs/current/dml-returning.html — `RETURNING` obtains data from modified rows "without needing a separate database query", and is "very handy when relying on computed default values"; this is the mechanism in step 1 +- https://www.postgresql.org/docs/current/transaction-iso.html — the isolation levels behind step 6's alternative to timestamp precision +- Field measurement 2026-08-26 (pg-boss 10.4.2): `src/plans.js` declares `created_on timestamp with time zone not null default now()` — the column is server-stamped, so an application-generated boundary compares two clocks. A mutation replacing the propagated boundary with a fresh `new Date()` reddened one wiring assertion, and an independent reviewer raised the transaction-visibility limit separately, which is what step 5 records +- Field measurement 2026-08-24 (`rtb-unified`, `packages/orpc/src/routers/batch.ts`): the in-flight batch check states "one `now` per decision" as an invariant and enforces it by having the boundary helper take `now` as a parameter; the classification's own result type omitted the boundary, which is the shape that invites the step-2 recomputation in the follow-up step diff --git a/wiki/infrastructure/ci-cd/changed-files-only-gates.md b/wiki/infrastructure/ci-cd/changed-files-only-gates.md new file mode 100644 index 0000000..c0b8630 --- /dev/null +++ b/wiki/infrastructure/ci-cd/changed-files-only-gates.md @@ -0,0 +1,126 @@ +--- +id: infrastructure-ci-cd-changed-files-only-gates +domain: infrastructure +category: ci-cd +applies_to: [general] +confidence: verified +sources: + - https://prettier.io/docs/en/cli + - https://prettier.io/docs/en/ignore + - https://zsh.sourceforge.io/FAQ/zshfaq03.html + - https://www.gnu.org/software/bash/manual/bash.html + - https://eslint.org/docs/latest/use/command-line-interface +last_verified: 2026-08-27 +related: + [ + infrastructure-ci-cd-pipeline-structure, + testing-quality-tests-that-cannot-fail, + testing-quality-checks-that-cannot-pass, + testing-quality-harness-reverse-controls, + platforms-shells-portable-shell-scripts, + testing-quality-history-dependent-checks-on-shallow-clones, + ] +--- + +# A Lint or Format Gate Scoped to the Files a Change Touched + +## When this applies + +A CI step builds a list of changed files in the shell and passes it to a tool +that takes filenames as operands — `prettier --check`, `eslint`, a type checker, +a custom script. You are writing that step, or such a gate is green and you are +deciding whether "green" means "no violations" or "nothing was examined". + +## Do this + +1. **Count the list and branch on empty before calling the tool.** Emit the + count, and on zero emit an explicit skip line and exit success *without* + invoking the tool. Invoking it with no operands is the vacuous case that + exits **0** while checking nothing, and its message names a parser rather + than the empty list, so nothing in the log says "no subject". + +2. **Read the exit code, not the success sentence.** "All matched files use + Prettier code style!" is printed in cases that examined nothing, including + ones that exit non-zero — so the sentence alone never establishes a pass. + Measured (Prettier 3.7.4), the shapes split like this: + +| What the gate passed | Output | Exit | Examined | +|----------------------|--------|------|----------| +| No operands at all | `[error] No parser and no file path given, couldn't infer a parser.` | **0** | nothing — silent vacuous pass | +| Operands all ignore-filtered (`.prettierignore`/`.gitignore`) | `Checking formatting...` + success sentence | **0** | nothing — silent vacuous pass | +| Only unsupported extensions, with `--ignore-unknown` | `Checking formatting...` + success sentence | **0** | nothing — silent vacuous pass | +| A pattern/operand matching no file | `[error] No files matching the pattern were found: "…"` **and** the success sentence | **2** | nothing — loud, unless the error is suppressed | +| Only unsupported extensions, without `--ignore-unknown` | `[error] No parser could be inferred…` | **2** | nothing — loud | +| Real files, all clean | `Checking formatting...` + success sentence | **0** | yes | + +3. **Log the file list you passed, not just its length.** The three exit-0 rows + above are indistinguishable from a real pass in the log, and two of them + occur with a non-empty operand list — so a count alone does not separate + them. The list is what lets a reviewer tell which row happened. + +4. **Keep `--no-error-on-unmatched-pattern` off unless the empty case is + handled by step 1.** That flag turns the loud row into a silent one: the + unmatched-pattern error is the only signal distinguishing "operands named + nothing" from "operands were clean". + +5. **Pass the list as separate words, in a form the shell splits.** Use an array + (`"${FILES[@]}"` in bash, an array or `${(f)…}` in zsh), or pipe into `xargs`. + zsh does not word-split unquoted parameter expansions by default — the FAQ + states "In most Bourne-shell derivatives, multiple-word variables such as + `var="foo bar"` are split into words … By default, zsh does not have that + behaviour: the variable remains intact" — so `cmd $FILES` arrives as **one** + operand naming a file that does not exist: + +| Shell the step runs under | Pass the list as | +|---------------------------|------------------| +| `bash`/`sh` (GitHub Actions default `shell: bash`) | An array `"${FILES[@]}"`; unquoted `$FILES` splits but also globs | +| `zsh` (a developer's local run of the same script) | An array, or `xargs` — unquoted `$FILES` stays one word | +| Unknown / both | `printf '%s\n' "$LIST" \| xargs -r ` — `-r` also supplies the empty-list guard from step 1 | + +6. **Prove the gate can fail, in the same invocation shape CI uses**, by putting + a deliberately violating probe file in the list and requiring red + ([testing-quality-tests-that-cannot-fail]). + +7. **Place the probe on a path the tool does not ignore.** Prettier ignores + paths in `.gitignore` and `.prettierignore`, and an ignored operand produces + the success sentence with exit 0 — so a probe in a scratch directory makes + the control pass while testing nothing. Put it inside a checked package, + confirm red, then delete it. + +8. **Resolve the base ref explicitly and fail the step when it is missing.** An + absent ref yields an empty list, which step 1 turns into a reported skip + rather than a silent pass; under a shallow clone, fetch the depth the diff + needs ([testing-quality-history-dependent-checks-on-shallow-clones]). + +## Edge cases + +| Case | Then | +|------|------| +| Renamed or deleted files appear in the diff | Filter to files present in the working tree (`git diff --diff-filter=d`); a deleted path makes the operand match nothing, which exits 2 with a message about patterns rather than about lint | +| Paths contain spaces or non-ASCII | Use NUL-delimited output (`git diff -z`) with `xargs -0`; newline-delimited lists split those paths into fragments, and each fragment then matches nothing | +| The list is long enough to hit the argument limit | Pipe through `xargs`, which batches; a single exec of the whole list fails with `E2BIG`, and that message names no lint rule | +| The gate runs on a merge commit | Diff against the merge base, not the first parent, or the list carries the other branch's files while the change's own are absent | +| The tool is `eslint` rather than `prettier` | Same empty-list shape, plus its own `--no-error-on-unmatched-pattern`; ESLint also skips files matched by `ignores`, so step 7's probe placement rule holds unchanged | +| Only the count is available in the log of an old run | The run cannot be re-read as evidence — the ignore-filtered and `--ignore-unknown` rows have non-zero counts and examined nothing; re-run with step 3's list logging before citing it | +| The step pipes the tool's output (`| tee`, `| tail`) | The pipeline's exit code is the last command's — set `set -o pipefail`, or the exit-2 rows are reported as passes | + +## Instead of + +| If you are about to | Do this instead | Why | +|---------------------|-----------------|-----| +| Read "All matched files use Prettier code style!" as the gate having checked the change | Branch on an empty list, log the passed list, and judge by exit code | Measured: that sentence is printed for an ignore-filtered list (exit 0) and alongside an unmatched-pattern error (exit 2) — it tracks neither "examined" nor "passed" | +| Invoke the tool with whatever the diff produced, including nothing | Skip explicitly at zero operands | Measured: `prettier --check` with no operands exits **0**, and its message mentions a missing parser rather than an empty list | +| Write `npx prettier --check $FILES` | Build an array and pass `"${FILES[@]}"`, or pipe to `xargs -r` | Under zsh the variable stays one word, so the tool matches nothing; it exits 2, but the log also carries the success sentence, which is what gets quoted as evidence | +| Add `--ignore-unknown` to stop errors on odd extensions | Add it and log the passed list | Measured: with the flag, a list of only unsupported files exits 0 with the success sentence — the error it removed was the signal that nothing was examined | +| Add `--no-error-on-unmatched-pattern` to quiet a noisy step | Fix the list construction, and keep step 1's empty branch | The flag converts the one loudly-failing empty case into a silent pass | +| Validate the gate with a probe file in a scratch or temp directory | Put the probe inside a checked package, require red, then delete it | Prettier ignores `.gitignore`d paths, and an ignored operand returns the success sentence with exit 0 | + +## Sources + +- https://prettier.io/docs/en/cli — `--check` reports whether files are formatted; `--ignore-unknown` makes Prettier "ignore unknown files matched by patterns"; `--no-error-on-unmatched-pattern` "prevents errors when pattern is unmatched", i.e. erroring is the default for an unmatched pattern +- https://prettier.io/docs/en/ignore — `.prettierignore` and `.gitignore` paths are excluded from formatting, the mechanism behind the ignore-filtered rows and step 7 +- https://zsh.sourceforge.io/FAQ/zshfaq03.html — "In most Bourne-shell derivatives, multiple-word variables such as `var="foo bar"` are split into words … By default, zsh does not have that behaviour: the variable remains intact"; `setopt shwordsplit` restores Bourne behaviour, and the FAQ recommends arrays +- https://www.gnu.org/software/bash/manual/bash.html — word splitting of unquoted expansions, and `"${arr[@]}"` expanding to one word per element (step 5's portable form) +- https://eslint.org/docs/latest/use/command-line-interface — `--no-error-on-unmatched-pattern` and `ignores`, the ESLint analogues in the edge-case table +- Local measurement 2026-08-27 (Prettier **3.7.4**, macOS, fixture: `good.ts` clean, `bad.ts` unformatted, `pic.png`): no operands → `[error] No parser and no file path given, couldn't infer a parser.`, **rc=0**; `'nope/**/*.ts'` → `[error] No files matching the pattern were found` *plus* `All matched files use Prettier code style!`, **rc=2**; single word-split operand `"good.ts bad.ts"` → same pair, **rc=2**; `pic.png` alone → `[error] No parser could be inferred`, **rc=2**; `--ignore-unknown pic.png` → success sentence, **rc=0**; `good.ts` while listed in `.prettierignore` → success sentence, **rc=0**; `good.ts` normally → success sentence, rc=0. This measurement corrected an earlier draft of this page, which had generalised "empty match set ⇒ exit 0" from a field log in which the success sentence and the unmatched-pattern error appeared together +- Field observation 2026-08-24 (`rtb-unified`, zsh): `npx prettier --check $FILES` emitted `[error] No files matching the pattern were found: "a.ts b.ts …"` together with the success sentence — the pairing that makes such a log read as a pass when only the sentence is quoted. A probe at `packages/orpc/src/zz-fmt-probe.ts` produced `[warn]` and a red gate; the same probe under `.claude/tmp/` passed, that path being `.gitignore`d diff --git a/wiki/infrastructure/containers/exec-added-processes-and-the-memory-budget.md b/wiki/infrastructure/containers/exec-added-processes-and-the-memory-budget.md new file mode 100644 index 0000000..8f19b37 --- /dev/null +++ b/wiki/infrastructure/containers/exec-added-processes-and-the-memory-budget.md @@ -0,0 +1,99 @@ +--- +id: infrastructure-containers-exec-added-processes-and-the-memory-budget +domain: infrastructure +category: containers +applies_to: [kubernetes, docker, cgroup-v2] +confidence: verified +sources: + - https://docs.kernel.org/admin-guide/cgroup-v2.html + - https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/ + - https://kubernetes.io/docs/reference/generated/kubectl/kubectl-commands#exec + - https://kubernetes.io/docs/tasks/configure-pod-container/assign-memory-resource/ +last_verified: 2026-08-27 +related: + [ + infrastructure-containers-host-cgroup-visibility, + infrastructure-containers-resource-limits-and-probes, + backend-common-concurrency-shared-state-and-pools, + backend-common-concurrency-distributed-locks, + infrastructure-observability-missing-container-metrics, + ] +--- + +# Running an Extra Process Inside a Container That Already Hosts a Service + +## When this applies + +You are about to `kubectl exec` (or `docker exec`) a process into a pod that is +already running a service — a CLI invoked as a remote worker, a debug shell, a +side job, an ad-hoc script — and you are judging whether it is safe. Also when +you concluded it was safe because the application's own queue, semaphore, or +worker pool is not saturated. + +## Do this + +1. **Read the container's own cgroup limit and usage before adding the + process.** The self-view needs no host cgroup namespace + ([infrastructure-containers-host-cgroup-visibility]) — inside the container, + read from `/sys/fs/cgroup/`: + +| File | What it tells you | Use it to | +|------|-------------------|-----------| +| `memory.max` | The hard limit; at it "the OOM killer is invoked in the cgroup" | Establish the budget the new process shares | +| `memory.current` | "The total amount of memory currently being used by the cgroup and its descendants" | Compute headroom now | +| `memory.peak` | "The max memory usage recorded for the cgroup … since either the creation of the cgroup or the most recent reset" | See whether the limit has already been reached under normal load | +| `memory.events` → `max` | "The number of times the cgroup's memory usage was about to go over the max boundary" | Detect that reclaim is already fighting the limit | +| `memory.events` → `oom_kill` | "The number of processes belonging to this cgroup killed by any kind of OOM killer" | Confirm kills have already happened | + +2. **Treat the application's concurrency control and the cgroup budget as two + separate limits.** An in-process semaphore (`maxConcurrentAgents: 20`, a + worker pool, a job queue) counts only the work the application itself + started. A process attached with `exec` joins the **same cgroup** and spends + the same memory, while being invisible to that counter — so "the queue has + free slots" is not a statement about memory at all. + +3. **Decide from headroom, not from queue depth**, using one rule: require + `memory.max − memory.peak` to exceed the new process's expected peak. When + `memory.peak` already equals `memory.max`, or the `max` event counter is + non-zero, the container has no proven headroom — run the process elsewhere + (a separate pod or Job with its own limit), or raise the limit first. + +4. **Size the consequence before treating it as a small risk.** The container's + limit applies to the cgroup, so an OOM kill selects a process in that cgroup + — which can be the service rather than the process you added. Establish what + a restart of that service costs: in-flight work lost, and any lock or lease + the process held ([backend-common-concurrency-distributed-locks]). + +5. **When the exec'd work is recurring rather than one-off, give it its own + cgroup.** A Kubernetes Job or a sidecar with its own `resources.limits` + makes the budget explicit and keeps a runaway invocation from selecting the + service as the OOM victim. + +## Edge cases + +| Case | Then | +|------|------| +| The pod runs cgroup v1 | Read `memory.limit_in_bytes`, `memory.usage_in_bytes`, and `memory.max_usage_in_bytes`; `memory.peak` and `memory.events` are v2 names | +| `memory.peak` was reset by another reader | It reflects "since the most recent reset", so a low value can mean a recent reset rather than low usage — corroborate with `memory.events` and the container's restart count | +| Only `requests` is set, with no `limits` | There is no `memory.max` to read (it reads `max`); the pod can consume node memory until the **node** reclaims, making the blast radius other pods ([infrastructure-containers-resource-limits-and-probes]) | +| The container's restart count is already non-zero | Prior OOM kills are the most likely cause — check the last state's reason before adding load, since the steady state is already over budget | +| The process you add is short-lived but memory-spiky (a compiler, a bundler, a model client) | Peak is what the limit tests, not average; size step 3 against its peak or run it in its own pod | +| You cannot read the cgroup files (restricted mount) | Take the limit from the pod spec (`resources.limits.memory`) and current usage from metrics; do not treat unavailability of the numbers as headroom | +| The service is the only consumer and the exec'd process replaces its work | Headroom still applies — the two overlap until the service's own task drains | + +## Instead of + +| If you are about to | Do this instead | Why | +|---------------------|-----------------|-----| +| Judge an `exec` safe because the service's queue or semaphore has free slots | Read `memory.current`/`memory.peak` against `memory.max` and decide from headroom | The semaphore counts work the application started; an exec'd process is in the same cgroup and outside that count | +| Add the process and watch for problems | Check `memory.events` first — a non-zero `max` counter means reclaim is already at the boundary | The failure mode is an OOM kill of a process in the cgroup, which can be the service, not a slow response you can observe and cancel | +| Read a non-zero `memory.events: max` as "the container was OOM-killed" | Read `oom_kill` for kills and `max` for approaches to the limit | They are separate counters — `max` counts times usage was about to exceed the boundary, which is a headroom signal rather than a kill record | +| Run a recurring side job by `exec` because it is convenient | Give it a Job or sidecar with its own `resources.limits` | A separate cgroup bounds the blast radius to that work instead of to the service sharing the limit | + +## Sources + +- https://docs.kernel.org/admin-guide/cgroup-v2.html — `memory.current` is "the total amount of memory currently being used by the cgroup and its descendants"; `memory.peak` is "the max memory usage recorded for the cgroup and its descendants since either the creation of the cgroup or the most recent reset for that FD"; at `memory.max`, "if a cgroup's memory usage reaches this limit and can't be reduced, the OOM killer is invoked in the cgroup"; in `memory.events`, `max` is "the number of times the cgroup's memory usage was about to go over the max boundary" while `oom_kill` is "the number of processes belonging to this cgroup killed by any kind of OOM killer" — the two counters distinguished in the last `Instead of` row +- https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/ — container `resources.limits.memory` is enforced by the container runtime through the cgroup, which is why every process in the container shares one budget +- https://kubernetes.io/docs/tasks/configure-pod-container/assign-memory-resource/ — a container exceeding its memory limit is a candidate for termination; the limit is the cgroup's, not any one process's +- https://kubernetes.io/docs/reference/generated/kubectl/kubectl-commands#exec — `kubectl exec` executes a command in an existing container, so the new process is created inside that container's cgroup rather than in one of its own +- Field measurement 2026-08-26 (an in-cluster review-bot pod, `limits.memory: 3Gi`): `memory.current` 2.54 GiB (85% of the limit), `memory.peak` 3.0 GiB — exactly at the limit — and `memory.events` `max 5`. The application's own `maxConcurrentAgents: 20` semaphore did not count CLI processes started via `exec`, so queue depth reported free capacity while the cgroup had no proven headroom diff --git a/wiki/infrastructure/containers/host-cgroup-visibility.md b/wiki/infrastructure/containers/host-cgroup-visibility.md index 0f06bcc..faf7198 100644 --- a/wiki/infrastructure/containers/host-cgroup-visibility.md +++ b/wiki/infrastructure/containers/host-cgroup-visibility.md @@ -48,7 +48,7 @@ mount inside the private namespace, so what looks like "the host's | Case | Then | |------|------| | Files are visible but writes fail (`cgroup.procs`: no such file / EPERM) | A different restriction: the `nsdelegate` mount option (systemd default) blocks cross-namespace-boundary writes even when reads work — reading stats is fine, migrating processes is not | -| Reading only the container's OWN limits/usage | No host namespace needed — the default namespaced view is exactly right for self-monitoring | +| Reading only the container's OWN limits/usage | No host namespace needed — the default namespaced view is exactly right for self-monitoring; for the preflight before adding a process to that container, [infrastructure-containers-exec-added-processes-and-the-memory-budget] | | Sizing decisions based on the stats you read | Limits/QoS interpretation: [infrastructure-containers-resource-limits-and-probes] | ## Instead of diff --git a/wiki/infrastructure/index.md b/wiki/infrastructure/index.md index 901e1de..f781a71 100644 --- a/wiki/infrastructure/index.md +++ b/wiki/infrastructure/index.md @@ -29,6 +29,7 @@ Match your situation to a "load when" line; load only matching pages. |------|-----------| | [pipeline-structure](ci-cd/pipeline-structure.md) | Creating or restructuring a CI pipeline; CI is slow, unreliable, or reports failures too late; deciding where a new check/stage belongs | | [secrets-handling](ci-cd/secrets-handling.md) | A build or deploy step needs credentials (registry, cloud, private packages, signing); reviewing how secrets flow through CI; a secret leaked (log/chat/commit) and deciding the response | +| [changed-files-only-gates](ci-cd/changed-files-only-gates.md) | A CI step builds a changed-files list in the shell and passes it to `prettier`/`eslint`/a checker as operands; deciding whether a green gate means "no violations" or "nothing examined"; the list is empty because a base ref did not resolve; the script runs under zsh where an unquoted variable does not word-split; placing a probe file to prove the gate can fail | ## config @@ -43,6 +44,7 @@ Match your situation to a "load when" line; load only matching pages. | Page | Load when | |------|-----------| | [host-cgroup-visibility](containers/host-cgroup-visibility.md) | A container must read the host's full cgroup v2 hierarchy (other pods' CPU/memory stats) via a hostPath/`-v` mount of `/sys/fs/cgroup`; the mounted directory is missing the `kubepods` subtree with no error | +| [exec-added-processes-and-the-memory-budget](containers/exec-added-processes-and-the-memory-budget.md) | About to `kubectl exec`/`docker exec` an extra process into a pod already running a service (remote CLI worker, debug shell, side job); judging it safe because the app’s own queue or semaphore has free slots; reading `memory.current`/`memory.peak`/`memory.events` as a headroom preflight; deciding between exec and a Job/sidecar with its own limits | | [image-builds](containers/image-builds.md) | Writing or reviewing a Dockerfile; images rebuild everything on small changes, build slowly, or are too large; choosing an image tagging scheme | | [resource-limits-and-probes](containers/resource-limits-and-probes.md) | Writing or reviewing Kubernetes-style deployment manifests; pods OOMKilled, evicted, or CPU-throttled; a dependency outage triggered a restart storm; traffic hitting pods that are not ready | | [failing-pod-on-a-repo-synced-cluster](containers/failing-pod-on-a-repo-synced-cluster.md) | A pod will not start or keeps restarting (`Pending`, `ContainerCreating`, `CrashLoopBackOff`) on a cluster whose manifests a GitOps controller (Argo CD, Flux) applies; choosing between editing the live object and committing the manifest; reading `lastState.terminated` exitCode/reason before logs to separate an OOM kill from an application exit; you hold no cluster access and must hand the diagnosis over | diff --git a/wiki/qa/process/evaluating-review-feedback.md b/wiki/qa/process/evaluating-review-feedback.md index 471b5c7..9fc15b1 100644 --- a/wiki/qa/process/evaluating-review-feedback.md +++ b/wiki/qa/process/evaluating-review-feedback.md @@ -8,7 +8,7 @@ sources: - https://github.com/obra/superpowers - https://google.github.io/eng-practices/review/reviewer/standard.html - https://google.github.io/eng-practices/review/reviewer/looking-for.html -last_verified: 2026-08-22 +last_verified: 2026-08-27 related: [qa-process-defect-class-resweep-after-review, qa-process-adversarial-change-review, qa-process-llm-review-pipelines] --- @@ -54,6 +54,8 @@ robustness or supporting features; you disagree with a finding. | You pushed back and turn out to be wrong | State the correction factually and implement the fix — the correction is the apology | | Two findings conflict with each other | Surface the conflict to the reviewer(s) and get a resolution before implementing either | | A reviewer agent flagged pre-existing code outside the diff | Verify it, then file it as separate work — expanding the current change silently mixes concerns for every later reader | +| The finding sits in the review **body** rather than an inline comment, so it quotes code without naming a file | Step 2 has no cited lines to open: grep the quoted string across the whole changed set before ruling on it, and rule only against the file the grep resolves it to | +| The quoted code does not match the file you assumed, and sibling files implement the same contract | Read it as "not yet located", not as a false positive — the usual shape is that one sibling was already fixed and another still carries the defect, so the quote matches the file you did not check | ## Instead of @@ -62,9 +64,11 @@ robustness or supporting features; you disagree with a finding. | Implement findings top-to-bottom as you read | Read all, verify all, then fix in dependency order | Later findings change what the earlier fixes should be | | Reply "You're absolutely right!" and start editing | Verify the claim against the code, then let the fix speak | Performative agreement commits you before verification, and adds noise for the next reader | | Build the "proper" version a reviewer sketched | Grep for real usage first; propose removal when unused | Unused robustness is dead weight that still has to be maintained and reviewed | +| Rebut an unanchored finding because the file you assumed does not contain the quoted code | Grep the quoted string across the changed set, then rule against the file it resolves to | A body-level finding names no file; rebutting from the wrong file rejects a real defect with an argument that looks verified | ## Sources - https://github.com/obra/superpowers — receiving-code-review skill: verify-before-implementing, clarify-all-before-any, performative-agreement ban, YAGNI usage check; field-tested across agentic coding sessions - https://google.github.io/eng-practices/review/reviewer/standard.html — technical facts and data overrule opinions and personal preferences - https://google.github.io/eng-practices/review/reviewer/looking-for.html — reviewers guard against over-engineering: solve the known problem, not the speculated future one +- Field measurement 2026-08-19 (PR #327, round 16): a bot's body-level finding quoted `label = r.get("key") if _nonempty_str(...)` with no file. The assumed file, `fill_plan.py:307`, already wrapped the call as `_label(x.get("key"))`, which read as a false positive. Grepping the quoted shape across the sibling modules resolved it to `report.py:393/416/425`, an exact match and a real defect — the two files implement the same contract and only one had been fixed diff --git a/wiki/testing/index.md b/wiki/testing/index.md index be81c36..4e65ac9 100644 --- a/wiki/testing/index.md +++ b/wiki/testing/index.md @@ -49,6 +49,7 @@ Match your situation to a "load when" line; load only matching pages. | [unasserted-return-fields](quality/unasserted-return-fields.md) | Testing a function that returns a composite (dict/record/tuple) of computed fields while assertions read only some of them; judging whether a large assertion count covers such a return; auditing which fields no assertion mentions and confirming it by mutation; asserting invariants that bind fields to each other (`lo ≤ point ≤ hi`, parts sum to total) and choosing between an input grid and generated inputs | | [stale-artifact-baselines](quality/stale-artifact-baselines.md) | Using a previously published output file (JSON/CSV export, approved snapshot) as the "before" baseline for measuring a code change's impact; its totals match the current run and you are about to treat the comparison as clean; dating an artifact's generation from its schema fields; rebuilding a before side by reverting only the change under measurement; reporting previously published numbers beside a new delta | | [surviving-mutant-equivalence-triage](quality/surviving-mutant-equivalence-triage.md) | A mutation run (PIT, Stryker, or a hand-seeded mutation) left a mutant alive on code you own and you are deciding what to change; a reviewer asks for a test covering a specific surviving mutant; a defensive branch carries a comment explaining why it is needed and its mutant survives; separating a missing test from an equivalent mutant from an uncovered line | +| [mutation-harness-file-custody](quality/mutation-harness-file-custody.md) | You wrote a script that backs up, mutates and restores source files rather than using PIT/Stryker; choosing how it keys backups and how it proves a restore; reading a `git diff`/`grep` of a tree while such a harness runs; a mutant verdict flipped after a restore bug was fixed; same-named files in different directories (`routers/deal.ts` vs `schemas/deal.ts`) | | [source-text-wiring-assertions](quality/source-text-wiring-assertions.md) | A test reads a source file as a string and asserts by regex that a call is present (cleanup in every handler, logging after each branch, teardown in each exit path) because the behavior has no reachable seam; such a guard is green while one of the call sites is gone; choosing between a count assertion, an order anchor, and a function-body slice; such a guard (a negative `not.toMatch`, or a token-count assertion) reddens on correct code and a comment is the only thing that changed | | [history-dependent-checks-on-shallow-clones](quality/history-dependent-checks-on-shallow-clones.md) | A test or gate resolves git history (`log --diff-filter`, `merge-base`, `rev-list`) and may run under a shallow CI checkout (actions/checkout default `fetch-depth: 1`); a history query reported every tracked file as added by one commit; deciding between a `--is-shallow-repository` skip-guard and deepening the fetch | | [expectation-sets-with-one-distinct-value](quality/expectation-sets-with-one-distinct-value.md) | Locking that a computed value is really carried into a response/DTO/event and every test case expects the same literal for that field; a constant-substitution mutant survives a green suite and you are about to add more assertions; choosing between a delete probe and a value-substitution probe for wiring; auditing fixtures for one-distinct-value expectation sets | diff --git a/wiki/testing/quality/mutation-harness-file-custody.md b/wiki/testing/quality/mutation-harness-file-custody.md new file mode 100644 index 0000000..8b00dd3 --- /dev/null +++ b/wiki/testing/quality/mutation-harness-file-custody.md @@ -0,0 +1,101 @@ +--- +id: testing-quality-mutation-harness-file-custody +domain: testing +category: quality +applies_to: [general] +confidence: field-tested +sources: + - https://stryker-mutator.io/docs/mutation-testing-elements/mutant-states/ + - https://pitest.org/quickstart/basic_concepts/ + - https://git-scm.com/docs/git-status + - https://docs.python.org/3/library/pathlib.html +last_verified: 2026-08-27 +related: + [ + testing-quality-tests-that-cannot-fail, + testing-quality-harness-reverse-controls, + testing-quality-surviving-mutant-equivalence-triage, + testing-quality-completion-predicates, + testing-data-artifact-leakage-from-a-suite, + qa-process-scope-purity-checks, + ] +--- + +# A Hand-Rolled Mutation Harness That Edits and Restores Files In Place + +## When this applies + +You wrote a script that backs up source files, mutates them, runs the suite, and +restores them — rather than using PIT or Stryker. You are choosing how it keys +its backups, or you are reading the tree (a `git diff`, a `grep`, a file) while +such a harness is running, or you are about to cite its SURVIVED/KILLED verdicts. + +## Do this + +1. **Key each backup by the file's full path with separators flattened** + (`src/routers/deal.ts` → `src__routers__deal.ts`), not by its basename. In a + repo organised by layer, the same domain noun names a file in several + directories — `routers/deal.ts`, `schemas/deal.ts` — and a basename key makes + the second backup overwrite the first, so restore writes one file's contents + into the other. + +2. **Verify each restore by comparing bytes against the backup before starting + the next mutation.** Read both files and require equality + (`Path(a).read_bytes() == Path(b).read_bytes()`, or `cmp -s`); stop the run on + the first mismatch. A cross-restore leaves a syntactically valid file, so the + next mutation's failures come from the corruption rather than from the + mutation, and every verdict after that point is unattributable. + +3. **Confirm the restore with the harness's own evidence, not `git diff`.** A + file the repo does not track produces an empty `git diff` whether the restore + worked or not, so "clean diff" and "file destroyed" are the same observation. + Assert instead on content: the mutation marker is absent, and a token unique + to the original file is present at its original count. + +4. **Read the tree only when the harness is not holding it.** Between backup and + restore the files are *deliberately* wrong, so a `git diff` or `grep` taken + inside that window describes the mutation, not the code. Require both signals + before reading: the process is gone (`pgrep -f`, or the harness's own exit) + **and** the backup directory is empty. Discard anything read earlier rather + than reasoning from it. + +5. **Prove the harness discriminates before quoting a score** — a + semantics-preserving no-op must SURVIVE ([testing-quality-harness-reverse-controls]). + Uniform results across every mutant (all killed, or all survived) are a + harness fault until shown otherwise. + +6. **Re-run the whole matrix after fixing a custody bug, and replace the earlier + verdicts rather than patching them.** Corruption moves verdicts in both + directions, so the prior run's KILLED entries are as suspect as its SURVIVED + ones. + +## Edge cases + +| Case | Then | +|------|------| +| Two backed-up files legitimately have identical length | Length and line count agree by coincidence — byte comparison (step 2) is what separates them; a line-count check reports the cross-restore as healthy | +| The harness dies partway (crash, cancelled session, usage limit) | The tree still holds a mutation. Restore from the backups by path key, then run the full suite and require the pre-run pass count before treating the tree as clean | +| Another agent or session shares the worktree | Its reads land inside your window and its writes land inside your backup — give the harness its own worktree, or serialise access with a lock ([backend-common-concurrency-distributed-locks]) | +| The mutated file is untracked (new work not yet committed) | `git checkout -- ` cannot restore it and reports nothing; the byte-compared copy from step 2 is the only restore path ([testing-quality-tests-that-cannot-fail]) | +| The suite writes into the same tree (snapshots, generated fixtures) | Exclude those paths from the custody check, or the restore comparison fails on files the suite legitimately changed ([testing-data-artifact-leakage-from-a-suite]) | +| A mutation makes the module fail to import | The suite total drops and the run still looks mostly green — compare test totals to the pre-mutation run, not just pass/fail | +| The mutant survived and the code looks correct | Triage it before writing a test ([testing-quality-surviving-mutant-equivalence-triage]); a custody bug and an equivalent mutant both present as SURVIVED | + +## Instead of + +| If you are about to | Do this instead | Why | +|---------------------|-----------------|-----| +| Store backups in a flat directory keyed by `os.path.basename(path)` | Key by the flattened relative path | Same-named files in different directories collide, and restore writes one file's contents into the other | +| Confirm the restore with `git status`/`git diff` being clean | Compare bytes against the backup, and assert an original token's presence and count | An untracked file shows a clean diff in both the restored and the destroyed case | +| Compare line counts to check the restore | Compare bytes | Measured: the two crossed files were both 154 lines, so the count matched while the contents were swapped | +| Read `git diff` while the harness runs to see the current code | Wait for process exit **and** an empty backup directory, then read | Files inside the window are the mutation; a normal-looking predicate read there is the injected one | +| Report the run's SURVIVED/KILLED table after fixing a restore bug | Re-run the whole matrix and publish the new table | Corruption flips verdicts both ways — measured, two mutants moved SURVIVED → KILLED after the key was fixed | + +## Sources + +- https://stryker-mutator.io/docs/mutation-testing-elements/mutant-states/ — the mutant states (Killed, Survived, No coverage, Timeout) a hand-rolled harness is reproducing; the verdict vocabulary this page's custody rules exist to keep meaningful +- https://pitest.org/quickstart/basic_concepts/ — "'Survived' means the mutation was not detected by the covering test"; a survival caused by a corrupted restore carries the same label as a real one, which is why step 2 gates the next mutation +- https://git-scm.com/docs/git-status — untracked files are reported separately from tracked modifications, and a path the repo does not track contributes no diff — the basis for step 3 +- https://docs.python.org/3/library/pathlib.html — `Path.read_bytes()` for the byte-exact restore comparison in step 2 +- Field measurement 2026-08-21 (`rtb-unified`, NEWRTB-2936): a harness keyed backups by basename, so `routers/deal.ts` and `schemas/deal.ts` shared one entry. Restoring mutant M8 wrote the schema file's contents into the router, producing `Cannot find module './common.js' imported from src/routers/deal.ts`; `grep -c dealRouter` returned 0, confirming the destruction. Both files were 154 lines, so a line-count check reported them as matching. After re-keying by flattened relative path and re-running, mutants M9 and M10 moved from SURVIVED to KILLED — the earlier verdicts had been produced by the corruption +- Field measurement 2026-08-21 (same session): a `git diff` read while `mutate.py` was mid-cycle showed an adapter predicate as `state = 'created'`; the committed code reads `state < 'active'`, and that substitution was precisely mutation D8. Re-reading after the harness exited, plus an anchor count, established the real text diff --git a/wiki/testing/quality/source-text-wiring-assertions.md b/wiki/testing/quality/source-text-wiring-assertions.md index 283b77c..afbe6ff 100644 --- a/wiki/testing/quality/source-text-wiring-assertions.md +++ b/wiki/testing/quality/source-text-wiring-assertions.md @@ -11,7 +11,7 @@ sources: - https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Regular_expressions/Quantifier - https://testing.googleblog.com/2015/01/testing-on-toilet-change-detector-tests.html - https://eslint.org/docs/latest/extend/custom-rules -last_verified: 2026-08-10 +last_verified: 2026-08-27 related: [ testing-quality-tests-that-cannot-fail, @@ -112,7 +112,7 @@ artifact for a structural shape → [testing-quality-guard-shape-vs-consequence] | The pattern must survive a formatter that reflows lines | Match on the token sequence with `[\s\S]{0,N}` between tokens rather than on a literal multi-line string, and keep the step-6 reformat control | | A site's mutant survives despite the assertion | Classify it before strengthening the pattern ([testing-quality-surviving-mutant-equivalence-triage]) — the call may be redundant at that site | | The guard reddens on a change that only added a comment | The comment contains the searched token; apply step 2 rather than loosening the pattern, which would also stop catching the deleted site | -| The assertion is negative (`not.toMatch`) or a count | These are the shapes comments flip: a `// never use X here` note satisfies the negative, and a doc comment naming the component inflates the count — step 2 is required, not optional, for both | +| The assertion is negative (`not.toMatch`) or a count | These are the shapes comments flip: a `// never use X here` note satisfies the negative, and a doc comment naming the component inflates the count — step 2 is required, not optional, for both. A slice taken as `slice(anchorIndex, indexOf(marker))` fails the same way and more quietly: when the marker also appears in a comment above the code, its earlier index collapses the slice to `''` and every assertion inside it is vacuously green — assert the slice is non-empty and holds a known token before asserting the wiring | | No parser is available for the file's language | Cut each line from its first line-comment prefix to end of line — dropping whole comment lines leaves trailing comments, the shape that actually leaks (measured: a trailing `// renders ` keeps the count at 2, cutting to EOL gives 1). Record block comments and comment-shaped text inside strings as out of scope | | The code under test is itself about comments (a doc gate, a lint rule) | Comments are the subject, so keep the raw source — and assert on the raw and stripped forms as two separate named tests so a reader knows which one each claim is about | @@ -140,5 +140,5 @@ artifact for a structural shape → [testing-quality-guard-shape-vs-consequence] - Measurement 2026-08-07 (Node): `/ANCHOR\([\s\S]{0,20}CALL\(/` and its lazy form `{0,20}?` returned identical verdicts on four inputs (anchor-then-call in range, call-before-anchor only, call beyond the bound, call both before and after the anchor) — the bound decides reach, and a call elsewhere in the file neither blocks nor is excluded by the pattern - https://eslint.org/docs/latest/extend/custom-rules — "While comments are not technically part of the AST, ESLint provides the `sourceCode.getAllComments()`, `sourceCode.getCommentsBefore()`, `sourceCode.getCommentsAfter()`, and `sourceCode.getCommentsInside()` to access them"; rules visit "nodes while traversing the abstract syntax tree (AST as defined by ESTree)". This is the split step 2 relies on: a structural check runs over a tree comments do not appear in, a text check runs over the file where they do - Measurement 2026-08-10 (Node + TypeScript 5, fixture with a JSDoc block, a line comment, a trailing comment, a URL string literal and a regex literal): on the raw source, `= 3` count assertion stayed green after a mutant deleted the post-resolve site. Binding it to the once-occurring anchor `resolveBuildingDetailEntry\(` within a 200-character bound produced red on that same mutant, and a comment-only edit kept it green diff --git a/wiki/testing/quality/tests-that-cannot-fail.md b/wiki/testing/quality/tests-that-cannot-fail.md index 307c012..fe33077 100644 --- a/wiki/testing/quality/tests-that-cannot-fail.md +++ b/wiki/testing/quality/tests-that-cannot-fail.md @@ -15,8 +15,8 @@ sources: - https://www.gnu.org/software/sed/manual/html_node/Exit-status.html - https://git-scm.com/docs/git-checkout - https://git-scm.com/docs/git-restore -last_verified: 2026-08-12 -related: [testing-quality-minimum-case-set, testing-quality-behavior-not-implementation, testing-mocking-what-to-mock, testing-async-async-testing, testing-quality-checks-that-cannot-pass, testing-quality-spec-artifact-checks, testing-quality-harness-reverse-controls, testing-quality-schema-additions-under-a-golden-gate, testing-quality-differential-run-agreement, testing-quality-completion-predicates, testing-quality-guard-shape-vs-consequence, testing-quality-injected-clock-duration-assertions, testing-quality-write-path-assertions, testing-quality-value-preserving-refactor-assertions, testing-quality-unasserted-return-fields, testing-quality-stale-artifact-baselines, backend-common-change-impact-call-site-enumeration, platforms-shells-portable-shell-scripts, qa-document-verification-spec-document-gates, testing-quality-surviving-mutant-equivalence-triage, testing-quality-source-text-wiring-assertions, testing-quality-default-values-under-test, testing-mocking-captured-call-arguments, testing-quality-captured-log-message-assertions, testing-strategy-failing-test-first] +last_verified: 2026-08-27 +related: [testing-quality-minimum-case-set, testing-quality-behavior-not-implementation, testing-mocking-what-to-mock, testing-async-async-testing, testing-quality-checks-that-cannot-pass, testing-quality-spec-artifact-checks, testing-quality-harness-reverse-controls, testing-quality-schema-additions-under-a-golden-gate, testing-quality-differential-run-agreement, testing-quality-completion-predicates, testing-quality-guard-shape-vs-consequence, testing-quality-injected-clock-duration-assertions, testing-quality-write-path-assertions, testing-quality-value-preserving-refactor-assertions, testing-quality-unasserted-return-fields, testing-quality-stale-artifact-baselines, backend-common-change-impact-call-site-enumeration, platforms-shells-portable-shell-scripts, qa-document-verification-spec-document-gates, testing-quality-surviving-mutant-equivalence-triage, testing-quality-source-text-wiring-assertions, testing-quality-default-values-under-test, testing-mocking-captured-call-arguments, testing-quality-captured-log-message-assertions, testing-strategy-failing-test-first, testing-quality-mutation-harness-file-custody] --- # Proving a Test Can Fail @@ -71,6 +71,7 @@ suite reported as covered, or you are auditing a suspiciously green suite. | Assertion swallowed by `try/catch`, or a `.catch` that ignores the error | Remove the catch and let the failure throw; for an expected failure, assert the rejection explicitly (next row) | | Error-path test that passes when no error is thrown (`expect` sits in the `catch` block; nothing asserts the throw happened) | Use `await expect(...).rejects.toThrow(ErrorType)` / `assertThrows`-style APIs, which fail when the code succeeds | | Always-true assertion (`toBeDefined`/`toBeTruthy` on a value that is always defined, `expect(arr.length).toBeGreaterThanOrEqual(0)`) | Assert the specific expected value or shape — the observable-outcome rule in [testing-quality-minimum-case-set] | +| Negative assertion (`expect(mock).not.toHaveBeenCalled()`, `not.toContain`) on a fixture that never supplies the input which would produce the positive — an empty id list that early-returns, a flag that skips the block | Give the case an input that reaches the call, so the positive is possible, then mutate toward calling it and require red. The defect is in the fixture, not the expectation: while the input is absent the assertion holds for every implementation, so strengthening it changes nothing | | Testing the mock instead of the code (mock returns X, test asserts X came back) | Assert the unit's transformation of its inputs, not the pass-through; when no transformation exists at this layer, test the layer that has one ([testing-mocking-what-to-mock]) | | Copied test body with the name changed but identical inputs and expectation | Give each case distinct inputs and its own expectation; delete exact duplicates — a renamed copy re-proves the same fact and guards nothing new | | Assertion inherited from a shared base class, mixin, or parameterised harness, whose name announces the new subject's whole shape while its body pins the original narrow scope | Read the inherited body and list what it compares; add a subject-specific assertion for each part of the shape the name claims, then prove each one with its own mutation | @@ -113,6 +114,7 @@ suite reported as covered, or you are auditing a suspiciously green suite. | Delete a suspicious always-green test to clean up | Fix it via the table above, then re-verify it can fail | The test names a behavior someone meant to guard; deletion drops the intent along with the defect | | Prove a test file can fail by seeding one mutation and watching the file go red | Seed one mutation per assertion and require exactly the owning test to redden | A file-level red is produced by whichever assertion happens to be strictest; the silent ones remain unproven | | Pick a mutation from what the test's name says it covers | Pick it from what the assertion body actually reads | An assertion inherited into a shared contract keeps its original narrow scope, so a reasonable-looking mutation sails past it | +| Write "this assertion catches mutation X" in a comment next to a new negative assertion | Apply X, require red, and write the comment from the observed result | The claim is about the fixture as much as the expectation — when the fixture cannot reach the call, X survives and the comment records a guarantee the suite does not provide | | Undo a red-run mutation with `git checkout -- ` while the fix under test is unstaged | Restore from a copy saved before mutating and compare hashes, or `git add` the fix first | The checkout restores the index copy and discards every unstaged change, so it removes the fix and the mutation together while reporting nothing | ## Sources @@ -129,4 +131,5 @@ suite reported as covered, or you are auditing a suspiciously green suite. - https://tiswww.case.edu/php/chet/bash/COMPAT — bash-4.0 changed `set -e` handling so the shell exits when a compound command fails; bash-3.2 and earlier do not, which is what lets a false mid-test `[[ ]]` pass silently under bats on macOS system bash - https://bats-core.readthedocs.io/en/stable/gotchas.html, https://www.shellcheck.net/wiki/SC2314 — the documented same-shape gotcha: bats commands whose failure is excluded from errexit (negated `!` commands) "can never fail when used in the middle of a test" - Local reproduction 2026-08-06 (Bats 1.14.0, GNU bash 3.2.57, macOS arm64): a false `[[ "a" == *"zzz"* ]]` mid-test → `ok`; the same false comparison as `[ "a" = "zzz" ]` or piped `grep -qF` mid-test → `not ok`; the `[[ ]]` as the test's last line → `not ok`. Outside bats, `bash -ec '[[ … ]]; echo survived'` printed and exited 0 while the `[ ]` form aborted — bash-3.2 errexit semantics, not a bats defect +- Field measurement 2026-08-25 (`rtb-unified`, a resume-on-exception regression test): a new `expect(mock).not.toHaveBeenCalled()` was annotated as catching a "resume on any exception" widening. The fixture passed `staleQueuedJobIds: []`, so the code early-returned before the call; applying the widening left 116/116 green. The signal was in the fixture, not the assertion — an independent reviewer found it by running the mutation, and the same assertion already existed on another case, so its discriminating power for this one was zero - Field reproduction 2026-08-12 (a Python health-check daemon, `heal_detector.py`): the `secret_source == "none"` branch sets `run_ok = False; send_failed = True` before the message loop, and the loop's own failure handling sets the same two. Deleting the branch entirely left all 42 tests passing, because both `none` cases supplied trigger messages that fail in the loop. Adding one case with `messages=[]` — the branch's only active writer — turned the same deletion RED in 1 test, with no assertion changed