diff --git a/.dev-loop/INGEST_REPORT.md b/.dev-loop/INGEST_REPORT.md index c375750..dfca3fd 100644 --- a/.dev-loop/INGEST_REPORT.md +++ b/.dev-loop/INGEST_REPORT.md @@ -1,95 +1,200 @@ -# Consolidated review — knowledge PRs #6–#13 +# Knowledge flush — 7 insight(s) -Eight fork PRs (`dch0202-rsquare`, 2026-07-28 → 2026-08-02) were reviewed together -against `AGENTS.md`. Each PR was audited by an independent reviewer (format rules, -sources, vague-qualifier ban, ≤120 body lines, index/log invariants), then -cross-compared to catch duplication the per-PR flushes could not see — they branched -independently off the same main and rewrote the same shared index/log files. Fork -branches can't be edited from here and several PRs needed content changes (drop a -duplicate, merge a colliding page), so this branch carries the reconciled end-state -rather than merging each PR as-is (which would import the duplicates). +Drained `~/.dev-loop/queue` (7 pending rows across 5 session files). Result: +**5 new pages, 2 revised pages, 1 new category** (`infrastructure/orchestration`). ## Verified best-practice -Sources are per-page and were live-verified in each originating PR's flush; the -independent re-reviews re-checked them. Landed pages and their evidence base: - -| Page | Confidence | Source basis | -|------|-----------|--------------| -| backend/common/llm/completion-response-validation | verified | OpenAI reasoning guide + chat `object` spec (5 `finish_reason` values), vLLM/LiteLLM reasoning fields; field incident (200/`length`/empty content/8,173-char reasoning) | -| backend/common/llm/context-window-budget | verified | Claude context-window docs, LiteLLM exception mapping, vLLM/Claude Code env-var docs | -| backend/common/integrations/externally-owned-defaults | verified | OpenAI deprecations (notice windows) + models `list`, LiteLLM model_discovery; field incident (alias removed between PR verify and review → 400) | -| backend/common/storage/object-key-persistence | verified | AWS S3 CompleteMultipartUpload + managed-upload API/source, aws-sdk-js issues #1158/#5656 | -| infrastructure/containers/host-cgroup-visibility | field-tested | cgroup_namespaces(7), Docker `--cgroupns=host`, nsenter, k8s #103363; OrbStack repro | -| infrastructure/observability/missing-container-metrics | verified/field-tested | k8s resource-metrics-pipeline docs, kube-prometheus-stack values, kubernetes-mixin; OrbStack #2217 repro | -| platforms/environment/unicode-text-matching | verified | UAX #15, Unicode core §3.12, APFS FAQ, POSIX grep; local repro (macOS 15/APFS, grep 2.6.0-FreeBSD, Python 3.13) | -| platforms/shells/command-text-inspected-before-execution | verified | Claude Code hooks docs, POSIX shell §2.6; local reproduction | -| platforms/processes/non-interactive-cli-invocation | verified | GNU nohup, OpenBSD ssh/ssh_config, git, timeout man pages; no-request-in-gateway-log field incident | -| qa/document-verification/spec-document-gates | field-tested | ESLint, Google mutation testing, RFC 2119, Vale, markdownlint; 32/32 mutant / 62/62 intact RFC sessions | -| qa/document-verification/editing-a-gated-document | field-tested | pgrep, Vale, markdownlint; in-house editing methodology | -| testing/quality/checks-that-cannot-pass | verified | James Shore AoAD2, POSIX grep exit status, Semgrep rule-testing, pytest exit codes; BSD/ugrep measurement | -| testing/quality/spec-artifact-checks | verified | JSON Schema, ESLint RuleTester, pitest, GFM table spec; local cell-count repro + GitHub renderer cross-check | -| testing/quality/harness-reverse-controls | verified | mutation-testing + CI-control sources; field repro (re-fetched all cited URLs, PASS) | - -Three pages were reconciled from two overlapping PR versions each, keeping the more -complete/better-sourced body and folding in the other's unique cases: -- **completion-response-validation** — #12 body (all five `finish_reason` values, - `tool_calls`/`function_call` carve-out, streaming, Responses API, "reasoning is - scratch, not deliverable") kept in `llm/` (coherent with #6/#13); folded in #6's - DeepSeek first-party edge + the field incident. -- **externally-owned-defaults** — #12 generalized body (any repo-external resource) - in `integrations/`; folded in #6's alias-removed field incident + the - gateway-config-vs-live-upstream nuance. -- **non-interactive-cli-invocation** — #12 body (GNU-nohup extension precision, - ssh -n stdin-detach vs BatchMode, pre-log DNS/TLS/proxy + `curl -v`) kept; folded - in #11's DEBIAN_FRONTEND, pager/color TTY case, wrapper-CLI case, field incident. +### I1 — A plugin hook rewrites what a tool returns +**Claim:** when a tool returns content that does not match the file, test its +suggested workaround once; if it fails, switch to `grep -n` + `awk` and name that +fallback in every spawned worker's brief. +**Sources checked:** [Claude Code hooks reference](https://code.claude.com/docs/en/hooks) — +documents `PostToolUse` → `hookSpecificOutput.updatedToolOutput`, which "replaces +the tool's result", and `PreToolUse` → `updatedInput`, which "replaces a tool's +arguments before it runs"; explicitly names this the interception point "for +redaction or transformation use cases … `PostToolUse` for inbound tool results". +**Verification:** the *mechanism* is documented, which upgrades the insight from +"a plugin was weird" to a named, expected harness capability. The specific +substitution behavior is the session's own observation (4 files, retry with +`offset=151, limit=120` returned line 1 again, two worker panes independently +logged it). Also corrected the queued claim's framing: the file is intact, the +*channel* is mediated — so the page directs at channel choice, not at the file. +**Confidence: `verified`** (mechanism cited; field incident recorded separately +under "Field context"). + +### I2+I3 — A control signal is not evidence +**Claim:** confirm a status file or watcher verdict against the primary artifact +(git log, substrate liveness) before restarting, discarding, or merging; report +an orchestrator-assigned task id rather than a discovered session name; `stat` +the status file after writing it. +**Sources checked + reproduced:** +- [tmux manual](https://man.openbsd.org/tmux) — "if a session is omitted, the + current session is used if available; if no current session is available, the + most recently used is chosen." +- **Reproduced locally 2026-08-05:** `env -u TMUX tmux display-message -p '#S'` + printed `lo-test` — an unrelated session — with **exit 0**. This is the exact + false-positive mechanism the queued insight described (`lo-14/15/16-npmcli`): + a worker outside tmux does not get an error, it gets someone else's session name. +- **Verified against this machine's own guardrail source:** + `plugins/guardrails/hooks/bash-guard.sh:217-245` — `worktree_escape` fires when a + linked worktree's command *text* names the main root together with a write verb + (`rm|mv|cp|tee|mkdir|touch|install|dd` or a `>`/`>>` redirect), independent of + the command's purpose; `plans/worker-safety-v0.1/design.md:82` states the rule + "only sees the literal command text", which is why the Write tool succeeded on + the same path. Default mode `ask` → `deny` under `GROUNDWORK_NONINTERACTIVE=1`. +- [Claude Code hooks](https://code.claude.com/docs/en/hooks) — `PreToolUse` runs + "Before a tool call executes. Can block it", so a blocked call leaves no side effect. +**Confidence: `verified`.** + +### I4 — Shared orchestration state across concurrent runs +**Claim:** namespace the state directory per run id; survey it for foreign task +ids before writing; confirm a foreign run from `git worktree list` / branches / +default-branch HEAD rather than from the status files. +**Sources checked:** [git-worktree](https://git-scm.com/docs/git-worktree) — +`git worktree list` enumerates every linked worktree of the repository including +another process's, `git worktree prune` clears administrative files for removed +ones (this is what makes the repo, not the status dir, the reliable witness). +**Verification:** the *incident* is field evidence only (two dev-loop runs sharing +`.orchestration/status`; run B merged four of run A's branches to the default +branch `373d9fa`→`24ea9e8` without A's gate). The remedy is standard shared-mutable- +state hygiene, but no external source prescribes this specific layout. +**Confidence: `field-tested`** — deliberately not upgraded to verified. + +### I5 — A throttle the token request slips past +**Claim:** throttle at the transport layer every request passes through, stamp the +timestamp immediately before the send, and decide whether the token endpoint takes +a slot from the provider's documented bucket policy. +**Sources checked — and this materially corrected the queued directive:** +- [GitHub REST rate limits](https://docs.github.com/en/rest/using-the-rest-api/rate-limits-for-the-rest-api) — + "No more than 2,000 OAuth access token requests per hour", a **secondary** limit + distinct from the primary REST limit. +- [Okta OAuth token endpoint limits](https://developer.okta.com/docs/reference/rl2-token-oauth/) — + metered per authorization server, separately from other API endpoints. +- [Auth0 rate limit policy](https://auth0.com/docs/troubleshoot/customer-support/operational-policies/rate-limit-policy) — + per-endpoint policies, Authentication API metered separately. +- [KIS open-trading-api](https://github.com/koreainvestment/open-trading-api) — + `EGW00201` 초당 거래건수 초과; "토큰 재발급 - 1분당 1회 발급됩니다"; 모의투자 계좌는 + REST API 호출 제한이 낮음 (corroborates the reported 2/sec paper-account limit). +- [RFC 6749 §4.4](https://datatracker.ietf.org/doc/html/rfc6749#section-4.4) — the + token is obtained by an ordinary HTTP POST, subject to the server's limits. +**Correction made:** the queued directive said flatly "include token requests in +the throttle counter". That is **not universally right** — GitHub, Okta and Auth0 +meter the token endpoint in its own bucket, where charging it to the API limiter +would under-use the quota. The page therefore ships a three-row decision table +(shared bucket / separate bucket / undocumented) instead of the flat rule, and +keeps the genuinely universal part: the throttle must sit below the auth layer, +because that is what the bug actually was. +**Confidence: `verified`.** + +### I6 — Enumerating call sites for a signature migration +**Claim:** search by callee symbol, not parameter name; audit test helper +definitions that reproduce the old shape. +**Sources checked:** +- [Python tutorial — keyword arguments](https://docs.python.org/3/tutorial/controlflow.html#keyword-arguments) — + the same parameter may be passed positionally or by keyword, so the parameter + name is simply absent from a positional call's text. This is the mechanism. +- [rope ChangeSignature](https://deepwiki.com/python-rope/rope/4.6-change-signature-and-other-refactorings) — + resolves call sites through the callee **symbol** and normalizes positional↔keyword. +- [ReSharper Change Signature](https://www.jetbrains.com/help/resharper/Refactorings__Change_Signature.html) — + "finds and updates all usages, base symbols, implementations, and overrides". +**Verification:** every serious refactoring tool is symbol-driven, not +parameter-name-driven — that is the industry answer to this exact problem, and it +also supplied a directive the queued insight lacked (prefer the symbol-aware +rename; use grep to *verify* it, not to plan it). +**Confidence: `verified`.** Session measurement retained as field context +(`Ran 472 tests / FAILED (failures=11)`). + +### I7 — `${VAR:-default}` swallows a deliberate empty value +**Claim:** `:-` substitutes for unset **and** null; `-` only for unset. So passing +`VAR=` to disable a feature is silently ignored. +**Sources checked:** [POSIX Shell Command Language §2.6.2](https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html) — +"use of the <colon> in the format shall result in a test for a parameter that +is unset or null; omission of the <colon> shall result in a test for a +parameter that is only unset." +**Reproduced locally 2026-08-05** with `VAR=` under **sh, bash, zsh and dash**: +`${VAR:-d}` → `d`, `${VAR-d}` → empty, in all four. +**Confidence: `verified`** (strongest of the batch — spec text + 4-shell repro). ## Existing-layer check -Cross-PR and against-main duplication was the focus. Findings and resolutions: - -- **spec-artifact-checks (#8) ≡ document-conformance-checks (#9)** — same case - (coverage-vs-validity split, per-check negative controls, GFM pipe parsing, - ESLint/Semgrep/mutation examples). #9's report predated awareness of #8. → - **#8 kept canonical; #9's page dropped, `testing/docs-as-spec` category not created.** -- **completion-response-validation (#6) ≈ llm-response-completeness (#12)** — ~95% - same case (HTTP 200 ≠ usable output; `length`/blank/reasoning-budget). → - **merged into one `llm/` page; #12's `integrations/` copy dropped.** -- **gateway-model-alias-defaults (#6) ≈ externally-owned-defaults (#12)** — ~80%; - #12 generalizes the model-alias case to any external resource. → - **kept the general `integrations/` page; #6's LLM-only page dropped.** -- **non-interactive-cli-invocation** — created by BOTH #11 and #12 (file collision). - → **single reconciled page.** -- Distinct (no overlap, all landed): checks-that-cannot-pass, harness-reverse-controls, - spec-document-gates, editing-a-gated-document, unicode-text-matching, - command-text-inspected-before-execution, object-key-persistence, context-window-budget, - host-cgroup-visibility, missing-container-metrics. -- Reciprocal `related:` links added on existing pages (tests-that-cannot-fail, - timeouts-and-retries, environment-config, release-gates, background-services, - portable-shell-scripts, timezone-and-locale, paths-case-and-line-endings, - acceptance-criteria, resource-limits-and-probes, logs-metrics-signals, - minimum-case-set). A dropped-page backlink (#6 → gateway-model-alias-defaults on - environment-config and release-gates) was retargeted to externally-owned-defaults. -- Invariants verified programmatically: all `related:`/inline `[id]` references - resolve, every page listed in its domain index, no duplicate ids, no page >120 - body lines. +**Pages read before writing:** `INDEX.md`, `AGENTS.md`, `templates/page.md`, and +the full domain indexes for `platforms`, `infrastructure`, `testing`, `backend`, +`debugging`. Full bodies read where "load when" overlapped: +`platforms/shells/portable-shell-scripts`, +`platforms/shells/command-text-inspected-before-execution`, +`backend/common/reliability/timeouts-and-retries`. + +| Insight | Overlap found | Resolution | +|---------|---------------|------------| +| I7 | `portable-shell-scripts` already had an edge case pointing `set -u` users at `"${OPT:-}"` — same expansion family, opposite direction (there `:-` is the fix; here it is the trap) | **Merged, not created.** Added directive 6 (two-row form-choice table), one edge case, one Instead-of row, and the §2.6.2 quote + 4-shell repro to the existing source line. `last_verified` 2026-07-10 → 2026-08-05 | +| I2 (guardrail half) | `command-text-inspected-before-execution` already owns "a gate blocked a correct command"; its existing edge case covers exit-2-to-stderr | **Merged 3 edge cases**, none duplicating: blocked command emits *no side effect* so a poller waits forever; a path-scoped rule matches text not purpose (so the harness's own script is refused); Write succeeding where Bash was refused is gate *scope*. Cross-linked to the new orchestration page. `last_verified` 2026-07-30 → 2026-08-05 | +| I5 | `timeouts-and-retries` covers outbound calls, 429 + `Retry-After`, and concurrency caps — but not *client-side rate limiting to stay under a published quota*, and says nothing about auth-layer requests | **New page**, `related:` both ways. Its 429 row is referenced from the new page's retry edge case rather than restated | +| I6 | `testing/quality` holds `tests-that-cannot-fail`, `checks-that-cannot-pass`, `harness-reverse-controls` — all "your verification evidence is weaker than you claim" | **New page** in that category (same shape: *your grep evidence is weaker than you claim*). `related:` both ways with `tests-that-cannot-fail`, which its "suite is green" edge case defers to | +| I1 | `command-text-inspected-before-execution` (harness *gates* a command) and `non-interactive-cli-invocation` (harness *runs* a CLI) are adjacent but neither covers a harness *rewriting a result* | **New page**, cross-linked to both | +| I3 | Nothing in the wiki covers worker liveness/completion verdicts | **New page** (merged with I2's orchestration half — one case: a control signal is a hint, the primary artifact is the evidence) | +| I4 | `backend/common/concurrency/distributed-locks` and `jobs/scheduled-job-overlap` cover mutual exclusion between *processes of one service*, not *two orchestration runs over one repo* | **New page**, `related:`-linked to both rather than duplicating their lock guidance | + +**Conflicts flagged:** none. No new directive contradicts an existing page. The +one tension — I5's queued wording vs. the providers' actual bucket policies — was +a defect in the *candidate*, resolved by branching the directive (see I5 above), +not by overwriting anything. + +**Merge decision on I2+I3:** the queue carried these as two rows from two sessions, +but they are one case (a control signal is not evidence) with two failure +instances (a false "dead" verdict; a never-emitted status write). Per +`wiki-ingest` step 4 they went into one page rather than two near-duplicates. + +**Back-links added both ways:** `timeouts-and-retries` → `client-side-rate-limiting`; +`tests-that-cannot-fail` → `migration-call-site-survey`; +`command-text-inspected-before-execution` → both new orchestration and harness pages. ## Routing decision -- `backend/common/llm/` (new) — LLM-specific server concerns: completion-response-validation, - context-window-budget. Coherent home shared by #6 and #13. -- `backend/common/integrations/` (new) — general repo-external-dependency concern: - externally-owned-defaults. Kept separate from `llm/` because its scope is any - external resource (bucket/queue/index), not LLM-only. -- `backend/common/storage/` (new) — object-key-persistence. -- `qa/document-verification/` (new) — spec-document-gates, editing-a-gated-document. - Introduced by both #10 and #11; unified into one index section. -- `testing/quality/` (existing) — checks-that-cannot-pass, spec-artifact-checks, - harness-reverse-controls (test/check-authoring discipline, distinct from - qa/document-verification which is release-process gate design). -- `platforms/{environment,shells,processes}/` (existing) — unicode-text-matching, - command-text-inspected-before-execution, non-interactive-cli-invocation. -- `infrastructure/{containers,observability}/` (existing) — host-cgroup-visibility, - missing-container-metrics. - -Source PRs #6–#13 are closed with a disposition comment crediting the author. +| # | Insight | Target | New category? | +|---|---------|--------|---------------| +| I1 | Hook rewrites a tool's result | `platforms/tools/harness-mediated-tool-results` | No — closest fit | +| I2+I3 | Status/liveness verdict vs. primary artifact | `infrastructure/orchestration/control-signals-vs-primary-artifacts` | **Yes** | +| I4 | Shared orchestration state directory | `infrastructure/orchestration/shared-run-state` | **Yes** (same) | +| I5 | Throttle bypassed by the token request | `backend/common/reliability/client-side-rate-limiting` | No | +| I6 | Call-site survey for a signature migration | `testing/quality/migration-call-site-survey` | No | +| I7 | `${VAR:-}` vs `${VAR-}` | merged → `platforms/shells/portable-shell-scripts` | No | + +**New category `infrastructure/orchestration` — why the existing six don't fit.** +`ci-cd` is pipeline structure and build secrets; `deploy` is how a service reaches +production; `observability` is instrumenting a running service; `config` is +per-environment settings; `containers` and `data` are plainly unrelated. None +covers *several agent/worker sessions coordinating over one repository* — the +subject of three of the seven candidates, which is what justifies a category +rather than a lone page. `INDEX.md` and the domain index route line were both +updated so the category is reachable. + +**Routing note on I6.** The queue's `domain` hint said `testing`, and it landed in +`testing/quality` — but on the category's actual theme (*is your evidence as strong +as your claim?*) rather than on "it broke some tests". Its "load when" leads with +the signature-migration trigger so a migration task routes in without a test failure +having happened yet. + +**Routing note on I1.** `platforms` is nominally OS-level, but it already absorbs +agent-harness concerns (`command-text-inspected-before-execution`, +`non-interactive-cli-invocation`), so `tools` was taken as the closest fit rather +than opening a second new category. The domain's route line now names it. + +## Lint + +Run against the `AGENTS.md` maintenance invariants across all 7 touched pages: + +- Body lines: 73 / 88 / 82 / 83 / 85 / 80 / 85 — all ≤ 120 ✅ +- Banned vague qualifiers (`usually`, `consider`, `generally`, `typically`, …): **0** ✅ +- `id:` matches file path on every page ✅ +- Every `related:` id resolves — re-checked **repo-wide**, not just on new pages ✅ +- Every inline `[page-id]` reference resolves ✅ +- All 5 new pages listed in their domain `index.md` with a "load when" line ✅ +- New category reachable from `INDEX.md` ✅ +- `log.md`: 1 `ingest` + 2 `revise` entries appended ✅ +- Prohibitions appear only in `Instead of` rows, each paired with a replacement ✅ + +## Queue + +The 7 flushed rows are appended to `~/.dev-loop/queue/.processed.jsonl` and removed +from their 5 session files, so the next flush does not re-ingest them. diff --git a/INDEX.md b/INDEX.md index d6d4238..f5363ba 100644 --- a/INDEX.md +++ b/INDEX.md @@ -12,12 +12,12 @@ follow the cross-pointers in their index or take the next matching seeded domain | [databases](wiki/databases/index.md) | **seeded** | Designing schemas/tables/keys, choosing or evaluating indexes, writing or optimizing queries, choosing transaction/isolation behavior | | [backend](wiki/backend/index.md) | **seeded** | Server-side application code — language-agnostic (`common/`: API contracts, idempotency, JWT, timeouts/retries, caching, jobs, transactions in app code, shared state/pools, errors, LLM completion validation & context budgeting, consuming external-API responses, externally-owned defaults, object-storage references) plus stack subtrees: `java/` (JPA, Spring proxies, JVM threads/memory), `node/` (event loop, promises, runtime validation, shutdown), `python/` (GIL/asyncio, pydantic, WSGI/ASGI workers, language traps) | | [frontend](wiki/frontend/index.md) | **seeded** | Web UI code: state placement, rendering performance, in-UI data fetching (races, infinite scroll), auth token handling, forms, XSS-safe output, accessibility | -| [infrastructure](wiki/infrastructure/index.md) | **seeded** | CI/CD pipelines, secrets in build/deploy, container image builds, rollout/rollback strategy, observability (logs/metrics/alerting) | +| [infrastructure](wiki/infrastructure/index.md) | **seeded** | CI/CD pipelines, secrets in build/deploy, container image builds, rollout/rollback strategy, observability (logs/metrics/alerting), orchestrating parallel worker sessions over one repository (status signals, liveness verdicts, run-scoped shared state) | | [testing](wiki/testing/index.md) | **seeded** | Writing or structuring automated tests: level choice, cases/assertions, test data, mock decisions, flaky tests (release-process quality → qa) | | [qa](wiki/qa/index.md) | **seeded** | Release-quality process: release gates, regression scoping, bug reports, severity/priority triage, exploratory testing, automated verification of document deliverables (spec/RFC gates) (writing automated test code → testing) | | [debugging](wiki/debugging/index.md) | **seeded** | Diagnosing a failure — finding what is wrong and why: reproducing, bisection, hypothesis testing, traces/logs, intermittent failures (fixing the diagnosed fault → its owning domain) | | [security](wiki/security/index.md) | **seeded** | Trust-boundary decisions: input validation, session-vs-token auth choice, per-resource authorization (IDOR), secrets hygiene, dependency trust, PII handling (XSS rendering → frontend; CI secrets → infrastructure; JWT implementation → backend/frontend auth) | -| [platforms](wiki/platforms/index.md) | **seeded** | OS-level differences breaking code across macOS/Linux/Windows: shell portability, BSD-vs-GNU CLI, filesystem case/line endings, Unicode normalization in text/file-name matching, commands inspected before execution, background services/cron, invoking prompt-capable CLIs non-interactively, toolchain version pinning | +| [platforms](wiki/platforms/index.md) | **seeded** | OS-level differences breaking code across macOS/Linux/Windows: shell portability, BSD-vs-GNU CLI, filesystem case/line endings, Unicode normalization in text/file-name matching, commands inspected before execution, agent-harness hooks that rewrite tool results, background services/cron, invoking prompt-capable CLIs non-interactively, toolchain version pinning | | [mobile](wiki/mobile/index.md) | **seeded** | App-side iOS/Android/cross-platform: process death/state survival, offline-first sync, mobile-network calls, store rollout/hotfix strategy, startup time | All ten domains are seeded. New categories grow via `skills/wiki-ingest/SKILL.md`. diff --git a/log.md b/log.md index 1c6293b..a2f2b8c 100644 --- a/log.md +++ b/log.md @@ -37,3 +37,6 @@ Append-only. Format: `## [YYYY-MM-DD] ` and `git status` in that worktree — the commit exists or it does not | +| "worker dead" | The substrate's own liveness call (runtime/task API, `tmux has-session -t `, container inspect) *plus* whether new commits appeared since the last check | +| "worker produced file X" | `test -e`/`stat` on X | +| "nothing happened yet" | Worker log mtime; an idle-looking worker mid-run and a worker that never started look identical from the status file | + +2. **Emit and consume status by an exact identifier the orchestrator assigned.** + Have the worker write the run-scoped task id the orchestrator gave it, not an + identifier it discovers from its environment. A discovered identifier can name + a different process: `tmux display-message -p '#S'` run with `TMUX` unset does + not fail — it resolves against the server's current/most-recently-used session + and prints an unrelated session's name with exit 0. + +3. **Verify the status write landed, in the same step that writes it.** After + calling the status script, `stat` the status file itself and compare its + mtime. A command blocked by a policy gate produces no side effect, and a + guardrail scoped to an absolute path fires on the command *text* regardless of + the command's purpose — including the orchestration harness's own scripts. + +4. **When the status write was blocked, report the signal as un-emitted and hand + it back to the orchestrator.** Say which path was refused and by which rule. + The orchestrator can re-route the signal; a worker that retries or works around + a path guardrail defeats the isolation the worktree exists to provide. + +5. **Give a "dead worker" verdict a second, substrate-specific witness before + acting on it.** Restarting or discarding live work is unrecoverable in a way + that waiting one more interval is not. + +## Edge cases + +| Case | Then | +|------|------| +| Monitor exit code says dead, substrate says alive | Believe the substrate and keep waiting. Log the disagreement with both outputs — a monitor keyed on the wrong identifier repeats the verdict every interval | +| Status file is absent | Distinguish "never written" from "write blocked": check the worker's transcript for a gate/escalation message before assuming the worker never started | +| Worker ran the status script and saw no output | No output is not success. A blocked Bash command and a silent successful run are indistinguishable on stdout; only the file's existence separates them | +| An editor/Write tool succeeds where Bash was refused for the same path | The guardrail inspects Bash command text only, so the two channels disagree by design. Do not use the working channel to route around the rule — report it | +| Heartbeat is fresh but no commits in N intervals | Fresh heartbeat plus unchanged primary artifact is the stalled state, distinct from alive-and-progressing and from dead; handle it as its own case | +| The monitor is the only thing that can see the worker | Add the primary-artifact check to the monitor rather than trusting its verdict; a monitor with no artifact check cannot produce evidence | + +## Instead of + +| If you are about to | Do this instead | Why | +|---------------------|-----------------|-----| +| Restart a worker because the watch script exited "dead" | Run the substrate liveness call and `git log` on its branch first | The verdict can come from an identifier the worker picked up from its environment, naming a different process entirely | +| Let a worker resolve its own session/pane id for the status file | Pass the orchestrator-assigned task id into the worker and have it echo that back | Discovered identifiers are ambient state; assigned ids are the orchestrator's own namespace | +| Trust a status script's silence as "signal sent" | `stat` the status file after the call and report the mtime | A gate-blocked command exits without writing and without printing | +| Retry a blocked status write with a rewritten path | Report the block and the rule id to the orchestrator | The rule is protecting the shared checkout; a workaround reintroduces exactly the corruption it prevents | + +## Sources + +- https://man.openbsd.org/tmux — target-session resolution: "if a session is omitted, the current session is used if available; if no current session is available, the most recently used is chosen" — so a session-name query from outside any session still returns a name. Reproduced 2026-08-05: `env -u TMUX tmux display-message -p '#S'` printed an unrelated session name with exit 0 +- https://code.claude.com/docs/en/hooks — a `PreToolUse` hook runs "Before a tool call executes. Can block it"; a blocked call never executes, so it leaves no side effect +- https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html — command text is expanded by the shell when it processes the line, so a gate reading the unexecuted string matches on literal paths irrespective of the command's purpose + +## Field context + +Two verdicts from the same orchestration run, 2026-08-04/05. (1) A watch script +returned exit 3 ("dead worker") three times; each time the substrate's own +liveness check returned alive and `git log` showed real commits on the task +branch. Cause: the worker's status script resolved its session via +`tmux display-message -p '#S'`, which returned unrelated session names +(`lo-14/15/16-npmcli`), so the monitor was polling the wrong session's existence. +(2) A worker running the orchestrator's `status-update.sh` with the status +directory pointing at the main checkout was refused by a `worktree_escape` +guardrail (`plugins/guardrails/hooks/bash-guard.sh`), which fires when a linked +worktree's command text names the main root together with a write verb — +independent of the command's purpose. The status directory was then empty +(`total 0`) while the orchestrator kept polling. A Write-tool write to the same +path succeeded, confirming the block is Bash-command-text-scoped, not a +filesystem permission. diff --git a/wiki/infrastructure/orchestration/shared-run-state.md b/wiki/infrastructure/orchestration/shared-run-state.md new file mode 100644 index 0000000..46d9097 --- /dev/null +++ b/wiki/infrastructure/orchestration/shared-run-state.md @@ -0,0 +1,94 @@ +--- +id: infrastructure-orchestration-shared-run-state +domain: infrastructure +category: orchestration +applies_to: [general, git-worktree] +confidence: field-tested +sources: + - https://git-scm.com/docs/git-worktree + - https://man.openbsd.org/tmux +last_verified: 2026-08-05 +related: [infrastructure-orchestration-control-signals-vs-primary-artifacts, backend-common-concurrency-distributed-locks, backend-common-jobs-scheduled-job-overlap] +--- + +# Orchestration State Kept in a Shared Directory Inside the Repository + +## When this applies + +Several agent or worker sessions work in one repository at the same time and +coordinate through files in it — a status directory, task briefs, escalation +records, a lock or claim file — and the directory path is fixed rather than +per-run. Also when starting an orchestration in a repo that may already have one +running. + +## Do this + +1. **Namespace the state directory per run.** Give each orchestration a run id + generated at start and write everything under it: + `.orchestration//{status,briefs,escalations}/`. Watchers glob inside + their own run id only, so another run's writes cannot wake them. + +2. **Survey the state directory before writing anything into it.** List it and + compare against the task ids this run created. + +| What the survey finds | Do | +|-----------------------|----| +| Empty, or only this run's ids | Proceed | +| Ids this run did not create | Establish whether the other run is live (step 3) before creating any task | +| A run id whose tasks are all terminal and older than the session | Archive the directory, then proceed | + +3. **Confirm a foreign run is live from the repository itself, not from the state + files.** Check `git worktree list` for worktrees this run did not create, + `git branch --sort=-committerdate` for recent task branches, and the default + branch's HEAD against the commit this run started from. A stale directory and a + concurrently running orchestration look the same in the status files. + +4. **When a live foreign run is confirmed, stop and surface it.** Report the + overlapping task ids, the foreign worktrees, and whether the default branch has + moved. Two orchestrations decomposing the same goal produce two implementations + of the same work in different files, and the conflict appears at merge, after + both have paid for the work. + +5. **Record the run id in every artifact the run emits** — branch names + (`/`), worktree directory names, status files, and commit trailers + — so ownership is readable without opening the state directory. + +6. **Re-check the default branch's HEAD at the approval gate.** Compare it to the + value captured at start; when it moved, identify what merged before treating + your integration branch as current. + +## Edge cases + +| Case | Then | +|------|------| +| The state directory is committed to the repo | Add the run-id subtree to `.gitignore` and keep only the schema/README tracked; committed status files collide as merge conflicts on every worker branch | +| Two runs must share one repo deliberately | Give each its own run id and its own branch prefix, and require both to write only inside their own subtree; the repo is shared, the state is not | +| Your watcher woke on a signal for an unknown task id | Do not act on it. A wake is not ownership — filter to this run's task ids, then re-check whether a foreign run is live | +| A foreign run merged your task branches without your gate | Stop and reconstruct from `git reflog` and the default branch's history before continuing; the integration branch no longer reflects only your approvals | +| Worktrees were removed but their branches remain | `git worktree prune` clears the administrative files; the branches persist and still signal a prior run — read branch names for the run id | +| Run ids are generated from a timestamp at one-second resolution | Two runs starting in the same second collide; add a random suffix or the process id | + +## Instead of + +| If you are about to | Do this instead | Why | +|---------------------|-----------------|-----| +| Write status into a fixed `.orchestration/status/` path | Write into `.orchestration//status/` | A fixed path is shared mutable state between runs that never agreed to share it | +| Treat an unfamiliar task id in the state directory as leftover junk | Check worktrees, recent branches, and the default branch HEAD first | Stale state and a live concurrent run are indistinguishable from the files alone | +| Have your watcher act on every status file it sees | Filter to the task ids this run created | Otherwise another run's completion signal reads as your own task finishing | +| Continue after noticing the default branch moved | Identify what merged, then decide | Your integration branch may already be missing or duplicating merged work | + +## Sources + +- https://git-scm.com/docs/git-worktree — `git worktree list` enumerates every linked worktree of the repository, including those created by another process; `git worktree prune` removes administrative files for deleted worktrees +- https://man.openbsd.org/tmux — a session query resolves against the server's current or most-recently-used session, so ambient identifiers do not distinguish one run's workers from another's + +## Field context + +Observed 2026-08-04/05: two dev-loop orchestrations ran in one repository and +shared `.orchestration/status`. The watcher of run A woke on run B's `plan_ready` +signal. Run B then merged four of run A's task branches into its integration +branch and into the default branch (`373d9fa` → `24ea9e8`) without passing run A's +approval gate, and both runs were concurrently implementing the last task of the +same issue under different example file names. No status file distinguished the +two runs; the overlap was only visible from `git worktree list` and the branch +list. diff --git a/wiki/platforms/index.md b/wiki/platforms/index.md index b97fa71..53ca779 100644 --- a/wiki/platforms/index.md +++ b/wiki/platforms/index.md @@ -6,8 +6,11 @@ case/line-ending/path behavior, file permissions and exec bits across git/archives/containers, hidden environment inputs (timezone/locale, Unicode normalization form in text and file names, per-context PATH resolution), keeping processes alive as services or scheduled jobs, and -pinning toolchain versions across machines. Application logic stays in backend; -SQL stays in databases. +pinning toolchain versions across machines. Also the agent-harness layer that sits +between you and an operation: gates that read command text before execution, and +hooks that rewrite what a tool returns. Application logic stays in backend; +SQL stays in databases; coordinating several worker sessions stays in +infrastructure/orchestration. Match your situation to a "load when" line; load only matching pages. @@ -23,6 +26,7 @@ Match your situation to a "load when" line; load only matching pages. | Page | Load when | |------|-----------| | [bsd-vs-gnu-cli](tools/bsd-vs-gnu-cli.md) | A command works on Linux but fails on macOS or vice versa (`date`, `sed -i`, `timeout`, `seq`, `grep -P`, `readlink`, `stat`); writing a script or CI step that must run on both userlands; deciding whether to install GNU coreutils on macOS or write POSIX-only | +| [harness-mediated-tool-results](tools/harness-mediated-tool-results.md) | A file-read/search tool returns content that does not match the file — truncated to one line, summarized, or replaced by a note telling you to call something else; the tool's own suggested workaround returns the same thing; deciding what to put in a spawned worker's brief about reading files in that repo; judging whether a short tool result means a short file | ## environment diff --git a/wiki/platforms/shells/command-text-inspected-before-execution.md b/wiki/platforms/shells/command-text-inspected-before-execution.md index e69daff..80305dd 100644 --- a/wiki/platforms/shells/command-text-inspected-before-execution.md +++ b/wiki/platforms/shells/command-text-inspected-before-execution.md @@ -7,8 +7,8 @@ confidence: verified sources: - https://code.claude.com/docs/en/hooks - https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html -last_verified: 2026-07-30 -related: [platforms-shells-portable-shell-scripts, platforms-environment-path-resolution] +last_verified: 2026-08-05 +related: [platforms-shells-portable-shell-scripts, platforms-environment-path-resolution, infrastructure-orchestration-control-signals-vs-primary-artifacts, platforms-tools-harness-mediated-tool-results] --- # Commands Read as Text by a Gate Before the Shell Runs Them @@ -67,6 +67,9 @@ on the first attempt. | Path contains a space, so quoting is unavoidable | Relocate or symlink the target to a space-free path for gated commands; a gate that excludes quote characters cannot receive a quoted path at all | | The gate needs `~` expanded | Write the absolute path; a gate that resolves `~` itself is doing so on the literal tilde, which only works if it implements the expansion | | The same command must also be portable/robust as a script | Keep the gate-read argument literal and leave the rest of the script quoted normally ([platforms-shells-portable-shell-scripts]) — this page narrows one argument, it does not license unquoted expansions elsewhere | +| The blocked command was the one that emits your progress/status signal | A blocked command produces no side effect, so a consumer polling for that signal waits forever. `stat` the artifact the command was to write, and report the signal as un-emitted ([infrastructure-orchestration-control-signals-vs-primary-artifacts]) | +| A gate scoped to a path refuses the harness's own script | Path-scoped rules match the command text, not the command's purpose, so a tool's own helper is refused like any other write. Report the refused path and the rule id rather than rewriting the path to evade it | +| An editor/Write tool succeeds on the exact path Bash was refused for | The gate is registered for the Bash tool and does not see other tools' writes. Treat the disagreement as gate scope, not as permission to route around the rule | ## Instead of diff --git a/wiki/platforms/shells/portable-shell-scripts.md b/wiki/platforms/shells/portable-shell-scripts.md index 23eed9b..4650778 100644 --- a/wiki/platforms/shells/portable-shell-scripts.md +++ b/wiki/platforms/shells/portable-shell-scripts.md @@ -10,7 +10,7 @@ sources: - https://zsh.sourceforge.io/Doc/Release/Parameters.html - https://google.github.io/styleguide/shellguide.html - https://www.shellcheck.net/ -last_verified: 2026-07-10 +last_verified: 2026-08-05 related: [platforms-tools-bsd-vs-gnu-cli, platforms-toolchains-version-management, platforms-shells-command-text-inspected-before-execution] --- @@ -51,7 +51,20 @@ non-interactive environment). | Array indexing | 0-based | 1-based (unless `KSH_ARRAYS`) | Iterate with `"${arr[@]}"`; when a numeric index is unavoidable, gate the script to one shell via the shebang | 5. Build argument lists as arrays and expand quoted: `args=(-o "$out"); cmd "${args[@]}"`. -6. Run `shellcheck` on every script before it ships or gates anything; it flags +6. Choose the default-expansion form by what an empty value must mean. The colon + decides it: with the colon the test is "unset **or null**", without it the test + is "unset" only (POSIX 2.6.2). + +| You want | Write | `VAR=` (empty) yields | `VAR` unset yields | +|----------|-------|-----------------------|--------------------| +| An empty value to mean "caller supplied nothing" — a blank config field falls back | `${VAR:-default}` | `default` | `default` | +| An empty value to mean "caller deliberately set it empty" — passing `VAR=` turns the feature off | `${VAR-default}` | *(empty)* | `default` | + + When you cannot change the script and it reads `${VAR:-default}`, disable the + feature by supplying a value that fails the script's own validation + (`WATCH_TMUX=/nonexistent-tmux` makes the `command -v` check fail) rather than + an empty string, which the expansion replaces with the default. +7. Run `shellcheck` on every script before it ships or gates anything; it flags unquoted expansions, bashisms under `#!/bin/sh`, and `set -e` blind spots. ## Edge cases @@ -62,6 +75,7 @@ non-interactive environment). | Critical command is in a pipeline but the interpreter is POSIX sh (no `pipefail`) | Run the critical command outside the pipeline (temp file between stages) and test `$?` directly | | Script runs via cron/CI/hooks and commands are "not found" | Non-interactive shells load no rc files — no user PATH, no version-manager shims. Call binaries by absolute path (see platforms-toolchains-version-management) | | `set -u` breaks on optional variables | Expand with an explicit default: `"${OPT:-}"` | +| An env var you set to empty to switch a feature off has no effect | The script reads `${VAR:-default}`, which substitutes the default for empty as well as unset. Pass a value the script's own check rejects, or change the script to `${VAR-default}` | ## Instead of @@ -70,10 +84,11 @@ non-interactive environment). | Build a command string and `eval` it | Build an array and expand it: `cmd "${args[@]}"` | `eval` re-parses quotes and globs; arrays pass arguments through exactly | | Put a command plus its flags in one variable and run `$cmd` | Variable holds the binary path only; flags are separate words | zsh runs the whole value as one command name; bash re-splits and re-globs it | | Trust a trailing `echo "done"` as proof a step ran | Verify the produced state with an independent command | Inside `&&` chains and subshells, `set -e` misses failures and the echo still prints | +| Pass `VAR=` to turn off a feature a script reads as `${VAR:-default}` | Pass a value the script's validation rejects, or change the script to `${VAR-default}` | `:-` substitutes the default for an empty value too, so the feature stays on and the disable is silently ignored | ## Sources -- https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html — POSIX shell quoting and field splitting (sections 2.2, 2.6.5) +- https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html — POSIX shell quoting and field splitting (sections 2.2, 2.6.5); parameter expansion (2.6.2): "use of the <colon> in the format shall result in a test for a parameter that is unset or null; omission of the <colon> shall result in a test for a parameter that is only unset". Reproduced 2026-08-05 with `VAR=` under sh, bash, zsh, and dash: `${VAR:-d}` → `d`, `${VAR-d}` → empty in all four - https://zsh.sourceforge.io/Doc/Release/Expansion.html — zsh: no word splitting of unquoted parameters (14.3); `=word` expansion (14.7.3) - https://zsh.sourceforge.io/Doc/Release/Parameters.html — zsh arrays numbered from 1 (KSH_ARRAYS excepted) - https://google.github.io/styleguide/shellguide.html — quote variables, prefer bash for scripts, arrays over eval diff --git a/wiki/platforms/tools/harness-mediated-tool-results.md b/wiki/platforms/tools/harness-mediated-tool-results.md new file mode 100644 index 0000000..f7a28ac --- /dev/null +++ b/wiki/platforms/tools/harness-mediated-tool-results.md @@ -0,0 +1,84 @@ +--- +id: platforms-tools-harness-mediated-tool-results +domain: platforms +category: tools +applies_to: [claude-code, agent-harness] +confidence: verified +sources: + - https://code.claude.com/docs/en/hooks +last_verified: 2026-08-05 +related: [platforms-shells-command-text-inspected-before-execution, platforms-processes-non-interactive-cli-invocation, infrastructure-orchestration-control-signals-vs-primary-artifacts] +--- + +# A Plugin Rewrites What a Tool Returns Before the Agent Sees It + +## When this applies + +A file-reading or search tool returns content that does not match the file on +disk — truncated to the first line, summarized, or replaced by a note telling you +to call something else; a plugin/hook is installed in the session; or you are +about to brief worker sessions that will read files in the same repo. + +## Do this + +1. **Read the substitution as a hook, not as the file.** A `PostToolUse` hook + returns `hookSpecificOutput.updatedToolOutput`, which *replaces* the tool's + result before the model sees it; `PreToolUse` returns + `hookSpecificOutput.updatedInput`, which rewrites the arguments before the + tool runs. The docs name this the interception point "for redaction or + transformation use cases". The file is intact; the channel to it is mediated. + +2. **Test the suggested workaround exactly once.** The substituted text carries + the plugin author's remediation (`offset`/`limit`, an observation-fetch call). + Run it once against the same file. + +| Result of the one retry | Do | +|-------------------------|----| +| Returns the real content | Use the suggested form for the rest of the session | +| Returns the same substitution again | Switch to the shell path (step 3) and stop retrying the tool | + +3. **Reach the bytes through a channel the hook does not match.** Hooks are + registered per tool name, so a different tool is not intercepted: locate with + `grep -n '' `, then read the range with + `awk 'NR>=120 && NR<=240' `. Confirm the fallback works by checking the + output has more than one line before relying on it. + +4. **Put the resolved channel in the brief of every session you spawn against + that repo.** State the tool that is mediated, that its own remediation was + tested and failed, and the exact fallback command form. Each worker that + rediscovers this spends its own probe-retry-fallback round on it. + +5. **Record the file's real length once** (`wc -l`) so later range reads are + bounded by a number you measured, not by a truncated view. + +## Edge cases + +| Case | Then | +|------|------| +| Only some files return the substitution | The hook matched on a path pattern; treat the mediated set as the unit and use the fallback for the whole directory rather than per file | +| The substitution names a retrieval call (`get_observations`, a corpus search) | Run it once — when it answers the question, prefer it; the index is cheaper than the file. When it returns unrelated or stale content, fall back to the shell path | +| Output is plausible but shorter than the file | Do not treat it as the file. Compare against `wc -l` before quoting it in a review, a diff, or a claim about coverage | +| A second agent reports the tool working normally | Hook config is per settings scope (user/project/local); confirm which scope each session loaded before concluding the hook was removed | +| The tool is mediated but writes still land | `PostToolUse` runs after execution, so write tools take effect even when their reported result is rewritten — verify the write on disk, not from the returned text | + +## Instead of + +| If you are about to | Do this instead | Why | +|---------------------|-----------------|-----| +| Retry the tool a third time with new offsets | Switch to `grep -n` + `awk` after the first failed retry | The substitution is produced by a hook that does not inspect your arguments; more argument variants return the same text | +| Conclude the file is empty, one line, or missing | Check `wc -l` on the path | The one-line result is the hook's message, not the file's length | +| Let each spawned worker discover the mediation itself | Name the mediated tool and the fallback command in the brief | The cost is per agent otherwise, and each pays it before doing any real work | +| Disable the plugin to get a clean read | Use the unmediated tool for the read you need | The plugin serves the session's other work; a per-read fallback is reversible and scoped | + +## Sources + +- https://code.claude.com/docs/en/hooks — `PostToolUse` `hookSpecificOutput.updatedToolOutput` "replaces the tool's result"; `PreToolUse` `hookSpecificOutput.updatedInput` "replaces a tool's arguments before it runs"; "For redaction or transformation use cases, intercept at `PreToolUse` for outbound tool inputs and `PostToolUse` for inbound tool results"; `PostToolUse` fires after the tool has executed + +## Field context + +Observed 2026-08-05 in a repo with a session-memory plugin installed: `Read` on +four source files returned only line 1 plus a note suggesting `offset`/`limit`; +the suggested retry (`offset=151, limit=120`) returned line 1 again. Two +orchestrated worker sessions independently logged the same interception and each +fell back to `cat -n`/`sed` on its own, having spent three to four tool calls +apiece rediscovering it. diff --git a/wiki/testing/index.md b/wiki/testing/index.md index 2cbad76..f4424e3 100644 --- a/wiki/testing/index.md +++ b/wiki/testing/index.md @@ -25,6 +25,7 @@ Match your situation to a "load when" line; load only matching pages. | [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 | | [checks-that-cannot-pass](quality/checks-that-cannot-pass.md) | Authoring a check whose target does not exist yet (grep/regex gate on an unwritten file or doc section, lint/scan rule, schema assertion on an unbuilt endpoint, a plan's verification command) and it has only ever been observed failing; reviewing a plan's gates before adopting them; separating "target missing" from "content missing" in a gate's exit status | | [spec-artifact-checks](quality/spec-artifact-checks.md) | Writing or reviewing an automated check that a mapping table covers every rule/field/enum case, or that ids resolve across documents; deciding whether a green check earned "verified" or only "present"; designing one negative control per check in a multi-check harness; parsing Markdown table rows programmatically in a doc-as-spec repo | +| [migration-call-site-survey](quality/migration-call-site-survey.md) | Changing a function/constructor signature or the shape of a value it takes and updating every caller; about to report a call-site count from a text search ("13 sites, 7 updated"); reviewing a migration PR that claims full coverage; the suite broke in one file after a migration you believed complete; deciding whether test helpers/factories that build the old shape are in scope | | [harness-reverse-controls](quality/harness-reverse-controls.md) | You built a harness that scores how well something is verified (mutation run, doc/spec gate suite, CI check matrix) and are about to cite its score in a commit, PR, README, or report; its verdicts come out uniform (every case caught, or every case green); deciding what control run proves the harness discriminates, how to score errored/never-ran cases, and what the harness's isolated working tree must contain | ## data diff --git a/wiki/testing/quality/migration-call-site-survey.md b/wiki/testing/quality/migration-call-site-survey.md new file mode 100644 index 0000000..3d3aee0 --- /dev/null +++ b/wiki/testing/quality/migration-call-site-survey.md @@ -0,0 +1,98 @@ +--- +id: testing-quality-migration-call-site-survey +domain: testing +category: quality +applies_to: [general, python, javascript, typescript] +confidence: verified +sources: + - https://docs.python.org/3/tutorial/controlflow.html#keyword-arguments + - https://deepwiki.com/python-rope/rope/4.6-change-signature-and-other-refactorings + - https://www.jetbrains.com/help/resharper/Refactorings__Change_Signature.html +last_verified: 2026-08-05 +related: [testing-quality-tests-that-cannot-fail, testing-quality-checks-that-cannot-pass, testing-data-test-data-and-isolation, debugging-methodology-verify-the-fix] +--- + +# Claiming a Signature Change Covered Every Call Site, Based on a Text Search + +## When this applies + +You are changing a function/constructor signature or the shape of a structure it +takes, must update every caller, and you surveyed the callers with a text search. +Also when reviewing a migration PR whose description states a call-site count +("13 sites, 7 updated"). + +## Do this + +1. **Enumerate by the callee, not by the parameter.** Search for the thing that + cannot be omitted at a call site — `verify(`, `Interpreter(`, `ClassName(` — + and read each hit. A parameter name appears in the call text only when the + caller chose keyword form; positional callers are invisible to a + parameter-name search, and the language permits either form for the same + parameter. + +2. **Take the count from the callee search, and treat any parameter-name search as + a subset.** When the two counts differ, the difference is positional callers — + the ones the migration would otherwise miss. + +3. **Audit helper/factory definitions in the same sweep.** Search the test tree + for functions that *construct* the old shape (`def rows_for`, `build_x`, + `make_fixture`) as well as ones that call the target. A helper appears once in a + call-site search yet supplies the old structure to every test that uses it, so + one missed helper reproduces the old contract across many callers. + +4. **Cover the call forms the language allows, per callee:** + +| Call form | Found by | Also search | +|-----------|----------|-------------| +| `verify(a, b, seed)` positional | `verify(` | — | +| `verify(seed=...)` keyword | `verify(` and `seed=` | — | +| `verify(*args)` / `verify(**kwargs)` unpacked | `verify(` | The construction of `args`/`kwargs` at its assignment | +| Bound/aliased: `f = verify` then `f(...)` | Neither reliably | `= verify`, `import verify`, and the alias name | +| Subclass override / interface implementation | Neither | The base declaration and `class .*\(.*Base` implementors | + +5. **Run the full suite before claiming the migration is complete, and read which + files fail.** A survey is a hypothesis about coverage; the suite is the + measurement. Failures clustered in one file name the call form the survey + missed, which tells you what to re-search rather than only what to patch. + +6. **When the language and tooling support it, prefer a symbol-aware rename** — an + IDE/refactoring-library Change Signature resolves call sites through the symbol + and rewrites positional and keyword forms alike. Use the text search to + *verify* its result, not to plan the migration. + +## Edge cases + +| Case | Then | +|------|------| +| The callee's name is short or common (`get`, `run`, `verify`) | Qualify by module or receiver (`mod.verify(`, `self.verify(`), then widen; a bare short name buries real hits in noise | +| Callers live outside this repo | The signature is a published contract — version it and keep the old form accepting both shapes for one release rather than migrating in place | +| A test helper produces the old shape *and* the code under test still accepts it | Both pass and hide the gap. Delete the old-shape path from the callee first, so every stale producer fails loudly | +| The suite is green after the change | Confirm the tests would fail without it — a helper that silently normalizes the shape makes them unable to detect the migration at all ([testing-quality-tests-that-cannot-fail]) | +| Dynamic dispatch (`getattr`, a registry dict, DI container) | Text search cannot resolve these; search the registration site and enumerate registered names | +| The change only adds a parameter with a default | Positional callers still break when the new parameter is inserted before an existing one — order matters, not just arity | + +## Instead of + +| If you are about to | Do this instead | Why | +|---------------------|-----------------|-----| +| Grep for the parameter name to find call sites | Grep for the callee name and read each hit | Positional call sites contain no parameter name and never appear in that search | +| Report "N call sites, M migrated" from a text search | Report the count from the callee search and confirm with a full suite run | The parameter-name count is a subset; the difference is exactly the sites that will break | +| Skip test helpers because they are "just fixtures" | Search helper definitions for the old shape in the same pass | One helper feeds the old structure to every test that calls it, from a single search hit | +| Call the migration done when the changed tests pass | Run the whole suite and read the failing file names | The failures identify the call form the survey missed, which is the information needed to finish | + +## Sources + +- https://docs.python.org/3/tutorial/controlflow.html#keyword-arguments — a function may be called with positional or keyword arguments for the same parameters, so the parameter name is absent from the text of a positional call +- https://deepwiki.com/python-rope/rope/4.6-change-signature-and-other-refactorings — rope's ChangeSignature updates call sites by resolving the callee symbol and normalizing argument forms (positional↔keyword), rather than by matching parameter-name text +- https://www.jetbrains.com/help/resharper/Refactorings__Change_Signature.html — Change Signature "finds and updates all usages, base symbols, implementations, and overrides of the modified symbol", including overrides that a call-site text search does not reach + +## Field context + +Measured 2026-08-05. A reconnaissance search on the parameter name +(`grep -rn "repo_rows" impl/tests/`) returned 13 hits, all keyword-form, and the +migration was scoped as "7 of 13". The full suite then reported +`Ran 472 tests / FAILED (failures=11)`, every failure in one file that passed the +value as the fourth *positional* argument to `verify()`. Re-searching by callee +(`grep -n "verify(" impl/tests/test_backend.py`) surfaced 8 positional call sites +plus a `rows_for()` helper definition that was still generating the old rule shape +for 5 further call sites — none of which the parameter-name search could reach. diff --git a/wiki/testing/quality/tests-that-cannot-fail.md b/wiki/testing/quality/tests-that-cannot-fail.md index 884ff61..9374280 100644 --- a/wiki/testing/quality/tests-that-cannot-fail.md +++ b/wiki/testing/quality/tests-that-cannot-fail.md @@ -11,7 +11,7 @@ sources: - https://martinfowler.com/bliki/TestCoverage.html - https://testing.googleblog.com/2013/05/testing-on-toilet-dont-overuse-mocks.html last_verified: 2026-07-10 -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, qa-document-verification-spec-document-gates] +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, qa-document-verification-spec-document-gates, testing-quality-migration-call-site-survey] --- # Proving a Test Can Fail