diff --git a/.dev-loop/INGEST_REPORT.md b/.dev-loop/INGEST_REPORT.md index 55ccfd1..c8e9c40 100644 --- a/.dev-loop/INGEST_REPORT.md +++ b/.dev-loop/INGEST_REPORT.md @@ -1,53 +1,272 @@ -# Knowledge consolidation — 15 open PRs (#17–#40) → one reconciled state +# Knowledge flush — 6 insight(s): 5 ingested, 1 dropped -The 15 open `knowledge/*` PRs (created 2026-08-04 → 2026-08-05, before the -harvest processed-store dedupe fix in #41) contained 123 file-versions of ~75 -unique pages, with the same insight landing at up to 3 different paths across -up to 8 PRs. Per-PR review would re-import those duplicates, so — as with the -#6–#13 consolidation — this branch carries the reconciled end-state and the 15 -PRs are closed in its favor. +Queue drained: `~/.dev-loop/queue/{231f63bc,27bf507a,498e892c}.jsonl` — 6 pending rows. ## Verified best-practice -Every adopted page's sources were carried from its originating PR's flush, where -they were live-verified at flush time; no new URLs were introduced during -consolidation (checked mechanically: every `http(s)` URL in every merged page -appears in a source PR's diff; every added body line in amended pages traces to -a source PR hunk — orphan-line verification). Confidence fields were kept as the -originating flushes set them, except client-side-rate-limiting where the union -of provider-doc citations (Okta, Auth0, GitHub, OpenAI, RFC 6585) supports -`verified` for the load-bearing claims. One subagent's fabricated content (12 -files matching neither main nor any PR, with invented source URLs) was detected -by the same verification and replaced with true PR content. +Every URL below was opened in this session and the quoted sentence checked against +the fetched page. No citation was inherited from a draft, another wiki page, or a +prior flush without reopening it. + +**1. Assertions for a signed/tokenized link** → `confidence: verified` +Claim: assert the token through the verifier the receiver runs (correct subject +accepts; wrong subject and wrong key reject), and issue the real request with a +token-stripped control — presence of `?t=` is satisfied by a hardcoded placeholder. +- `https://docs.aws.amazon.com/AmazonS3/latest/userguide/using-presigned-url.html` — + confirmed verbatim: "When you create a presigned URL, you must provide your + security credentials, and then specify the following:" + bucket / object key / + HTTP method / expiration interval; "Amazon S3 checks the expiration date and time + of a signed URL at the time of the HTTP request"; the `SignatureDoesNotMatch` FAQ + "verify that all request parameters—including the HTTP method, headers, and query + string—match exactly between URL generation and usage". +- `https://docs.python.org/3/library/hmac.html` — confirmed verbatim: "When + comparing the output of `digest()` to an externally supplied digest during a + verification routine, it is recommended to use the `compare_digest()` function + instead of the `==` operator"; `compare_digest` "uses an approach designed to + prevent timing analysis by avoiding content-based short circuiting behaviour". +- Field reproduction (2026-08-11, Python HMAC approval link): 6 mutations of the + assembly path; wrong-key and wrong-batch mutants left `?t=` well-formed so every + format assertion stayed green, while the verifier assertion and 3 live-GET + assertions reddened; the no-op control survived. + +**2. Cross-module consumer census** → `confidence: verified` +Claim: count production references to a task's new public symbols outside the +defining module and outside its own tests, then treat only symbols with *declared* +cross-module intent as defects. +- `https://knip.dev/reference/configuration` — confirmed verbatim: "In files with + multiple exports, some of them might be used only internally. If these exports + should not be reported, there is a `ignoreExportsUsedInFile` option available"; + "By default, Knip does not report unused exports in entry files". These two + options are the tool's own encoding of the internal-helper and entry-point + populations that a raw zero-reference count cannot separate from real orphans. +- `https://knip.dev/guides/handling-issues` — confirmed verbatim: "So a surprising + result is usually a real finding or a configuration gap, not a false positive to + silence", plus the pre-deletion checks (export in an entry file, re-exported from + an entry file, consumed externally). +- Field measurement (2026-08-11, 6 parallel tasks split by file ownership): 14 + public functions had zero cross-module production references; exactly 1 was a + real gap (a URL builder whose docstring named its consumer), the other 13 were + same-module helpers. All 6 tasks had passed review, 402 tests green, no conflicts. + +**3. Defect-class re-sweep over your own remediation diff** → `confidence: verified` +Claim: re-run each finding's *class* search over the post-edit file including the +lines the remediation just added; read the remediation as unreviewed code. +- `https://www.eecg.utoronto.ca/~yuan/papers/incorrect_fix_abstract.html` — + confirmed verbatim: "at least 14.8% to 24.4% of sampled fixes for post-release + bugs in these large OSes are incorrect" and "27% of the incorrect fixes are made + by developers who have never touched the source code files associated with the + fix". Title/authors/venue confirmed on the same page: Yin, Yuan, Zhou, Pasupathy, + Bairavasundaram, "How Do Fixes Become Bugs?", FSE 2011. +- `https://dl.acm.org/doi/10.1145/2025113.2025121` — cited as the published record + for that paper. **Not opened** (ACM interstitial); it is a locator for the + abstract page above, which was opened and quote-checked. Flagged here rather + than presented as read. +- Field incident (2026-08-11): review round 1 said a checker echoed a raw external + string into its error message; the same round's remediation added a sibling + checker with the identical leak, reproduced by an independent audit with a key + containing a carriage return. + +**4. Failing pod on a repo-synced cluster** → `confidence: verified` +Claim: branch on pod phase before reading logs; read `lastState.terminated` +exitCode/reason first; land the fix as a manifest PR, not a live edit. +- `https://kubernetes.io/docs/tasks/debug/debug-application/debug-pods/` — confirmed + verbatim: "Check the current state of the Pod and recent events with the following + command: `kubectl describe pods ${POD_NAME}`"; "There should be messages from the + scheduler about why it can not schedule your pod"; "The most common cause of + `Waiting` pods is a failure to pull the image". +- `https://kubernetes.io/docs/tasks/configure-pod-container/assign-memory-resource/` — + confirmed verbatim: the worked example prints `exitCode: 137` / `reason: OOMKilled` + under `lastState: terminated:`, introduced by "The output shows that the Container + was killed because it is out of memory (OOM)". +- `https://kubernetes.io/docs/reference/kubectl/generated/kubectl_logs/` — confirmed + verbatim: `-p, --previous` = "If true, print the logs for the previous instance of + the container in a pod if it exists." +- `https://argo-cd.readthedocs.io/en/stable/user-guide/auto_sync/` — confirmed + verbatim: "By default, changes that are made to the live cluster will not trigger + automated sync"; self-heal = "To enable automatic sync when the live cluster's + state deviates from the state defined in Git"; and the multi-source caveat + "Disabling self-heal does not guarantee that live cluster changes in multi-source + applications will persist." +- `https://man7.org/linux/man-pages/man1/bash.1.html` — confirmed verbatim: "The + return value of a *simple command* is its exit status, or 128+*n* if the command + is terminated by signal *n*" — the convention that makes 137 read as 128+9. + +**5. Cloud CLI invocation bounds** → `confidence: verified` +Claim: read the leaf subcommand's help; name the scope on every invocation; cap +list output at the call site; disable pager/prompts for unattended callers. +- `https://docs.aws.amazon.com/cli/latest/userguide/cli-usage-help.html` — confirmed + verbatim: `aws help` "displays help for the general AWS CLI options and the + available top-level commands"; `aws ec2 help` "displays the available Amazon + Elastic Compute Cloud (Amazon EC2) specific commands"; operation help "includes + descriptions of its input parameters, available filters, and what is included as + output"; and "`describe-instances` has a default behavior that describes ***all*** + instances in the current account and AWS Region". +- `https://docs.aws.amazon.com/cli/latest/userguide/cli-usage-pagination.html` — + confirmed verbatim: "By default, the AWS CLI uses a page size determined by the + individual service and retrieves all available items"; `--max-items` "prints out + only the number of items at a time that you specify"; mixing `--page-size` and + `--max-items` "you can get unexpected results with missing or duplicated items"; + "By default, this feature returns all output through your operating system's + default pager program", disabled per command by `--no-cli-pager`. +- `https://docs.aws.amazon.com/cli/latest/userguide/cli-configure-files.html` — + confirmed verbatim: "You can override an individual setting by either setting one + of the supported environment variables, or by using a command line parameter." +- `https://docs.cloud.google.com/sdk/gcloud/reference` — confirmed verbatim: + `--project` "The Google Cloud project ID to use for this invocation. If omitted, + then the current project is assumed"; `--quiet` "Disable all interactive prompts + when running `gcloud` commands. If input is required, defaults will be used, or an + error will be raised." +- Field incident (2026-06-17, macOS hook shelling out to `aws cloudwatch`): calls + inherited no region, every invocation failed closed, and the guard the hook + implemented never evaluated. + +**6. Python `.pyc` cache staleness in a mutation harness** → dropped, see Open-PR check. +Its remedy was checked anyway: `https://docs.python.org/3/using/cmdline.html` +confirms `-B` = "If given, Python won't try to write `.pyc` files on the import of +source modules" and `PYTHONDONTWRITEBYTECODE` "is equivalent to specifying the `-B` +option". Both govern **writing** only — so the candidate's directive ("run under +`-B`") is incomplete on its own: an existing `.pyc` is still validated and reused. +Open PR #52 already states exactly this correction, which is why the candidate is +dropped rather than ingested. ## Existing-layer check -- Merged-main near-dup scan before consolidation: pairwise Jaccard over - title + "When this applies" across all 141 merged pages → **0 flagged pairs**; - previously merged content carries no duplication. -- Cross-PR dedup during consolidation: 10 duplicate clusters collapsed to one - canonical page each (rate limiting 8→1, call-site enumeration 7→folded into - the canonical merged in #20, stderr/exit-0 diagnostics 4→1, sysroot 2→1, - env-off-switch 2→1, completion predicates 2→1, robots.txt 2→1, - harness-mediated results 2→1, leaked artifacts 2→1, orchestration category - naming unified). Three near-pairs kept distinct after trigger comparison, - with mutual `related:` links (differential setup vs interpretation; expansion - semantics vs off-switch design; import-time tactics vs level choice). -- 24 existing pages received union-merged amendments; additions already present - in main (from #16/#20) were skipped, and all non-canonical `related:` ids - were remapped to canonical page ids (post-merge broken-link scan: 0). +Routed via `INDEX.md`, then each target domain's `index.md`, then every page whose +"load when" line overlapped the candidate. + +Pages read: testing-quality-signed-link-verification-assertions, testing-quality-harness-reverse-controls, backend-python-language-bytecode-cache-staleness, backend-common-change-impact-cross-module-consumer-census, qa-process-defect-class-resweep-after-review, infrastructure-containers-failing-pod-on-a-repo-synced-cluster, platforms-processes-cloud-cli-invocation-bounds + +Read at frontmatter level only (to add reciprocal `related:` ids, bodies not +re-read): `testing-quality-write-path-assertions`, +`infrastructure-containers-resource-limits-and-probes`, +`platforms-processes-parsing-cli-structured-output`. Domain indexes read in full: +`wiki/{testing,qa,backend,infrastructure,platforms}/index.md`. + +**Overlaps found and how they were resolved** + +- **Candidate 6 vs `backend-python-language-bytecode-cache-staleness` (on `main`)** — + the merged page already carries the mtime+size invalidation mechanism, the + equal-size mutation case, `__pycache__` purging and hash-based `.pyc`. The + candidate's only new material was the `-B` / `exec(compile(...))` remedy, and + open PR #52 already adds precisely that (and corrects it). No amend written here: + amending that file would have collided with #52 for content #52 states better. +- **Candidate 1 vs `testing-quality-harness-reverse-controls`** — that page owns + the *harness-level* control (a no-op mutant must survive). The new page owns the + *assertion shape* for a signed link and cites the harness page for its + prove-it-can-fail step. Distinct cases; linked, not merged. +- **Candidate 2 vs `backend-common-change-impact-call-site-enumeration`** — that + page enumerates callers of a symbol whose contract you are *changing*; the new + page counts consumers of a symbol you just *added*. The new page states the + boundary in "When this applies" and links across. +- **Candidate 3 vs `infrastructure-containers-resource-limits-and-probes`** — that + page is authoring-time sizing of limits and probes; the new page is diagnosis of + an already-failing pod. Boundary stated in both directions. +- **Candidate 5 vs `qa-process-regression-scope`** — that page scopes re-testing for + a release; the new page sweeps one defect class across one remediation diff. + +**Conflicts flagged:** none. No new directive contradicts an existing page. + +**Reciprocal links added** to `write-path-assertions`, `resource-limits-and-probes`, +`parsing-cli-structured-output`. Reciprocal links were deliberately **not** added to +`call-site-enumeration.md`, `regression-scope.md` and `non-interactive-cli-invocation.md` +even though the new pages cite them: open PRs #68/#58/#51/#50 (the first two) and +#66/#57 (the third) already modify those files, and a one-line `related:` edit there +would conflict for no knowledge gain. Every id resolves either way — invariant 4 does +not require reciprocity. + +**Invariants re-checked programmatically after the edits** (whole repo, not just the +diff): every `related:` and inline page-id resolves (0 unresolved); each new page is +listed in its domain index; all 5 new pages are ≤120 body lines (72/74/67/76/84). +The resolver was run with a fabricated id as a negative control and reported it +unresolved, so a clean result is not a silently-empty check. + +## Open-PR check + +Listed with `gh pr list --repo choiyounggi/dev-loop --state open` — 18 open heads: +#74, #73, #72, #69, #68, #66, #64, #62, #61, #58, #57, #56, #55, #52, #51, #50, #49, #47. +Every head's changed-file list was pulled (`gh api .../pulls/N/files`) and filtered to +the five candidate paths plus the amend target. + +| Candidate | Overlapping open head | Verdict | +|---|---|---| +| 1 signed-link assertions | none creates this slug; #52 adds the adjacent `testing-quality-source-text-wiring-assertions` (regex-over-source wiring guards — different case, read to confirm) | **new** | +| 2 cross-module consumer census | none creates this slug; #58 adds `corpus-sweep-before-a-rejection-rule` (bounding a new *rejection rule* against a corpus — different case, read to confirm) | **new** | +| 3 failing pod on a synced cluster | no open head touches `wiki/infrastructure/containers/` | **new** | +| 4 cloud CLI invocation bounds | #66/#57/#62 touch other `platforms/processes/` pages; none touches this slug or its subject | **new** | +| 5 defect-class re-sweep | no open head touches `wiki/qa/process/` except #58 (`regression-scope`, a different page) | **new** | +| 6 Python `.pyc` cache in a mutation harness | **#52 modifies the exact target file** and already adds the `-B`/`PYTHONDONTWRITEBYTECODE` row, the `spec_from_file_location` row, the `exec(compile(...))` remedy and two 2026-08-11 reproductions | **drop** (pending duplicate, #52 carries it in better form) | + +No sibling duplicate PR was opened for candidate 6, and nothing was pushed to #52 — +it needs no additions from this candidate. + +**Repo-state note for the reviewer.** Two local branches from interrupted 2026-08-11 +flushes, `knowledge/dch0202-rsquare-20260811-151241` and `…-160220`, were pushed to +the fork but never opened as PRs. They carry earlier drafts of candidates 1–5 under +different slugs (`signed-link-assertions`, `new-symbols-without-a-consumer`, +`defect-class-sweep-over-a-fix`, `workload-startup-failure-triage`, +`cloud-cli-query-scoping`). This PR supersedes both; they can be deleted. ## Routing decision -- New categories: `infrastructure/agent-orchestration` (5 pages; unified the - competing `orchestration`/`agent-orchestration` names), `databases/data-survey` - (1), `qa/deliverables` (1). All other pages route into existing categories. -- Canonical-path decisions: rate limiting → `backend/common/reliability/` - (sits beside timeouts-and-retries; 6 of 8 variants chose it); stderr - diagnostics → `platforms/processes/` (concern spans beyond shells); leaked - artifacts → `testing/data/artifact-leakage-from-a-suite`; call-site - enumeration → the existing `backend/common/change-impact/` page. -- All 38 new pages listed in their domain indexes (nearest-index rule; backend - routes via its python sub-index for bytecode-cache-staleness); INDEX.md domain - summaries updated for infrastructure/qa/databases. Full-wiki lint: frontmatter, - ids, related-links, index coverage, size, qualifiers, staleness → 0 findings. +| # | Insight | Target | New category? | +|---|---|---|---| +| 1 | Signed/tokenized link assertions | `testing/quality/signed-link-verification-assertions.md` | no — `testing/quality` owns assertion-shape decisions | +| 2 | Cross-module consumer census | `backend/common/change-impact/cross-module-consumer-census.md` | no — `change-impact` already owns "who consumes this symbol" | +| 3 | Failing pod on a synced cluster | `infrastructure/containers/failing-pod-on-a-repo-synced-cluster.md` | no — `containers` holds the sizing/probe sibling | +| 4 | Cloud CLI invocation bounds | `platforms/processes/cloud-cli-invocation-bounds.md` | no — `processes` owns invoking other CLIs | +| 5 | Defect-class re-sweep after review | `qa/process/defect-class-resweep-after-review.md` | no — `qa/process` owns release-quality process | +| 6 | Python `.pyc` cache in a harness | — dropped to open PR #52 | n/a | + +Candidate 3's harvested `domain: infrastructure` and candidate 4's `platforms` hints +were both honoured. Candidate 2 was harvested as `testing`; it was routed to +`backend/common/change-impact` instead, because the artifact under examination is +application source (a symbol and its consumers), not a test — `INDEX.md` routes to +the domain owning the artifact you will change, and `change-impact` already holds the +sibling page for callers of a changed symbol. Candidate 5 was harvested as `qa` and +stayed there. + +Cross-Check: no independent second agent was run — subagent dispatch is disabled for +this session — so this is a single-agent flush. What that leaves: all 14 quoted +sources were re-opened first-hand here (not inherited), the ACM DOI is disclosed +above as a locator that was *not* opened, and the wiki invariants were verified by a +script carrying its own negative control. The five page bodies were drafted by an +interrupted 2026-08-11 flush and were reviewed and re-sourced in this session rather +than trusted; their field-reproduction paragraphs report that session's measurements +and are not independently re-runnable here. + +## Decision Log + +**Intent** +- Drain all 6 queued candidates in one PR: ingest 5 as new pages, drop 1 to an + existing open PR. One PR per flush keeps review to a single pass. +- Verify every citation first-hand in this session rather than inheriting the + quotes from the interrupted 2026-08-11 drafts these page bodies came from. +- Route candidate 2 to `backend/common/change-impact` rather than its harvested + `testing` hint, because the artifact examined is application source, not a test. + +**Alternatives rejected** +- *Amend `bytecode-cache-staleness.md` with candidate 6's `-B` remedy* — rejected: + open PR #52 already adds that row and additionally corrects it (`-B` governs + writing only, so a stale `.pyc` is still reused). Amending would have conflicted + with #52 for content #52 states better. +- *Open a PR from either interrupted branch (`…-151241`, `…-160220`)* — rejected: + their slugs are earlier, weaker drafts of the same five insights, and shipping + both would create the sibling-duplicate pile-up the skill warns about. +- *Add reciprocal `related:` ids to `call-site-enumeration`, `regression-scope`, + `non-interactive-cli-invocation`* — rejected: open PRs #68/#58/#51/#50/#66/#57 + already modify those files; invariant 4 requires ids to resolve, not reciprocity, + so the conflict buys nothing. +- *Cite the ACM DOI as read* — rejected: it was not opened. It is disclosed above + as a locator for the abstract page that was opened and quote-checked. + +**Where a reviewer should look** +- `.dev-loop/INGEST_REPORT.md` "Verified best-practice" — each quote is reproduced + as checked; spot-check any one against its URL. +- The `Open-PR check` table, specifically the candidate-6 **drop** row against + PR #52's diff on `wiki/backend/python/language/bytecode-cache-staleness.md`. +- Field-reproduction paragraphs in all five pages: these report the 2026-08-11 + session's measurements and were **not** re-run here. [추정] their numbers are + accurate as recorded; they are the one class of claim in this PR that this + session could not independently reproduce. +- `wiki/*/index.md` "load when" lines for the five new rows — invariant 1 requires + they enumerate the page's distinct use cases without contradicting its trigger. diff --git a/log.md b/log.md index c930fc2..e29c02f 100644 --- a/log.md +++ b/log.md @@ -43,3 +43,4 @@ Append-only. Format: `## [YYYY-MM-DD] /dev/null`) before reading emptiness as absence — an expired credential or a missing auth plugin exits with an empty list and a message you suppressed | +| The tool is invoked through a wrapper that re-reads config (`kubectl` via a plugin, `gcloud` via a shim) | Scope flags still belong on the leaf command; also pin the config file path (`KUBECONFIG`, `CLOUDSDK_CONFIG`) so the wrapper cannot pick another one | +| The output is only for a human to read once | Keep the cap anyway and add `--output table`/`--format` — an uncapped list is what makes the next command in the same session lose its context | + +## Instead of + +| If you are about to | Do this instead | Why | +|---------------------|-----------------|-----| +| Write the flags from memory and let the error message correct you | Read the leaf subcommand's help first | Flag sets differ per operation and per CLI major version; the leaf help is the only place the operation's parameters are listed | +| Rely on `aws configure`/`gcloud config set` for the target | Pass `--region`/`--profile`/`--project`/`--context` on every command | The active config is machine state: the same command means different things on another host, and reviewers cannot see the target | +| Run `list`/`describe` with no cap and skim the result | Add `--max-items N` and a `--query` projection | The default retrieves every item, so the cost of the call scales with the account, not with the question | +| Suppress stderr to keep the output clean | Keep stderr and read it | Auth and plugin failures surface only there, and their empty stdout reads as "no such resource" | + +## Sources + +- https://docs.aws.amazon.com/cli/latest/userguide/cli-usage-help.html — the three help levels and their content, including that the operation-level help "includes descriptions of its input parameters, available filters, and what is included as output"; `describe-instances` "has a default behavior that describes ***all*** instances in the current account and AWS Region" +- https://docs.aws.amazon.com/cli/latest/userguide/cli-usage-pagination.html — "By default, the AWS CLI uses a page size determined by the individual service and retrieves all available items"; `--max-items` "prints out only the number of items at a time that you specify"; `--no-paginate` "Disabling pagination has the AWS CLI only call once for the first page"; mixing `--page-size` and `--max-items` "can get unexpected results with missing or duplicated items"; client-side pager: "By default, this feature returns all output through your operating system's default pager program", disabled per command with `--no-cli-pager` +- https://docs.aws.amazon.com/cli/latest/userguide/cli-configure-files.html — "You can override an individual setting by either setting one of the supported environment variables, or by using a command line parameter" +- https://docs.cloud.google.com/sdk/gcloud/reference — `--project`: "The Google Cloud project ID to use for this invocation. If omitted, then the current project is assumed"; `--quiet`: "Disable all interactive prompts when running `gcloud` commands. If input is required, defaults will be used, or an error will be raised" +- Field incident 2026-06-17 (macOS, CI-style hook shelling out to `aws cloudwatch`): the calls inherited no region because the hook's environment had none, and every invocation failed closed — the guard the hook implemented never evaluated. Putting `--region` on each command in the hook restored it diff --git a/wiki/platforms/processes/parsing-cli-structured-output.md b/wiki/platforms/processes/parsing-cli-structured-output.md index b62164e..8bd620f 100644 --- a/wiki/platforms/processes/parsing-cli-structured-output.md +++ b/wiki/platforms/processes/parsing-cli-structured-output.md @@ -7,7 +7,7 @@ confidence: field-tested sources: - https://man7.org/linux/man-pages/man1/jq.1.html last_verified: 2026-08-04 -related: [platforms-processes-non-interactive-cli-invocation, backend-common-integrations-externally-owned-defaults] +related: [platforms-processes-non-interactive-cli-invocation, backend-common-integrations-externally-owned-defaults, platforms-processes-cloud-cli-invocation-bounds] --- # Parsing Another Tool's `--json` Output From Automation diff --git a/wiki/qa/index.md b/wiki/qa/index.md index 2278dfd..ed95c40 100644 --- a/wiki/qa/index.md +++ b/wiki/qa/index.md @@ -18,6 +18,7 @@ Match your situation to a "load when" line; load only matching pages. | [severity-and-priority](process/severity-and-priority.md) | Triaging a bug — deciding how bad it is and when it gets fixed; a triage stalled on a severity debate | | [post-release-verification](process/post-release-verification.md) | A release just deployed to production; defining what "released safely" means; an incident revealed a release was broken for hours before anyone noticed | | [scope-purity-checks](process/scope-purity-checks.md) | Proving a change/session/agent run touched nothing outside an allowed path set by filtering `git status --porcelain` output; a purity gate flags `?? dir/` for a directory that is wholly in scope; writing such a gate for an orchestration/CI workflow | +| [defect-class-resweep-after-review](process/defect-class-resweep-after-review.md) | Addressing review findings when the remediation itself adds code (a new function, branch, call site, formatter); about to hand that diff to a verifier, an audit, or CI as "review comments addressed"; deciding whether a bot's acknowledged/resolved thread means the class is closed; reporting fix completeness as a class-with-method count rather than a per-finding list | ## deliverables diff --git a/wiki/qa/process/defect-class-resweep-after-review.md b/wiki/qa/process/defect-class-resweep-after-review.md new file mode 100644 index 0000000..c16e8d3 --- /dev/null +++ b/wiki/qa/process/defect-class-resweep-after-review.md @@ -0,0 +1,80 @@ +--- +id: qa-process-defect-class-resweep-after-review +domain: qa +category: process +applies_to: [general] +confidence: verified +sources: + - https://www.eecg.utoronto.ca/~yuan/papers/incorrect_fix_abstract.html + - https://dl.acm.org/doi/10.1145/2025113.2025121 +last_verified: 2026-08-11 +related: [qa-process-regression-scope, backend-common-change-impact-call-site-enumeration, testing-quality-tests-that-cannot-fail] +--- + +# Re-Sweeping the Reviewed Defect Class Across Your Own Remediation Diff + +## When this applies + +You are addressing review findings and the remediation itself adds code — a new +function, a new branch, a new call site, a new formatter. Also when you are about +to hand that diff to a verifier, an audit, or CI as "review comments addressed". + +Choosing what to *re-test* for the release around it → +[qa-process-regression-scope]. + +## Do this + +1. **Name the class each finding belongs to, in the finding's own words** — + "raw external string reaches the error message", "list call with no bound", + "await missing on the returned promise". The finding cites instances; the class + is what the reviewer was actually objecting to, and it is the searchable unit. + +2. **Re-run each class's search over the whole file after the edit, including the + lines you just wrote.** The review indexed the code as it stood when it was + read, so anything the remediation added is outside its index — closing the + class in one function while opening it in a sibling function happens inside a + single round. + +3. **Report the class, not the line**: "class X occurs at N sites in this module; + all N carry the fix; method: `grep ` over ``". A per-finding + "fixed" list cannot be checked against the class + ([backend-common-change-impact-call-site-enumeration] for stating the + enumeration method next to the count). + +4. **Turn a class with two or more sites into one shared helper plus one test that + asserts the property at every site**, so the next round's new site inherits the + fix instead of needing to be found again + ([testing-quality-tests-that-cannot-fail] for making that assertion able to + fail). + +5. **Read the remediation diff as unreviewed code.** It was written under time + pressure, has no review history, and — measured across large OS code bases — + "at least 14.8% to 24.4% of sampled fixes for post-release bugs … are + incorrect", with 27% of the incorrect fixes made by developers who "have never + touched the source code files associated with the fix". + +## Edge cases + +| Case | Then | +|------|------| +| The class has no textual signature (semantic, e.g. "value used before validation") | Write the throwaway predicate — an AST pass or a 30-line script over the module — and report its verdict list; the sweep is the artifact, the tool is disposable | +| The finding is a false positive you intend to rebut | Run the class sweep anyway and put its result in the rebuttal — a bounded class list is what makes "this instance is intentional" reviewable | +| Two findings in the round share one class | Remediate once through a shared path, then let the sweep count adoption per site rather than tracking the two findings separately | +| The remediation is a revert | Sweep the class after reverting too: a revert can restore a pre-existing instance that the forward change had removed | +| The reviewer is a bot that marks the thread acknowledged/resolved | Judge closure by grepping the file, not by thread state — an acknowledgement records a reply, not a diff | +| The class spans files you do not own in this task | Sweep them read-only, report the out-of-scope instances with paths, and open a follow-up task naming them | + +## Instead of + +| If you are about to | Do this instead | Why | +|---------------------|-----------------|-----| +| Send the diff onward as soon as each cited line is fixed | Run each finding's class search over the post-edit file first | The citation is a sample of the class taken before your new code existed | +| Treat the previous round's approval as covering the fix | Read the remediation as unreviewed code and sweep it | Approval covered the code that was read; the fix is newer than the review | +| Fix exactly the lines the reviewer quoted | Fix the class in that module and report the count | A reviewer quotes what they happened to read; sibling sites keep the defect and reappear in the next round | +| Count "N findings resolved" as the completion metric | Count "class X: N sites, N remediated, method stated" | Finding counts measure the review's reach; class counts measure the code | + +## Sources + +- https://www.eecg.utoronto.ca/~yuan/papers/incorrect_fix_abstract.html — "at least 14.8% to 24.4% of sampled fixes for post-release bugs in these large OSes are incorrect" (Linux, OpenSolaris, FreeBSD, and a 12-year-old commercial OS); "Developers and reviewers for incorrect fixes usually do not have enough knowledge about the involved code. For example, 27% of the incorrect fixes are made by developers who have never touched the source code files associated with the fix" +- https://dl.acm.org/doi/10.1145/2025113.2025121 — Yin et al., "How do fixes become bugs?", ESEC/FSE 2011: the published record for the study above +- Field incident 2026-08-11 (Python validation engine, review round 1 → remediation): the finding was that a checker echoed a raw external string into its error message. The remediation closed that checker and, in the same round, added a sibling checker that echoed the same untrusted field. An independent audit reproduced it by passing a key containing a carriage return and reading the leaked original in the message; a grep for the class over the module after the edit would have listed both sites diff --git a/wiki/testing/index.md b/wiki/testing/index.md index 5f3ed78..cd48150 100644 --- a/wiki/testing/index.md +++ b/wiki/testing/index.md @@ -27,6 +27,7 @@ Match your situation to a "load when" line; load only matching pages. | [guard-shape-vs-consequence](quality/guard-shape-vs-consequence.md) | A repo-wide guard asserting that no shipped artifact (example, config, migration, fixture) has a structural shape has gone red on a legitimate new artifact; authoring such a scanning guard; deciding between exempting an artifact, deleting the guard, and sharpening it; an existing guard has accumulated an exemption/allow list | | [injected-clock-duration-assertions](quality/injected-clock-duration-assertions.md) | Asserting an elapsed duration between two readings of an injected/fake float clock (rate-limit interval, backoff, debounce, TTL); choosing that fake clock's start value; a single duration test fails on correct code by a margin in the far decimal places; choosing a comparison tolerance, or deciding between float seconds and integer nanoseconds | | [write-path-assertions](quality/write-path-assertions.md) | Writing an HTTP-level test for an endpoint that persists something (form submit, create/update, onboarding step) and choosing what to assert beyond the status code; such a test is green while the records are empty or defaulted; sending repeated form fields from a client (httpx/TestClient) and deciding the `data=` shape | +| [signed-link-verification-assertions](quality/signed-link-verification-assertions.md) | Choosing the assertions for code that builds a URL carrying its own credential (approval or magic link with `?t=`, object-storage presigned URL, webhook callback, unsubscribe link); such a test is green while the link the product sends is rejected by its receiver; deciding between asserting the parameter's presence, running the token through the production verifier, and issuing the real request; pinning a token to both its key and its subject | | [minimum-case-set](quality/minimum-case-set.md) | Writing tests for a function/endpoint/change and choosing which cases to cover; reviewing whether coverage suffices; picking boundary values by input type; adding a regression test for a bug fix | | [behavior-not-implementation](quality/behavior-not-implementation.md) | Deciding what a test should assert; a behavior-preserving refactor broke tests; tempted to expose privates for testing; deciding whether a snapshot test is appropriate | | [tests-that-cannot-fail](quality/tests-that-cannot-fail.md) | Reviewing tests that always pass; a bug shipped through an area the suite reported as covered; auditing a suspiciously green suite; judging whether an assertion, error-path test, or mock-based test can actually detect a defect | diff --git a/wiki/testing/quality/signed-link-verification-assertions.md b/wiki/testing/quality/signed-link-verification-assertions.md new file mode 100644 index 0000000..c1d0de6 --- /dev/null +++ b/wiki/testing/quality/signed-link-verification-assertions.md @@ -0,0 +1,85 @@ +--- +id: testing-quality-signed-link-verification-assertions +domain: testing +category: quality +applies_to: [general] +confidence: verified +sources: + - https://docs.aws.amazon.com/AmazonS3/latest/userguide/using-presigned-url.html + - https://docs.python.org/3/library/hmac.html +last_verified: 2026-08-11 +related: [testing-quality-tests-that-cannot-fail, testing-quality-guard-shape-vs-consequence, testing-quality-write-path-assertions, testing-quality-harness-reverse-controls] +--- + +# Testing Code That Assembles a Signed or Tokenized Link + +## When this applies + +You are choosing the assertions for code that builds a URL carrying its own +credential — an approval or magic link with `?t=`, an object-storage +presigned URL, a webhook callback URL, an unsubscribe link — and the obvious +assertion is that the parameter is present in the string. Also when such a test +is green while the link the product actually sends is rejected by its receiver. + +Whether a source-text-shaped assertion is the right level at all → +[testing-quality-behavior-not-implementation]. + +## Do this + +1. **Assert through the verifier the receiver runs, passing the subject the link + is supposed to authorize.** Hand the built URL's token plus that subject (row + id, batch id, object key, HTTP method) to the production verification + function and assert it accepts. A signature is opaque and fixed-width for + every key and every payload, so only recomputation separates a correct token + from a wrong one — this is why Python's `hmac` docs tell verification routines + to compare `digest()` output with `compare_digest()` rather than `==`. + +2. **Put both rejection arms in the same test**: the same token against a + *different* subject, and a token produced under a *different* key. Assert each + is rejected. One accept plus two rejects is the smallest set that pins the + token to its key and its subject + ([testing-quality-minimum-case-set]). + +3. **When the receiver is reachable from the suite, issue the real request** — + in-process test client, or the app bound to a port — and assert the success + status. Add the control in the same test: the identical path with the token + removed must be rejected. + +4. **Assert every field the receiver folds into the signature**, one per + assertion. A presigned URL is built from "your security credentials" plus a + bucket, an object key, "An HTTP method", and "An expiration time interval"; + AWS's own remedy for `SignatureDoesNotMatch` is to "verify that all request + parameters — including the HTTP method, headers, and query string — match + exactly between URL generation and usage". A test that reads only the query + string checks none of that match. + +5. **Prove the assertions can fail before citing them.** Mutate the assembly + twice — sign with another key, sign over another subject — and require the + verifier assertion (and the live-request assertion) to redden, with a no-op + mutation that stays green ([testing-quality-harness-reverse-controls]). + +## Edge cases + +| Case | Then | +|------|------| +| The verifier lives in another service or language | Pin one known-good `(key, subject, token)` vector produced by that service as a literal fixture, and assert the builder reproduces it byte for byte; keep the wrong-key/wrong-subject arms against your own verifier | +| The token is opaque and stored (a DB row, not a keyed digest) | Assert the row exists with the expected subject and single-use state, then assert the receiver accepts the link once and rejects the second use | +| The link only ever appears inside a rendered message (Slack block, email body) | Extract the URL from the rendered payload in the test and run steps 1–3 on it, so the notification test covers the link rather than the sentence around it | +| Expiry is enforced by the receiver | Inject the clock and assert accept before the boundary and reject after it ([testing-quality-injected-clock-duration-assertions]) | +| The token is a live credential for a real environment | Assert the verifier's verdict, never the token value, and keep test tokens keyed to a test-only secret so failure output carries no usable credential | +| The builder is the only signer and the test wants an expected value | Take the expected token from a committed vector, not from a second call into the signer — an expectation routed through the symbol under test drifts with it and pins nothing | + +## Instead of + +| If you are about to | Do this instead | Why | +|---------------------|-----------------|-----| +| `assert "?t=" in url` (or a regex for the parameter's shape) | Pass the token and its subject to the verifier and assert accept, plus wrong-subject and wrong-key rejects | A hardcoded placeholder satisfies the pattern, and the pattern is identical for a token signed with the wrong key | +| Compute the expected token in the test by calling the same signing helper | Assert via the verifier, and pin one literal vector for the byte-level check | Expectation and implementation move together, so the assertion survives any change to either | +| Skip the real request because the receiver needs a server | Bind the app in-process (test client) and GET the built URL, with a token-stripped control | The assembly and the receiver disagree about scope or encoding exactly where no test crosses the boundary | +| Assert the link is present in the notification body and stop | Extract the URL and verify its token | A message can carry a link whose token was never signed over the right subject — the receiver rejects it and the send path reports success | + +## Sources + +- https://docs.aws.amazon.com/AmazonS3/latest/userguide/using-presigned-url.html — "When you create a presigned URL, you must provide your security credentials, and then specify the following: An Amazon S3 bucket / An object key / An HTTP method / An expiration time interval"; "Amazon S3 checks the expiration date and time of a signed URL at the time of the HTTP request"; on `SignatureDoesNotMatch`: "verify that all request parameters—including the HTTP method, headers, and query string—match exactly between URL generation and usage" +- https://docs.python.org/3/library/hmac.html — "When comparing the output of `digest()` to an externally supplied digest during a verification routine, it is recommended to use the `compare_digest()` function instead of the `==` operator"; `compare_digest` "uses an approach designed to prevent timing analysis by avoiding content-based short circuiting behaviour" — verification is a recomputation, not an inspection of the token's shape +- Field reproduction 2026-08-11 (Python, HMAC-signed approval link): six mutations of the link-assembly path, scored against the same suite. Signing with a different key and signing over a different batch id both left the `?t=` parameter present and well-formed, so every format assertion stayed green; the assertion that ran the token through `auth.verify` with the expected batch, plus three assertions that GET the link against the live receiver, reddened on both. The no-op control mutation stayed green diff --git a/wiki/testing/quality/write-path-assertions.md b/wiki/testing/quality/write-path-assertions.md index ddd4434..9e4f261 100644 --- a/wiki/testing/quality/write-path-assertions.md +++ b/wiki/testing/quality/write-path-assertions.md @@ -8,7 +8,7 @@ sources: - https://github.com/encode/httpx/blob/master/httpx/_content.py - https://www.python-httpx.org/quickstart/ last_verified: 2026-08-05 -related: [testing-quality-tests-that-cannot-fail, testing-quality-minimum-case-set, backend-python-boundaries-runtime-validation] +related: [testing-quality-tests-that-cannot-fail, testing-quality-minimum-case-set, backend-python-boundaries-runtime-validation, testing-quality-signed-link-verification-assertions] --- # Testing an Endpoint That Writes: Asserting Past the Status Code