[object Object]

← back to Cli Printing Press

fix(cli): retro #301 — six improvements from recipe-goat regenerate (#303)

8aad8d858bf7c7e3d7036ad433323dd26b0bdbb2 · 2026-04-26 02:22:31 -0700 · Trevin Chow

* fix(cli): skip flag-descriptor tokens in positional-arg inference

`Use` strings of the form "save <url> [--tags=<csv>] [--stdin]" were
leaking `<csv>` as a phantom positional via inferPositionalArgs, which
then breaks `cobra.MaximumNArgs(1)` validators on commands that accept
exactly one real positional. The placeholder regex couldn't tell
`[--tags=<csv>]` (a flag descriptor) apart from `[id]` (an optional
positional).

Refactor placeholder extraction into a pure function and prefilter
bracketed tokens whose body starts with leading dashes or contains `=`
before the placeholder regex runs. Tested across simple positionals,
flag descriptors with embedded placeholders, short-form flags, and
spaced flag bodies.

Surfaced during the recipe-goat regenerate (PR mvanhorn/printing-press-library#127); part of retro #301 finding F2.

* fix(cli): walk AddCommand graph instead of guessing via Use-string specificity

The verify-skill flag-commands check was producing false-positive
"--<flag> is declared elsewhere but not on <command>" findings whenever
two cobra commands at different paths shared a leaf name (e.g.,
`recipe-goat-pp-cli save <url>` plus `recipe-goat-pp-cli profile save
<name>`). The old find_command_source picked candidates by counting
positional/optional/variadic tokens in the `Use:` string and returned
only the highest-specificity tier — which dropped the lower-specificity
file from the flag-declaration union, causing real flags on the dropped
command to be reported as missing.

Replace the specificity heuristic with a walk of `rootCmd.AddCommand`
calls plus the `<parent>.AddCommand(newXxxCmd(...))` chain inside each
constructor function. The graph walk picks exactly one file per command
path with no false positives. Falls back to the legacy specificity
behavior for unconventional CLIs that don't follow the standard
`func newXxxCmd` factory pattern.

Sync internal/cli/verify_skill_bundled.py from the canonical
scripts/verify-skill/verify_skill.py. Add a Go regression test
(TestVerifySkill_NoFalsePositiveOnSharedLeafName) that materializes a
synthetic CLI with the F1 shape and asserts "All checks passed". Add
Python unit tests for collect_command_constructors, find_root_children,
resolve_command_path, find_command_source's new path, and
_extract_function_body's brace-tracking through strings/comments.

Surfaced during the recipe-goat regenerate (PR
mvanhorn/printing-press-library#127); part of retro #301 finding F1.

* fix(cli): carve out novel-static-reference commands from reimplementation check

Novel features that ship curated static data (substitution tables,
holiday lists, currency metadata, conversion factors) have no API
client call and no store call by design — the data IS the feature.
The dogfood reimplementation_check was flagging these as
"hand-rolled response: no API client call, no store access" because
its only carve-outs were store-signal and client-signal.

Add a third carve-out: a per-command opt-out marker
`// pp:novel-static-reference` anywhere in the file. When the marker
is present, classifyReimplementation exempts the command on the same
footing as the store/client carve-outs.

Tests verify both directions:
- TestCheckReimplementation_NovelStaticReferenceMarker_Exempted: file
  with the marker is exempted even with no client/store signals.
- TestCheckReimplementation_WithoutMarker_StillFlagged: same file shape
  without the marker is still flagged with the existing reason.

Document the new annotation in AGENTS.md alongside the existing
Anti-Reimplementation block.

Surfaced during the recipe-goat regenerate (PR
mvanhorn/printing-press-library#127); part of retro #301 finding F3.

* feat(cli): add cliutil.ProbeReachable helper for multi-source health checks

Multi-source CLIs (recipe-goat, movie-goat, contact-goat, weather-goat,
flightgoat) keep rediscovering that some CDN-fronted hosts (BBC,
RecipeTin Eats, AllRecipes, Serious Eats, The Kitchn) terminate HEAD
requests with TLS shutdown / EOF even though they serve GET cleanly.
Authors hand-roll HEAD-based probes that lie — reporting unreachable
for hosts the real fetch path is successfully scraping. recipe-goat
hit this directly: doctor reported six sites unreachable that the goat
ranker was using fine.

Add a generator template emitting internal/cliutil/probe.go in every
printed CLI:

  ProbeReachable(ctx, client, url) (status, code int, err error)

Implementation: GET with Range: bytes=0-1023, drain up to 2 KiB into
io.Discard, classify the result.

  - 200/206 → reachable (host honors Range, full or partial body)
  - 416 → reachable (host doesn't support Range, but headers came back
    so the host is up)
  - other 4xx/5xx → blocked (host is up but refusing this request)
  - network-layer failures → unreachable

Doctor commands doing per-source fan-out should call ProbeReachable
with the same client used by the real fetch path so probe drift can't
recur.

Tests cover all five status paths plus the nil-client fallback and a
header-sent assertion. Verified end-to-end by generating a synthetic
CLI from a probe-enabled spec and running the emitted cliutil tests.

Surfaced during the recipe-goat regenerate (PR
mvanhorn/printing-press-library#127); part of retro #301 finding F4.

* docs(skills): re-validate prior research after machine upgrades

When the user picks 'Generate a fresh CLI' on an existing library
entry whose .printing-press.json was stamped by an older binary
(differing minor or major version), prompt once before kicking off
Phase 1 research. The prompt names the high-impact machine deltas
that commonly invalidate prior briefs (Surf-Chrome HTTP transport,
MCP intent tools, scoring rubrics, auth modes) and asks whether to
re-validate or reuse the prior brief verbatim. Skips on patch deltas,
same-version regenerations, and first generations.

Recipe-goat hit this directly: the 2026-04-13 brief framed
reachability as Tier 1/2/3 — a pre-Surf classification that became
obsolete when the binary gained Surf-Chrome impersonation in 2.x.
The user had to manually prompt 'validate Tier 2/3 status with surf'
mid-Phase-4 to catch it; with this guardrail, the skill prompts at
the start instead.

Surfaced during the recipe-goat regenerate (PR
mvanhorn/printing-press-library#127); part of retro #301 finding F5.

* docs(skills): warn against hardcoded runtime-state counts in narrative

Add rule 11 to the Phase 1 narrative-block authoring rules: avoid
hardcoded counts in headline / value_prop when the count tracks a
runtime list (Sites slice, retailer list, league list, vendor list).
A phrase like 'across 15 trusted sites' embeds the integer into
root.go's Short/Long, README, SKILL.md, the MCP tools description,
and which.go simultaneously — and when the underlying registry grows,
a single-line addition leaves ~10 hardcoded copies stale.

Prefer plural-without-count phrasing ('across the major recipe
sites'), a qualitative descriptor ('dozens of vendors'), or — if a
count genuinely matters — a {{ N }} placeholder with a human-facing
comment naming which runtime list it tracks.

Recipe-goat's regenerate ended up fixing 'across 15 trusted sites' →
'across 37 trusted sites' in 11 places (root.go Short, root.go Long
banner, root.go highlights bullet, README, SKILL line 11, SKILL line
133, .goreleaser.yaml, spec.yaml, tools-manifest.json, MCP tools.go,
agent_context.go, which.go) — work the original brief could have
avoided by writing a count-free narrative.

Surfaced during the recipe-goat regenerate (PR
mvanhorn/printing-press-library#127); part of retro #301 finding F6.

Files touched

Diff

commit 8aad8d858bf7c7e3d7036ad433323dd26b0bdbb2
Author: Trevin Chow <trevin@trevinchow.com>
Date:   Sun Apr 26 02:22:31 2026 -0700

    fix(cli): retro #301 — six improvements from recipe-goat regenerate (#303)
    
    * fix(cli): skip flag-descriptor tokens in positional-arg inference
    
    `Use` strings of the form "save <url> [--tags=<csv>] [--stdin]" were
    leaking `<csv>` as a phantom positional via inferPositionalArgs, which
    then breaks `cobra.MaximumNArgs(1)` validators on commands that accept
    exactly one real positional. The placeholder regex couldn't tell
    `[--tags=<csv>]` (a flag descriptor) apart from `[id]` (an optional
    positional).
    
    Refactor placeholder extraction into a pure function and prefilter
    bracketed tokens whose body starts with leading dashes or contains `=`
    before the placeholder regex runs. Tested across simple positionals,
    flag descriptors with embedded placeholders, short-form flags, and
    spaced flag bodies.
    
    Surfaced during the recipe-goat regenerate (PR mvanhorn/printing-press-library#127); part of retro #301 finding F2.
    
    * fix(cli): walk AddCommand graph instead of guessing via Use-string specificity
    
    The verify-skill flag-commands check was producing false-positive
    "--<flag> is declared elsewhere but not on <command>" findings whenever
    two cobra commands at different paths shared a leaf name (e.g.,
    `recipe-goat-pp-cli save <url>` plus `recipe-goat-pp-cli profile save
    <name>`). The old find_command_source picked candidates by counting
    positional/optional/variadic tokens in the `Use:` string and returned
    only the highest-specificity tier — which dropped the lower-specificity
    file from the flag-declaration union, causing real flags on the dropped
    command to be reported as missing.
    
    Replace the specificity heuristic with a walk of `rootCmd.AddCommand`
    calls plus the `<parent>.AddCommand(newXxxCmd(...))` chain inside each
    constructor function. The graph walk picks exactly one file per command
    path with no false positives. Falls back to the legacy specificity
    behavior for unconventional CLIs that don't follow the standard
    `func newXxxCmd` factory pattern.
    
    Sync internal/cli/verify_skill_bundled.py from the canonical
    scripts/verify-skill/verify_skill.py. Add a Go regression test
    (TestVerifySkill_NoFalsePositiveOnSharedLeafName) that materializes a
    synthetic CLI with the F1 shape and asserts "All checks passed". Add
    Python unit tests for collect_command_constructors, find_root_children,
    resolve_command_path, find_command_source's new path, and
    _extract_function_body's brace-tracking through strings/comments.
    
    Surfaced during the recipe-goat regenerate (PR
    mvanhorn/printing-press-library#127); part of retro #301 finding F1.
    
    * fix(cli): carve out novel-static-reference commands from reimplementation check
    
    Novel features that ship curated static data (substitution tables,
    holiday lists, currency metadata, conversion factors) have no API
    client call and no store call by design — the data IS the feature.
    The dogfood reimplementation_check was flagging these as
    "hand-rolled response: no API client call, no store access" because
    its only carve-outs were store-signal and client-signal.
    
    Add a third carve-out: a per-command opt-out marker
    `// pp:novel-static-reference` anywhere in the file. When the marker
    is present, classifyReimplementation exempts the command on the same
    footing as the store/client carve-outs.
    
    Tests verify both directions:
    - TestCheckReimplementation_NovelStaticReferenceMarker_Exempted: file
      with the marker is exempted even with no client/store signals.
    - TestCheckReimplementation_WithoutMarker_StillFlagged: same file shape
      without the marker is still flagged with the existing reason.
    
    Document the new annotation in AGENTS.md alongside the existing
    Anti-Reimplementation block.
    
    Surfaced during the recipe-goat regenerate (PR
    mvanhorn/printing-press-library#127); part of retro #301 finding F3.
    
    * feat(cli): add cliutil.ProbeReachable helper for multi-source health checks
    
    Multi-source CLIs (recipe-goat, movie-goat, contact-goat, weather-goat,
    flightgoat) keep rediscovering that some CDN-fronted hosts (BBC,
    RecipeTin Eats, AllRecipes, Serious Eats, The Kitchn) terminate HEAD
    requests with TLS shutdown / EOF even though they serve GET cleanly.
    Authors hand-roll HEAD-based probes that lie — reporting unreachable
    for hosts the real fetch path is successfully scraping. recipe-goat
    hit this directly: doctor reported six sites unreachable that the goat
    ranker was using fine.
    
    Add a generator template emitting internal/cliutil/probe.go in every
    printed CLI:
    
      ProbeReachable(ctx, client, url) (status, code int, err error)
    
    Implementation: GET with Range: bytes=0-1023, drain up to 2 KiB into
    io.Discard, classify the result.
    
      - 200/206 → reachable (host honors Range, full or partial body)
      - 416 → reachable (host doesn't support Range, but headers came back
        so the host is up)
      - other 4xx/5xx → blocked (host is up but refusing this request)
      - network-layer failures → unreachable
    
    Doctor commands doing per-source fan-out should call ProbeReachable
    with the same client used by the real fetch path so probe drift can't
    recur.
    
    Tests cover all five status paths plus the nil-client fallback and a
    header-sent assertion. Verified end-to-end by generating a synthetic
    CLI from a probe-enabled spec and running the emitted cliutil tests.
    
    Surfaced during the recipe-goat regenerate (PR
    mvanhorn/printing-press-library#127); part of retro #301 finding F4.
    
    * docs(skills): re-validate prior research after machine upgrades
    
    When the user picks 'Generate a fresh CLI' on an existing library
    entry whose .printing-press.json was stamped by an older binary
    (differing minor or major version), prompt once before kicking off
    Phase 1 research. The prompt names the high-impact machine deltas
    that commonly invalidate prior briefs (Surf-Chrome HTTP transport,
    MCP intent tools, scoring rubrics, auth modes) and asks whether to
    re-validate or reuse the prior brief verbatim. Skips on patch deltas,
    same-version regenerations, and first generations.
    
    Recipe-goat hit this directly: the 2026-04-13 brief framed
    reachability as Tier 1/2/3 — a pre-Surf classification that became
    obsolete when the binary gained Surf-Chrome impersonation in 2.x.
    The user had to manually prompt 'validate Tier 2/3 status with surf'
    mid-Phase-4 to catch it; with this guardrail, the skill prompts at
    the start instead.
    
    Surfaced during the recipe-goat regenerate (PR
    mvanhorn/printing-press-library#127); part of retro #301 finding F5.
    
    * docs(skills): warn against hardcoded runtime-state counts in narrative
    
    Add rule 11 to the Phase 1 narrative-block authoring rules: avoid
    hardcoded counts in headline / value_prop when the count tracks a
    runtime list (Sites slice, retailer list, league list, vendor list).
    A phrase like 'across 15 trusted sites' embeds the integer into
    root.go's Short/Long, README, SKILL.md, the MCP tools description,
    and which.go simultaneously — and when the underlying registry grows,
    a single-line addition leaves ~10 hardcoded copies stale.
    
    Prefer plural-without-count phrasing ('across the major recipe
    sites'), a qualitative descriptor ('dozens of vendors'), or — if a
    count genuinely matters — a {{ N }} placeholder with a human-facing
    comment naming which runtime list it tracks.
    
    Recipe-goat's regenerate ended up fixing 'across 15 trusted sites' →
    'across 37 trusted sites' in 11 places (root.go Short, root.go Long
    banner, root.go highlights bullet, README, SKILL line 11, SKILL line
    133, .goreleaser.yaml, spec.yaml, tools-manifest.json, MCP tools.go,
    agent_context.go, which.go) — work the original brief could have
    avoided by writing a count-free narrative.
    
    Surfaced during the recipe-goat regenerate (PR
    mvanhorn/printing-press-library#127); part of retro #301 finding F6.
---
 AGENTS.md                                          |   5 +-
 internal/cli/verify_skill_bundled.py               | 263 ++++++++++++++++++++-
 internal/cli/verify_skill_test.go                  |  74 ++++++
 internal/generator/generator.go                    |   1 +
 internal/generator/generator_test.go               |  58 ++++-
 internal/generator/templates/cliutil_probe.go.tmpl | 104 ++++++++
 internal/generator/templates/cliutil_test.go.tmpl  | 185 +++++++++++++++
 internal/pipeline/reimplementation_check.go        |  75 ++++--
 internal/pipeline/reimplementation_check_test.go   | 102 ++++++++
 internal/pipeline/runtime.go                       |  41 +++-
 internal/pipeline/runtime_test.go                  |  70 ++++++
 scripts/verify-skill/test_resolve_command_path.py  | 242 +++++++++++++++++++
 scripts/verify-skill/verify_skill.py               | 263 ++++++++++++++++++++-
 skills/printing-press/SKILL.md                     |  23 ++
 14 files changed, 1445 insertions(+), 61 deletions(-)

diff --git a/AGENTS.md b/AGENTS.md
index 7703dce1..d4d79c72 100644
--- a/AGENTS.md
+++ b/AGENTS.md
@@ -28,12 +28,13 @@ Concretely, the generator and review loop reject:
 - Aggregations computed in-process when the API has an aggregation endpoint
 - Enum mappings and reference data synthesized locally when the API returns them
 
-Two carve-outs are legitimate:
+Three carve-outs are legitimate:
 
 - Commands that read from the generated `internal/store` package to join or query sync'd data (the `stale`, `bottleneck`, `health`, `reconcile` family). These are local-data commands, not fake API calls.
 - Commands that cache an API response in the store after calling it. Presence of both a client call and a store call is fine.
+- Commands whose data is the curated content itself — substitution tables, holiday lists, currency metadata, conversion factors. The data IS the feature; calling an API or hitting the store would be wrong. Opt in by adding the directive `// pp:novel-static-reference` anywhere in the command's source file (typically near the package-level data declaration). The reimplementation check exempts the command on the same footing as the store/client carve-outs.
 
-The rule is enforced in two places. The absorb manifest has a Kill Check (see `skills/printing-press/references/absorb-scoring.md`) that rejects reimplementation candidates before they enter the feature list. Dogfood runs `reimplementation_check` over every built novel-feature command and flags any handler file that shows neither a client call nor a store access.
+The rule is enforced in two places. The absorb manifest has a Kill Check (see `skills/printing-press/references/absorb-scoring.md`) that rejects reimplementation candidates before they enter the feature list. Dogfood runs `reimplementation_check` over every built novel-feature command and flags any handler file that shows neither a client call nor a store access (and lacks the static-reference opt-out).
 
 ## Build, Test & Lint
 
diff --git a/internal/cli/verify_skill_bundled.py b/internal/cli/verify_skill_bundled.py
index d266ab4c..7f3a41b0 100755
--- a/internal/cli/verify_skill_bundled.py
+++ b/internal/cli/verify_skill_bundled.py
@@ -43,6 +43,7 @@ import re
 import shlex
 import sys
 from dataclasses import dataclass, field
+from functools import lru_cache
 from pathlib import Path
 from typing import Iterable
 
@@ -117,18 +118,259 @@ def parse_use(use_str: str) -> tuple[str, int, int, bool]:
     return name, required, optional, variadic
 
 
+# CONSTRUCTOR_RE matches `func newXxxCmd(...) *cobra.Command {`. The
+# parameter list pattern `\([^()]*(?:\([^()]*\)[^()]*)*\)` handles one
+# level of nested parens — the case that matters in practice is a
+# function-typed parameter like `func newFooCmd(handler func() error)
+# *cobra.Command`. Without the nested-paren handling the regex stops
+# at the first `)` (the closer of `func()`) and the constructor is
+# silently dropped from the constructor map. Two-level nesting (e.g.
+# `func()` inside another `func()`) would still fail; flag for the
+# legacy fallback when that pathological shape appears.
+CONSTRUCTOR_RE = re.compile(
+    r'^func\s+(new[A-Z]\w*Cmd)\s*'
+    r'\([^()]*(?:\([^()]*\)[^()]*)*\)'
+    r'\s*\*cobra\.Command\s*\{',
+    re.MULTILINE,
+)
+ADDCMD_CHILD_RE = re.compile(r'\.AddCommand\s*\(\s*(new[A-Z]\w*Cmd)\s*\(')
+ROOT_ADDCMD_RE = re.compile(r'rootCmd\.AddCommand\s*\(\s*(new[A-Z]\w*Cmd)\s*\(')
+
+
+def _extract_function_body(text: str, start_offset: int) -> str | None:
+    """Given the offset just after the opening `{` of a function body,
+    return the body text (excluding the closing `}`). Tracks string and
+    comment state so braces inside string literals or comments don't
+    confuse the depth counter. Returns None if the body is unclosed.
+    """
+    depth = 1
+    i = start_offset
+    n = len(text)
+    in_string: str | None = None  # holds the active string opener: '"', '`', or "'"
+    in_line_comment = False
+    in_block_comment = False
+    while i < n and depth > 0:
+        c = text[i]
+        if in_line_comment:
+            if c == '\n':
+                in_line_comment = False
+            i += 1
+            continue
+        if in_block_comment:
+            if c == '*' and i + 1 < n and text[i + 1] == '/':
+                in_block_comment = False
+                i += 2
+                continue
+            i += 1
+            continue
+        if in_string is not None:
+            if c == '\\' and in_string != '`' and i + 1 < n:
+                # Skip the escaped char (still inside the string)
+                i += 2
+                continue
+            if c == in_string:
+                in_string = None
+            i += 1
+            continue
+        if c == '/' and i + 1 < n:
+            if text[i + 1] == '/':
+                in_line_comment = True
+                i += 2
+                continue
+            if text[i + 1] == '*':
+                in_block_comment = True
+                i += 2
+                continue
+        if c in ('"', '`', "'"):
+            in_string = c
+            i += 1
+            continue
+        if c == '{':
+            depth += 1
+        elif c == '}':
+            depth -= 1
+        i += 1
+    if depth != 0:
+        return None
+    return text[start_offset:i - 1]
+
+
+@dataclass
+class CommandConstructor:
+    name: str
+    file: Path
+    use: str
+    args_info: tuple | None
+    children: list[str] = field(default_factory=list)
+
+
+@lru_cache(maxsize=None)
+def collect_command_constructors(cli_dir: Path) -> dict[str, CommandConstructor]:
+    """Scan internal/cli/*.go for `func newXxxCmd(...) *cobra.Command`
+    declarations. For each, capture the Use string, the cobra.Args
+    validator, and the list of child constructor names called via
+    `<var>.AddCommand(newYyyCmd(...))` within the function body.
+
+    Result maps constructor name → CommandConstructor.
+
+    Cached per cli_dir for the lifetime of the process. verify-skill
+    invokes find_command_source once per recipe in SKILL.md (typically
+    5-15) and the source tree doesn't change mid-run, so the
+    file-system scan only needs to happen once. Callers must not
+    mutate the returned dict (it's the cache's storage).
+    """
+    src = cli_dir / "internal/cli"
+    if not src.exists():
+        return {}
+    constructors: dict[str, CommandConstructor] = {}
+    for go_file in src.glob("*.go"):
+        if go_file.name.endswith("_test.go"):
+            continue
+        try:
+            text = go_file.read_text()
+        except Exception:
+            continue
+        for m in CONSTRUCTOR_RE.finditer(text):
+            fn_name = m.group(1)
+            body = _extract_function_body(text, m.end())
+            if body is None:
+                continue
+            use_match = USE_RE.search(body)
+            if not use_match:
+                continue
+            args_match = ARGS_RE.search(body)
+            args_info = (args_match.group(1), args_match.group(2)) if args_match else None
+            children = list(dict.fromkeys(
+                child.group(1) for child in ADDCMD_CHILD_RE.finditer(body)
+            ))
+            constructors[fn_name] = CommandConstructor(
+                name=fn_name,
+                file=go_file,
+                use=use_match.group(1),
+                args_info=args_info,
+                children=children,
+            )
+    return constructors
+
+
+@lru_cache(maxsize=None)
+def find_root_children(cli_dir: Path) -> list[str]:
+    """Return the constructor names called from `rootCmd.AddCommand(...)`
+    anywhere in internal/cli/*.go. The ordering follows source order, but
+    the result is deduplicated.
+
+    Cached per cli_dir; callers must not mutate the returned list.
+    See collect_command_constructors for rationale."""
+    src = cli_dir / "internal/cli"
+    if not src.exists():
+        return []
+    seen: dict[str, None] = {}
+    for go_file in src.glob("*.go"):
+        if go_file.name.endswith("_test.go"):
+            continue
+        try:
+            text = go_file.read_text()
+        except Exception:
+            continue
+        for m in ROOT_ADDCMD_RE.finditer(text):
+            seen.setdefault(m.group(1), None)
+    return list(seen)
+
+
+def resolve_command_path(
+    cli_dir: Path,
+    cmd_path: list[str],
+    constructors: dict[str, CommandConstructor] | None = None,
+    root_children: list[str] | None = None,
+):
+    """Walk the AddCommand graph to find the canonical declaring file for
+    cmd_path. Returns (file, use_str, args_info) or (None, None, None) if
+    the path can't be resolved (unknown command, unconventional CLI).
+
+    This is the durable replacement for the old specificity-based
+    disambiguation in find_command_source. It picks the file based on
+    actual command-tree structure rather than guessing from `Use:` token
+    counts. See retro #301 finding F1.
+    """
+    if not cmd_path:
+        return None, None, None
+    if constructors is None:
+        constructors = collect_command_constructors(cli_dir)
+    if root_children is None:
+        root_children = find_root_children(cli_dir)
+    if not constructors or not root_children:
+        return None, None, None
+
+    current = None
+    for fn_name in root_children:
+        info = constructors.get(fn_name)
+        if info is None:
+            continue
+        leaf, _, _, _ = parse_use(info.use)
+        if leaf == cmd_path[0]:
+            current = info
+            break
+    if current is None:
+        return None, None, None
+
+    for token in cmd_path[1:]:
+        next_info = None
+        for child_fn in current.children:
+            child = constructors.get(child_fn)
+            if child is None:
+                continue
+            leaf, _, _, _ = parse_use(child.use)
+            if leaf == token:
+                next_info = child
+                break
+        if next_info is None:
+            return None, None, None
+        current = next_info
+
+    return current.file, current.use, current.args_info
+
+
 def find_command_source(cli_dir: Path, cmd_path: list[str]):
-    """Locate source file(s) whose cobra.Command matches this path.
-
-    Returns (go_files, use_str, args_info) where go_files is a list.
-    Why a list: CLIs sometimes declare the same Use in two files (historical
-    artifact or generator + transcendence both shipping a version of the
-    same command). Cobra only registers one at runtime, but we don't know
-    which without parsing root.go's AddCommand calls. Returning all matching
-    files lets callers union their flags when checking declarations.
+    """Locate the source file whose cobra.Command matches this path.
+
+    Returns (go_files, use_str, args_info) where go_files is a list (kept
+    list-shaped for backwards compatibility — most callers iterate it for
+    `flag_declared_in` lookups).
+
+    Resolution strategy:
+
+      1. Walk the rootCmd.AddCommand graph (the durable approach added in
+         retro #301 F1). When the CLI follows the standard `func newXxxCmd`
+         + `<parent>.AddCommand(newXxxCmd(...))` convention, this returns
+         exactly one file per command path with no false positives, even
+         when two different commands share a leaf name (e.g.,
+         `recipe-goat-pp-cli save` vs `recipe-goat-pp-cli profile save`).
+
+      2. If the graph walk fails (unconventional CLI, missing rootCmd,
+         constructor functions not named `newXxxCmd`), fall back to a
+         legacy specificity heuristic that scans every Go file for any
+         `Use:` whose first token matches the leaf. The legacy path is
+         imperfect (it can pick the wrong file when leaves collide) but
+         keeps the tool useful on CLIs that don't follow the standard
+         convention.
     """
     if not cmd_path:
         return [], None, None
+
+    file, use_str, args_info = resolve_command_path(cli_dir, cmd_path)
+    if file is not None:
+        return [file], use_str, args_info
+
+    # Legacy fallback — kept to preserve behavior for unconventional CLIs.
+    return _legacy_find_command_source(cli_dir, cmd_path)
+
+
+def _legacy_find_command_source(cli_dir: Path, cmd_path: list[str]):
+    """Pre-retro-#301 specificity-based heuristic. Retained as a fallback
+    for CLIs whose command structure doesn't match the standard
+    `rootCmd.AddCommand(newXxxCmd(...))` pattern resolve_command_path
+    expects (e.g., commands constructed via local helpers, or files that
+    declare cobra.Commands without going through a `newXxxCmd` factory)."""
     leaf = cmd_path[-1]
     src = cli_dir / "internal/cli"
     if not src.exists():
@@ -157,17 +399,12 @@ def find_command_source(cli_dir: Path, cmd_path: list[str]):
     if not candidates:
         return [], None, None
 
-    # Multi-token paths: prefer filename-match (e.g., contacts_search.go for
-    # cmd_path ["contacts", "search"]).
     if len(cmd_path) >= 2:
         expected_basename = "_".join(cmd_path).replace("-", "_") + ".go"
         for spec, go_file, use_str, args_info in candidates:
             if go_file.name == expected_basename:
                 return [go_file], use_str, args_info
 
-    # Single-token paths (or no filename match): take all files at the
-    # highest specificity tier. For flag verification, any one of them
-    # declaring the flag counts. For Use string, pick the representative.
     candidates.sort(key=lambda c: -c[0])
     top_spec = candidates[0][0]
     top_files = [c[1] for c in candidates if c[0] == top_spec]
diff --git a/internal/cli/verify_skill_test.go b/internal/cli/verify_skill_test.go
index 166d6274..1eed77db 100644
--- a/internal/cli/verify_skill_test.go
+++ b/internal/cli/verify_skill_test.go
@@ -69,6 +69,80 @@ fixture-pp-cli search "chicken" --max-time 30m
 		"diagnostic must name the exact mismatch so the skill reader knows what to fix")
 }
 
+// TestVerifySkill_NoFalsePositiveOnSharedLeafName is the regression
+// guard for retro #301 finding F1: when two cobra commands share a leaf
+// name at different paths (e.g., a top-level `save <url>` plus a
+// `profile save <name>` subcommand), the old specificity-based file
+// picker silently dropped the lower-specificity file from the
+// flag-declaration union check. The result was a false-positive
+// `--<flag> is declared elsewhere but not on save` even though the flag
+// was correctly declared on the top-level save command.
+//
+// This test writes a synthetic CLI with that exact shape and asserts
+// the verifier does NOT report a false-positive flag-commands finding
+// when the SKILL example uses a flag declared on the top-level command.
+func TestVerifySkill_NoFalsePositiveOnSharedLeafName(t *testing.T) {
+	bin := buildPrintingPressBinary(t)
+	dir := t.TempDir()
+	require.NoError(t, os.MkdirAll(filepath.Join(dir, "internal", "cli"), 0o755))
+
+	// root.go wires both save and profile into rootCmd. The verifier
+	// must follow rootCmd.AddCommand calls to know that cmd_path=['save']
+	// resolves to save_cmd.go (not profile.go's profile-save subcommand).
+	require.NoError(t, os.WriteFile(filepath.Join(dir, "internal", "cli", "root.go"), []byte(`package cli
+import "github.com/spf13/cobra"
+func Execute() error {
+	rootCmd := &cobra.Command{Use: "fixture-pp-cli"}
+	rootCmd.AddCommand(newSaveCmd())
+	rootCmd.AddCommand(newProfileCmd())
+	return rootCmd.Execute()
+}
+`), 0o644))
+
+	// Top-level save: declares --tags. Use string is intentionally
+	// short so the legacy specificity heuristic would lose the tie-break
+	// against profile.go's longer Use string.
+	require.NoError(t, os.WriteFile(filepath.Join(dir, "internal", "cli", "save_cmd.go"), []byte(`package cli
+import "github.com/spf13/cobra"
+func newSaveCmd() *cobra.Command {
+	var tags string
+	cmd := &cobra.Command{Use: "save <url>"}
+	cmd.Flags().StringVar(&tags, "tags", "", "Comma-separated tags")
+	return cmd
+}
+`), 0o644))
+
+	// profile save: declares its own flags. Use string is more specific
+	// (more positionals + variadic), which would win the legacy
+	// tie-break and drop save_cmd.go from the flag check.
+	require.NoError(t, os.WriteFile(filepath.Join(dir, "internal", "cli", "profile.go"), []byte(`package cli
+import "github.com/spf13/cobra"
+func newProfileCmd() *cobra.Command {
+	cmd := &cobra.Command{Use: "profile"}
+	cmd.AddCommand(newProfileSaveCmd())
+	return cmd
+}
+func newProfileSaveCmd() *cobra.Command {
+	var label string
+	cmd := &cobra.Command{Use: "save <name> [--<flag> <value> ...]"}
+	cmd.Flags().StringVar(&label, "label", "", "Profile label")
+	return cmd
+}
+`), 0o644))
+
+	// SKILL uses --tags on the top-level save command. The graph walk
+	// must resolve to save_cmd.go (which declares --tags) and not
+	// profile.go (which declares --label).
+	skill := "---\nname: pp-fixture\n---\n\n# Fixture\n\n```bash\nfixture-pp-cli save https://example.com --tags foo,bar\n```\n"
+	require.NoError(t, os.WriteFile(filepath.Join(dir, "SKILL.md"), []byte(skill), 0o644))
+	require.NoError(t, os.WriteFile(filepath.Join(dir, ".printing-press.json"), []byte(`{"cli_name":"fixture-pp-cli"}`), 0o644))
+
+	out, err := exec.Command(bin, "verify-skill", "--dir", dir).CombinedOutput()
+	require.NoError(t, err, "verifier must NOT raise findings for valid shared-leaf usage; got: %s", string(out))
+	require.Contains(t, string(out), "All checks passed",
+		"shared-leaf disambiguation should resolve via rootCmd.AddCommand graph, not specificity heuristic")
+}
+
 // TestVerifySkill_PassesWhenSkillMatches confirms the verifier doesn't
 // false-positive on a well-formed CLI.
 func TestVerifySkill_PassesWhenSkillMatches(t *testing.T) {
diff --git a/internal/generator/generator.go b/internal/generator/generator.go
index ff9b721d..32e4693d 100644
--- a/internal/generator/generator.go
+++ b/internal/generator/generator.go
@@ -933,6 +933,7 @@ func (g *Generator) Generate() error {
 		"client.go.tmpl":         filepath.Join("internal", "client", "client.go"),
 		"cliutil_fanout.go.tmpl": filepath.Join("internal", "cliutil", "fanout.go"),
 		"cliutil_text.go.tmpl":   filepath.Join("internal", "cliutil", "text.go"),
+		"cliutil_probe.go.tmpl":  filepath.Join("internal", "cliutil", "probe.go"),
 		"cliutil_test.go.tmpl":   filepath.Join("internal", "cliutil", "cliutil_test.go"),
 		"types.go.tmpl":          filepath.Join("internal", "types", "types.go"),
 		"golangci.yml.tmpl":      ".golangci.yml",
diff --git a/internal/generator/generator_test.go b/internal/generator/generator_test.go
index 862d4cbe..a9bd23ec 100644
--- a/internal/generator/generator_test.go
+++ b/internal/generator/generator_test.go
@@ -24,22 +24,49 @@ import (
 func TestGenerateProjectsCompile(t *testing.T) {
 	t.Parallel()
 
+	// expectedFiles is the total file count per fixture; mustInclude is
+	// the set of paths that every generated CLI must ship regardless of
+	// spec shape. Two assertions instead of one: count guards against
+	// templates silently disappearing, mustInclude guards against
+	// renames/moves that preserve the count but break consumers. When a
+	// new always-emitted template is added, bump expectedFiles for each
+	// fixture and add the path to mustInclude. When a template is
+	// renamed, only mustInclude needs updating.
+	// mustInclude lists files emitted by gen.Generate() directly. Files
+	// produced by downstream steps (tools-manifest.json from
+	// manifest-gen, workflow_verify.yaml from the publish pipeline,
+	// dogfood-results.json from dogfood) aren't in scope for this test
+	// — it only exercises the generator.
+	mustInclude := []string{
+		"go.mod",
+		"Makefile",
+		"README.md",
+		"SKILL.md",
+		"internal/cli/root.go",
+		"internal/cli/which.go",
+		"internal/cli/profile.go",
+		"internal/cli/feedback.go",
+		"internal/cli/agent_context.go",
+		"internal/cliutil/fanout.go",
+		"internal/cliutil/text.go",
+		"internal/cliutil/probe.go",
+		"internal/cliutil/cliutil_test.go",
+		"internal/client/client.go",
+		"internal/config/config.go",
+	}
+
 	tests := []struct {
 		name          string
 		specPath      string
 		expectedFiles int
 	}{
-		// +3 for cliutil package: fanout.go, text.go, cliutil_test.go
-		// +1 for internal/cli/agent_context.go (Cloudflare-style runtime introspection)
-		// +1 for internal/cli/profile.go (HeyGen-style named-profile system)
-		// +1 for internal/cli/deliver.go (HeyGen-style --deliver output routing)
-		// +1 for internal/cli/feedback.go (HeyGen-style in-band agent feedback channel)
-		// +1 for internal/store/schema_version_test.go (PRAGMA user_version gate, discrawl-inspired)
-		// +2 for internal/cli/which.go + which_test.go (capability-to-command resolver)
-		// +1 for internal/store/upsert_batch_test.go (regression for issue #268: typed-table dispatch)
-		{name: "stytch", specPath: filepath.Join("..", "..", "testdata", "stytch.yaml"), expectedFiles: 44},
-		{name: "clerk", specPath: filepath.Join("..", "..", "testdata", "clerk.yaml"), expectedFiles: 49},
-		{name: "loops", specPath: filepath.Join("..", "..", "testdata", "loops.yaml"), expectedFiles: 46},
+		// expectedFiles is total file count under the generated tree.
+		// Bump it AND add to mustInclude above when adding always-emitted
+		// templates. Per-spec dynamic files (per-resource command files,
+		// generated tests) account for the difference between fixtures.
+		{name: "stytch", specPath: filepath.Join("..", "..", "testdata", "stytch.yaml"), expectedFiles: 45},
+		{name: "clerk", specPath: filepath.Join("..", "..", "testdata", "clerk.yaml"), expectedFiles: 50},
+		{name: "loops", specPath: filepath.Join("..", "..", "testdata", "loops.yaml"), expectedFiles: 47},
 	}
 
 	for _, tt := range tests {
@@ -53,6 +80,15 @@ func TestGenerateProjectsCompile(t *testing.T) {
 
 			require.Equal(t, tt.expectedFiles, countFiles(t, outputDir))
 
+			// Beyond the count, every fixture must contain these
+			// always-emitted files. Catches the rename / move case
+			// that preserves the count but breaks consumers.
+			for _, rel := range mustInclude {
+				path := filepath.Join(outputDir, rel)
+				_, err := os.Stat(path)
+				require.NoError(t, err, "must-include path missing: %s", rel)
+			}
+
 			runGoCommand(t, outputDir, "mod", "tidy")
 			runGoCommand(t, outputDir, "build", "./...")
 
diff --git a/internal/generator/templates/cliutil_probe.go.tmpl b/internal/generator/templates/cliutil_probe.go.tmpl
new file mode 100644
index 00000000..c7d62a2b
--- /dev/null
+++ b/internal/generator/templates/cliutil_probe.go.tmpl
@@ -0,0 +1,104 @@
+// Copyright {{currentYear}} {{.Owner}}. Licensed under Apache-2.0. See LICENSE.
+// Generated by CLI Printing Press (https://github.com/mvanhorn/cli-printing-press). DO NOT EDIT.
+
+package cliutil
+
+import (
+	"context"
+	"fmt"
+	"io"
+	"net/http"
+	"time"
+)
+
+// defaultProbeTimeout caps the request when the caller passes a nil
+// client and didn't set a context deadline. Without this cap, a probe
+// against a non-responsive host could hang indefinitely (the global
+// http.DefaultClient has no Timeout). Callers who pass their own
+// *http.Client are expected to set Timeout there; this value only
+// applies to the nil-client fallback.
+const defaultProbeTimeout = 10 * time.Second
+
+// ReachabilityStatus is one of the strings returned by ProbeReachable.
+// Callers (typically a doctor command listing per-source health) match
+// on these constants when deciding whether to render OK/WARN/FAIL.
+const (
+	// ReachabilityReachable means the host responded with a 2xx, a 206
+	// (partial — Range honored), or a 416 (Range not honored, but the
+	// host did respond with headers). All three are evidence that the
+	// host is alive and responding to GET.
+	ReachabilityReachable = "reachable"
+	// ReachabilityBlocked means the host responded with a 4xx (other
+	// than 416) or 5xx. The host is up but is refusing this request —
+	// usually a CDN bot screen, a paywall, or a server error.
+	ReachabilityBlocked = "blocked"
+	// ReachabilityUnreachable means the request errored at the network
+	// layer — DNS failure, connection refused, TLS shutdown, timeout.
+	ReachabilityUnreachable = "unreachable"
+)
+
+// ProbeReachable does a lightweight reachability probe against url
+// using client and returns a (status, code, err) triple. The probe
+// uses GET with a `Range: bytes=0-1023` header so it never pulls more
+// than ~1 KB of body, regardless of how the host responds; the body
+// is read and discarded so the connection can be released.
+//
+// Why not HEAD: many recipe-site CDNs (BBC, RecipeTin Eats, AllRecipes,
+// Serious Eats, The Kitchn) terminate HEAD requests with a TLS
+// shutdown / EOF even though they serve GET cleanly. A HEAD-based
+// probe lies — reporting "unreachable EOF" for hosts that work fine
+// for the real fetch path. recipe-goat hit this in retro #301
+// finding F4: doctor reported six sites unreachable that the goat
+// ranker was successfully scraping.
+//
+// Use this from any doctor or health-check command that does
+// per-source reachability fan-out, with the same client that the real
+// fetch path uses (typically a Surf-Chrome client built via
+// surf.NewClient().Builder().Impersonate().Chrome().Build()). Probe
+// drift between the doctor probe and the fetch path is the bug class
+// this helper exists to prevent.
+//
+// Returned values:
+//   - status is one of ReachabilityReachable, ReachabilityBlocked, or
+//     ReachabilityUnreachable.
+//   - code is the HTTP status code, or 0 when the request errored at
+//     the network layer.
+//   - err is non-nil only for network-layer failures. A 4xx or 5xx
+//     response is reported via status/code with err == nil.
+func ProbeReachable(ctx context.Context, client *http.Client, url string) (status string, code int, err error) {
+	if client == nil {
+		// Build a copy of DefaultClient with a bounded timeout — the
+		// global DefaultClient has none, so a nil-client probe against
+		// a slow host would hang. Callers passing their own client are
+		// expected to set Timeout themselves.
+		client = &http.Client{Timeout: defaultProbeTimeout}
+	}
+	req, reqErr := http.NewRequestWithContext(ctx, "GET", url, nil)
+	if reqErr != nil {
+		return ReachabilityUnreachable, 0, fmt.Errorf("building request: %w", reqErr)
+	}
+	// Range: bytes=0-1023 keeps body bounded for hosts that honor it.
+	// Hosts that don't support Range respond with 200 + full body or
+	// 416 — both are caught below as "reachable", and the limited
+	// io.Copy below ensures we never pull more than 1 KiB anyway.
+	req.Header.Set("Range", "bytes=0-1023")
+	resp, doErr := client.Do(req)
+	if doErr != nil {
+		return ReachabilityUnreachable, 0, doErr
+	}
+	defer resp.Body.Close()
+	// Drain up to 2 KiB so the connection can be reused. We read past
+	// the 1024-byte Range hint to cover hosts that ignored it.
+	_, _ = io.Copy(io.Discard, io.LimitReader(resp.Body, 2048))
+	switch {
+	case resp.StatusCode >= 200 && resp.StatusCode < 300:
+		return ReachabilityReachable, resp.StatusCode, nil
+	case resp.StatusCode == http.StatusRequestedRangeNotSatisfiable:
+		// 416 means the host doesn't support Range. We still got
+		// headers back, so the host is up and responding to GET —
+		// classify as reachable.
+		return ReachabilityReachable, resp.StatusCode, nil
+	default:
+		return ReachabilityBlocked, resp.StatusCode, nil
+	}
+}
diff --git a/internal/generator/templates/cliutil_test.go.tmpl b/internal/generator/templates/cliutil_test.go.tmpl
index 9505455e..91a86aac 100644
--- a/internal/generator/templates/cliutil_test.go.tmpl
+++ b/internal/generator/templates/cliutil_test.go.tmpl
@@ -8,6 +8,8 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"net/http"
+	"net/http/httptest"
 	"sync"
 	"sync/atomic"
 	"testing"
@@ -424,3 +426,186 @@ func TestFanoutReportErrorsTruncates(t *testing.T) {
 		t.Errorf("truncated line should be ~140 chars, got %d (%q)", len(out), out)
 	}
 }
+
+// ---- ProbeReachable ----
+
+// TestProbeReachable_200 asserts that a plain 2xx GET is classified
+// reachable with the right code.
+func TestProbeReachable_200(t *testing.T) {
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusOK)
+		_, _ = w.Write([]byte("hello"))
+	}))
+	defer srv.Close()
+
+	status, code, err := ProbeReachable(context.Background(), srv.Client(), srv.URL)
+	if err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+	if status != ReachabilityReachable {
+		t.Errorf("status: want %q, got %q", ReachabilityReachable, status)
+	}
+	if code != 200 {
+		t.Errorf("code: want 200, got %d", code)
+	}
+}
+
+// TestProbeReachable_206_Reachable asserts hosts that honor Range
+// (returning 206 Partial Content) are classified reachable.
+func TestProbeReachable_206_Reachable(t *testing.T) {
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusPartialContent)
+	}))
+	defer srv.Close()
+
+	status, code, err := ProbeReachable(context.Background(), srv.Client(), srv.URL)
+	if err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+	if status != ReachabilityReachable {
+		t.Errorf("status: want reachable, got %q", status)
+	}
+	if code != 206 {
+		t.Errorf("code: want 206, got %d", code)
+	}
+}
+
+// TestProbeReachable_416_Reachable asserts hosts that don't support
+// Range (returning 416 Range Not Satisfiable) are still reachable —
+// the headers came back, the host is up. This is the F4 motivating
+// case: HEAD-then-GET probes incorrectly report unreachable here.
+func TestProbeReachable_416_Reachable(t *testing.T) {
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusRequestedRangeNotSatisfiable)
+	}))
+	defer srv.Close()
+
+	status, code, err := ProbeReachable(context.Background(), srv.Client(), srv.URL)
+	if err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+	if status != ReachabilityReachable {
+		t.Errorf("status: want reachable (416 means headers came back), got %q", status)
+	}
+	if code != 416 {
+		t.Errorf("code: want 416, got %d", code)
+	}
+}
+
+// TestProbeReachable_403_Blocked asserts CDN bot screens (4xx other
+// than 416) are classified blocked, not unreachable. The host is up
+// and refusing this request — a doctor command should render WARN
+// rather than FAIL.
+func TestProbeReachable_403_Blocked(t *testing.T) {
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusForbidden)
+	}))
+	defer srv.Close()
+
+	status, code, err := ProbeReachable(context.Background(), srv.Client(), srv.URL)
+	if err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+	if status != ReachabilityBlocked {
+		t.Errorf("status: want blocked, got %q", status)
+	}
+	if code != 403 {
+		t.Errorf("code: want 403, got %d", code)
+	}
+}
+
+// TestProbeReachable_NetworkError_Unreachable asserts network-layer
+// failures (DNS, connection refused, timeout) report unreachable with
+// a non-nil err.
+func TestProbeReachable_NetworkError_Unreachable(t *testing.T) {
+	// Use a port that nothing is listening on.
+	status, code, err := ProbeReachable(context.Background(), http.DefaultClient, "http://127.0.0.1:1")
+	if err == nil {
+		t.Fatal("expected non-nil err for unreachable host")
+	}
+	if status != ReachabilityUnreachable {
+		t.Errorf("status: want unreachable, got %q", status)
+	}
+	if code != 0 {
+		t.Errorf("code: want 0 (no response), got %d", code)
+	}
+}
+
+// TestProbeReachable_NilClient_UsesDefault confirms the nil-client
+// guard so doctor commands don't have to plumb an explicit *http.Client
+// when default behavior is fine.
+func TestProbeReachable_NilClient_UsesDefault(t *testing.T) {
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusOK)
+	}))
+	defer srv.Close()
+
+	status, _, err := ProbeReachable(context.Background(), nil, srv.URL)
+	if err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+	if status != ReachabilityReachable {
+		t.Errorf("status: want reachable, got %q", status)
+	}
+}
+
+// TestProbeReachable_NilClient_HasTimeout asserts the nil-client
+// fallback uses a bounded-timeout client rather than http.DefaultClient
+// (which has no timeout). Without this, a probe against a slow host
+// could hang indefinitely. The test starts a server that hangs forever
+// and relies on the default 10s timeout to bail out — capped to 12s
+// total so a regression that drops the timeout would surface as a
+// test failure rather than a hung test.
+func TestProbeReachable_NilClient_HasTimeout(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skip slow timeout test in -short mode")
+	}
+	hang := make(chan struct{})
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		<-hang // never returns until t.Cleanup closes it
+	}))
+	t.Cleanup(func() {
+		close(hang)
+		srv.Close()
+	})
+
+	done := make(chan struct{})
+	var status string
+	var probeErr error
+	go func() {
+		status, _, probeErr = ProbeReachable(context.Background(), nil, srv.URL)
+		close(done)
+	}()
+	select {
+	case <-done:
+		// Probe returned within the bounded timeout — expected.
+		if probeErr == nil {
+			t.Fatalf("expected timeout err, got nil")
+		}
+		if status != ReachabilityUnreachable {
+			t.Errorf("status: want unreachable on timeout, got %q", status)
+		}
+	case <-time.After(12 * time.Second):
+		t.Fatalf("ProbeReachable hung past defaultProbeTimeout — nil-client fallback may be missing its bounded-timeout guard")
+	}
+}
+
+// TestProbeReachable_SendsRangeHeader confirms the probe sends
+// `Range: bytes=0-1023` so hosts that support Range bound the
+// response body before we even read it.
+func TestProbeReachable_SendsRangeHeader(t *testing.T) {
+	var receivedRange string
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		receivedRange = r.Header.Get("Range")
+		w.WriteHeader(http.StatusOK)
+	}))
+	defer srv.Close()
+
+	_, _, err := ProbeReachable(context.Background(), srv.Client(), srv.URL)
+	if err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+	if receivedRange != "bytes=0-1023" {
+		t.Errorf("Range header: want %q, got %q", "bytes=0-1023", receivedRange)
+	}
+}
diff --git a/internal/pipeline/reimplementation_check.go b/internal/pipeline/reimplementation_check.go
index f5e1f214..19708b9b 100644
--- a/internal/pipeline/reimplementation_check.go
+++ b/internal/pipeline/reimplementation_check.go
@@ -33,6 +33,13 @@ type ReimplementationCheckResult struct {
 	// ExemptedViaStore is the number of commands that passed the check
 	// by consulting the local store package (SQLite-derived features).
 	ExemptedViaStore int `json:"exempted_via_store"`
+	// ExemptedViaAnnotation is the number of commands that passed the
+	// check via the // pp:novel-static-reference marker (curated
+	// static-data features like substitution tables, holiday lists).
+	// Tracked separately from ExemptedViaStore so analytics over
+	// dogfood-results.json can distinguish the two carve-out classes
+	// even though they share the same ship/no-ship decision.
+	ExemptedViaAnnotation int `json:"exempted_via_annotation,omitempty"`
 	// Suspicious is the list of commands whose files show no client
 	// call and no store access - the candidate hand-rolled responses.
 	Suspicious []ReimplementationFinding `json:"suspicious,omitempty"`
@@ -169,10 +176,14 @@ func checkReimplementation(cliDir, researchDir string) ReimplementationCheckResu
 		// and take the most favorable classification - any single file
 		// with the right signals vindicates the command.
 		result.Checked++
-		finding, exempt, ok := classifyReimplementation(files, fileContent, storeHelpers)
-		if exempt {
+		finding, kind, ok := classifyReimplementation(files, fileContent, storeHelpers)
+		switch kind {
+		case exemptStore:
 			result.ExemptedViaStore++
 			continue
+		case exemptAnnotation:
+			result.ExemptedViaAnnotation++
+			continue
 		}
 		if !ok {
 			finding.Command = nf.Command
@@ -187,19 +198,52 @@ func checkReimplementation(cliDir, researchDir string) ReimplementationCheckResu
 	return result
 }
 
+// novelStaticReferenceRe matches the per-command opt-out marker
+// documented in AGENTS.md. A line of the form
+//
+//	// pp:novel-static-reference
+//
+// (any leading whitespace, optional " " before the directive) anywhere
+// in a command's source file declares that the command intentionally
+// ships curated static data — substitution tables, holiday lists,
+// currency metadata, conversion factors — rather than calling an API
+// or reading from the local store. The reimplementation check honors
+// the marker and exempts the command, treating it on the same footing
+// as the existing store/client carve-outs.
+//
+// Added for retro #301 finding F3.
+var novelStaticReferenceRe = regexp.MustCompile(`(?m)^\s*//\s*pp:novel-static-reference\b`)
+
+// exemptionKind labels which carve-out vindicated a command, so the
+// caller can route the bump to the right counter on
+// ReimplementationCheckResult. exemptNone covers both "passes via
+// client signal" (ok=true, kind=exemptNone) and "is suspicious"
+// (ok=false, kind=exemptNone) — the kind only carries meaning when
+// the result is exempt.
+type exemptionKind int
+
+const (
+	exemptNone exemptionKind = iota
+	exemptStore
+	exemptAnnotation
+)
+
 // classifyReimplementation returns the best classification across the
 // set of files that implement a single command. The rules, in order:
 //
-//  1. If any file shows a store signal, the command is exempted as a
-//     local-SQLite feature. Return (_, true, true).
-//  2. If any file shows a client signal, the command is fine. Return
-//     (_, false, true).
-//  3. Otherwise the command is suspicious. Return a ReimplementationFinding
-//     naming the primary file and a reason. Return (finding, false, false).
+//  1. If any file carries the `// pp:novel-static-reference` marker,
+//     the command is exempted as an intentional static-data feature.
+//     Return (_, exemptAnnotation, true).
+//  2. If any file shows a store signal, the command is exempted as a
+//     local-SQLite feature. Return (_, exemptStore, true).
+//  3. If any file shows a client signal, the command is fine. Return
+//     (_, exemptNone, true).
+//  4. Otherwise the command is suspicious. Return a ReimplementationFinding
+//     naming the primary file and a reason. Return (finding, exemptNone, false).
 //
-// The trivial-body regex is consulted only when rule 3 fires, to pick
+// The trivial-body regex is consulted only when rule 4 fires, to pick
 // between "empty stub" and "hand-rolled response" as the reason.
-func classifyReimplementation(files []string, fileContent map[string]string, storeHelpers map[string]bool) (ReimplementationFinding, bool, bool) {
+func classifyReimplementation(files []string, fileContent map[string]string, storeHelpers map[string]bool) (ReimplementationFinding, exemptionKind, bool) {
 	hasClient := false
 	hasTrivialBody := false
 	primaryFile := files[0]
@@ -208,11 +252,14 @@ func classifyReimplementation(files []string, fileContent map[string]string, sto
 		if !ok {
 			continue
 		}
+		if novelStaticReferenceRe.MatchString(content) {
+			return ReimplementationFinding{File: f}, exemptAnnotation, true
+		}
 		if hasStoreSignal(content) {
-			return ReimplementationFinding{File: f}, true, true
+			return ReimplementationFinding{File: f}, exemptStore, true
 		}
 		if callsStoreHelper(content, storeHelpers) {
-			return ReimplementationFinding{File: f}, true, true
+			return ReimplementationFinding{File: f}, exemptStore, true
 		}
 		if hasClientSignal(content) {
 			hasClient = true
@@ -222,13 +269,13 @@ func classifyReimplementation(files []string, fileContent map[string]string, sto
 		}
 	}
 	if hasClient {
-		return ReimplementationFinding{File: primaryFile}, false, true
+		return ReimplementationFinding{File: primaryFile}, exemptNone, true
 	}
 	reason := "hand-rolled response: no API client call, no store access"
 	if hasTrivialBody {
 		reason = "empty body: no implementation"
 	}
-	return ReimplementationFinding{File: primaryFile, Reason: reason}, false, false
+	return ReimplementationFinding{File: primaryFile, Reason: reason}, exemptNone, false
 }
 
 func hasStoreSignal(content string) bool {
diff --git a/internal/pipeline/reimplementation_check_test.go b/internal/pipeline/reimplementation_check_test.go
index 230a9a45..940b83e2 100644
--- a/internal/pipeline/reimplementation_check_test.go
+++ b/internal/pipeline/reimplementation_check_test.go
@@ -441,3 +441,105 @@ func TestCheckReimplementation_NoNovelFeatures_Skipped(t *testing.T) {
 		t.Errorf("expected Skipped=true, got %#v", got)
 	}
 }
+
+// TestCheckReimplementation_NovelStaticReferenceMarker_Exempted is the
+// regression guard for retro #301 finding F3: a novel feature that
+// intentionally ships curated static data (substitution tables, holiday
+// lists, currency metadata) has no API client call and no store call,
+// because the data IS the feature. Before the F3 fix, dogfood flagged
+// these as "hand-rolled response: no API client call, no store access"
+// even when they were the kind of feature explicitly approved during
+// Phase 1.5. The `// pp:novel-static-reference` marker in the file
+// header opts the command out of the reimplementation check.
+func TestCheckReimplementation_NovelStaticReferenceMarker_Exempted(t *testing.T) {
+	files := map[string]string{
+		"sub.go": `package cli
+
+// pp:novel-static-reference
+//
+// Substitution lookups are a curated static-data feature; the data is
+// shipped as a hardcoded table with no API or store backing.
+
+import "github.com/spf13/cobra"
+
+var subTable = map[string][]string{
+	"buttermilk": {"milk + lemon juice", "milk + vinegar", "yogurt"},
+	"eggs":       {"flax meal + water", "applesauce"},
+}
+
+func newSubCmd(flags *rootFlags) *cobra.Command {
+	return &cobra.Command{
+		Use: "sub",
+		RunE: func(cmd *cobra.Command, args []string) error {
+			_ = subTable
+			return nil
+		},
+	}
+}
+`,
+	}
+	cliDir, pipelineDir := seedReimplementationFixture(t, files, []NovelFeature{
+		{Name: "Substitution lookup", Command: "sub"},
+	})
+
+	got := checkReimplementation(cliDir, pipelineDir)
+	if got.Checked != 1 {
+		t.Fatalf("Checked: want 1, got %d", got.Checked)
+	}
+	if got.ExemptedViaAnnotation != 1 {
+		t.Fatalf("ExemptedViaAnnotation: want 1 (marker should exempt), got %d", got.ExemptedViaAnnotation)
+	}
+	if got.ExemptedViaStore != 0 {
+		t.Errorf("ExemptedViaStore: want 0 (annotation is its own carve-out, not store), got %d", got.ExemptedViaStore)
+	}
+	if len(got.Suspicious) != 0 {
+		t.Fatalf("Suspicious: want 0, got %d (%v)", len(got.Suspicious), got.Suspicious)
+	}
+}
+
+// TestCheckReimplementation_WithoutMarker_StillFlagged confirms the
+// F3 fix doesn't silently exempt commands that lack the explicit
+// `// pp:novel-static-reference` marker. Same shape as the test above
+// but without the comment — must still be flagged.
+func TestCheckReimplementation_WithoutMarker_StillFlagged(t *testing.T) {
+	files := map[string]string{
+		"sub.go": `package cli
+
+import "github.com/spf13/cobra"
+
+var subTable = map[string][]string{
+	"buttermilk": {"milk + lemon juice"},
+}
+
+func newSubCmd(flags *rootFlags) *cobra.Command {
+	return &cobra.Command{
+		Use: "sub",
+		RunE: func(cmd *cobra.Command, args []string) error {
+			_ = subTable
+			return nil
+		},
+	}
+}
+`,
+	}
+	cliDir, pipelineDir := seedReimplementationFixture(t, files, []NovelFeature{
+		{Name: "Substitution lookup", Command: "sub"},
+	})
+
+	got := checkReimplementation(cliDir, pipelineDir)
+	if got.Checked != 1 {
+		t.Fatalf("Checked: want 1, got %d", got.Checked)
+	}
+	if got.ExemptedViaAnnotation != 0 {
+		t.Fatalf("ExemptedViaAnnotation: want 0 (no marker), got %d", got.ExemptedViaAnnotation)
+	}
+	if got.ExemptedViaStore != 0 {
+		t.Fatalf("ExemptedViaStore: want 0 (no store signal either), got %d", got.ExemptedViaStore)
+	}
+	if len(got.Suspicious) != 1 {
+		t.Fatalf("Suspicious: want 1, got %d", len(got.Suspicious))
+	}
+	if !strings.Contains(got.Suspicious[0].Reason, "no API client call") {
+		t.Errorf("expected hand-rolled-response reason, got %q", got.Suspicious[0].Reason)
+	}
+}
diff --git a/internal/pipeline/runtime.go b/internal/pipeline/runtime.go
index 9509fc2d..6ba0f62f 100644
--- a/internal/pipeline/runtime.go
+++ b/internal/pipeline/runtime.go
@@ -573,23 +573,48 @@ func inferPositionalArgs(binary string, cmd *discoveredCommand) {
 	if m == nil {
 		return
 	}
-	rest := string(m[1])
 
-	// Extract <arg> and [arg] placeholders (but not [flags] or [command])
-	placeholderRe := regexp.MustCompile(`[<\[]([a-zA-Z][\w-]*)[>\]]`)
-	matches := placeholderRe.FindAllStringSubmatch(rest, -1)
-	if len(matches) == 0 {
-		return
+	for _, name := range extractPositionalPlaceholders(string(m[1])) {
+		cmd.Args = append(cmd.Args, syntheticArgValue(name))
 	}
+}
 
+// flagDescriptorRe matches a bracketed token whose body looks like a flag
+// descriptor rather than an optional positional. The body starts with one
+// or more leading dashes, or contains an `=` sign (e.g., `[--tags=<csv>]`,
+// `[--stdin]`, `[-v]`). Without scrubbing these first, the placeholder
+// regex picks up `<csv>` from such tokens as if it were a separate
+// positional, which then gets passed to the binary and breaks
+// `cobra.MaximumNArgs(1)` validators on commands that accept exactly one
+// real positional. See retro #301 finding F2.
+var flagDescriptorRe = regexp.MustCompile(`\[\s*-+[^\]]*\]|\[[^\]]*=[^\]]*\]`)
+
+// positionalPlaceholderRe extracts <name> and [name] placeholders from the
+// scrubbed Usage suffix. Runs after flagDescriptorRe.
+var positionalPlaceholderRe = regexp.MustCompile(`[<\[]([a-zA-Z][\w-]*)[>\]]`)
+
+// extractPositionalPlaceholders returns the placeholder names found in a
+// cobra Usage suffix (the part after `Usage:\n  cli-name cmd-name`).
+// It strips bracketed flag descriptors first so tokens like `[--tags=<csv>]`
+// don't contribute `<csv>` as a phantom positional, then drops cobra's
+// built-in `[flags]` / `[command]` placeholders.
+//
+// Returns lowercase placeholder names in source order.
+func extractPositionalPlaceholders(usageSuffix string) []string {
+	scrubbed := flagDescriptorRe.ReplaceAllString(usageSuffix, "")
+	matches := positionalPlaceholderRe.FindAllStringSubmatch(scrubbed, -1)
+	if len(matches) == 0 {
+		return nil
+	}
+	var names []string
 	for _, match := range matches {
 		name := strings.ToLower(match[1])
-		// Skip cobra built-in placeholders
 		if name == "flags" || name == "command" {
 			continue
 		}
-		cmd.Args = append(cmd.Args, syntheticArgValue(name))
+		names = append(names, name)
 	}
+	return names
 }
 
 // syntheticArgValue maps a positional arg placeholder name to a synthetic test value.
diff --git a/internal/pipeline/runtime_test.go b/internal/pipeline/runtime_test.go
index 0d512b27..a42b09ba 100644
--- a/internal/pipeline/runtime_test.go
+++ b/internal/pipeline/runtime_test.go
@@ -351,6 +351,76 @@ func main() {}
 	assert.FileExists(t, binaryPath)
 }
 
+// TestExtractPositionalPlaceholders covers the placeholder extractor used
+// by inferPositionalArgs. The bracketed-flag-descriptor cases are the
+// retro #301 F2 regression: cobra Use strings like
+// `save <url> [--tags=<csv>] [--stdin]` were leaking `<csv>` as a phantom
+// positional, which then violated MaximumNArgs(1) on save.
+func TestExtractPositionalPlaceholders(t *testing.T) {
+	tests := []struct {
+		name  string
+		usage string
+		want  []string
+	}{
+		{
+			name:  "single required positional",
+			usage: " <url> [flags]",
+			want:  []string{"url"},
+		},
+		{
+			name:  "single optional positional",
+			usage: " [id] [flags]",
+			want:  []string{"id"},
+		},
+		{
+			name:  "required plus optional positional",
+			usage: " <id> [extra] [flags]",
+			want:  []string{"id", "extra"},
+		},
+		{
+			name:  "no positionals",
+			usage: " [flags]",
+			want:  nil,
+		},
+		{
+			name:  "skips [command]",
+			usage: " [command] [flags]",
+			want:  nil,
+		},
+		{
+			name:  "F2: bracketed flag descriptor with placeholder is not a positional",
+			usage: " <url> [--tags=<csv>] [--stdin] [flags]",
+			want:  []string{"url"},
+		},
+		{
+			name:  "F2: multiple bracketed flag descriptors",
+			usage: " [--name=<n>] [--limit=<int>] [flags]",
+			want:  nil,
+		},
+		{
+			name:  "F2: short-form flag descriptor",
+			usage: " <q> [-v] [flags]",
+			want:  []string{"q"},
+		},
+		{
+			name:  "F2: spaced flag descriptor body",
+			usage: " <id> [ --debug ] [flags]",
+			want:  []string{"id"},
+		},
+		{
+			name:  "lowercases names",
+			usage: " <Region> [flags]",
+			want:  []string{"region"},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := extractPositionalPlaceholders(tt.usage)
+			assert.Equal(t, tt.want, got)
+		})
+	}
+}
+
 func TestSyntheticArgValue(t *testing.T) {
 	tests := []struct {
 		name     string
diff --git a/scripts/verify-skill/test_resolve_command_path.py b/scripts/verify-skill/test_resolve_command_path.py
new file mode 100644
index 00000000..044a8528
--- /dev/null
+++ b/scripts/verify-skill/test_resolve_command_path.py
@@ -0,0 +1,242 @@
+"""Focused unit tests for resolve_command_path and helpers.
+
+Run with: python3 -m pytest scripts/verify-skill/test_resolve_command_path.py
+or: python3 scripts/verify-skill/test_resolve_command_path.py
+
+These tests cover retro #301 finding F1: the shared-leaf disambiguation
+that the legacy specificity heuristic got wrong.
+"""
+from __future__ import annotations
+
+import sys
+import tempfile
+import unittest
+from pathlib import Path
+
+sys.path.insert(0, str(Path(__file__).parent))
+
+from verify_skill import (  # noqa: E402
+    collect_command_constructors,
+    find_root_children,
+    resolve_command_path,
+    find_command_source,
+    _extract_function_body,
+)
+
+
+def _write_cli(tmp: Path, files: dict[str, str]) -> Path:
+    """Materialize a synthetic CLI under tmp/internal/cli/<name>.go and
+    return tmp (the cli_dir)."""
+    cli_dir = tmp / "internal" / "cli"
+    cli_dir.mkdir(parents=True, exist_ok=True)
+    for name, content in files.items():
+        (cli_dir / name).write_text(content)
+    return tmp
+
+
+class TestExtractFunctionBody(unittest.TestCase):
+    def test_simple_body(self):
+        text = "func foo() {\n  return 1\n}\n"
+        body = _extract_function_body(text, text.index("{") + 1)
+        self.assertIn("return 1", body)
+
+    def test_braces_inside_string(self):
+        text = 'func foo() {\n  s := "{not a brace}"\n  return 1\n}\n'
+        body = _extract_function_body(text, text.index("{") + 1)
+        self.assertIn("return 1", body)
+
+    def test_braces_inside_raw_string(self):
+        text = "func foo() {\n  s := `{still not a brace}`\n  return 2\n}\n"
+        body = _extract_function_body(text, text.index("{") + 1)
+        self.assertIn("return 2", body)
+
+    def test_braces_inside_line_comment(self):
+        text = "func foo() {\n  // ignored brace }\n  return 3\n}\n"
+        body = _extract_function_body(text, text.index("{") + 1)
+        self.assertIn("return 3", body)
+
+    def test_braces_inside_block_comment(self):
+        text = "func foo() {\n  /* { ignored } */\n  return 4\n}\n"
+        body = _extract_function_body(text, text.index("{") + 1)
+        self.assertIn("return 4", body)
+
+    def test_unclosed_returns_none(self):
+        text = "func foo() {\n  return 5\n"  # missing closing brace
+        body = _extract_function_body(text, text.index("{") + 1)
+        self.assertIsNone(body)
+
+
+class TestCollectAndResolve(unittest.TestCase):
+    def test_resolves_top_level_when_leaf_collides_with_subcommand(self):
+        """Retro #301 F1: top-level `save <url>` and `profile save <name>`
+        share leaf 'save'. cmd_path=['save'] must resolve to the top-level
+        save_cmd.go, not profile.go's profile-save subcommand."""
+        with tempfile.TemporaryDirectory() as td:
+            cli_dir = _write_cli(Path(td), {
+                "root.go": '''package cli
+import "github.com/spf13/cobra"
+func Execute() error {
+    rootCmd := &cobra.Command{Use: "demo-pp-cli"}
+    rootCmd.AddCommand(newSaveCmd())
+    rootCmd.AddCommand(newProfileCmd())
+    return rootCmd.Execute()
+}
+''',
+                "save_cmd.go": '''package cli
+import "github.com/spf13/cobra"
+func newSaveCmd() *cobra.Command {
+    cmd := &cobra.Command{Use: "save <url>"}
+    return cmd
+}
+''',
+                "profile.go": '''package cli
+import "github.com/spf13/cobra"
+func newProfileCmd() *cobra.Command {
+    cmd := &cobra.Command{Use: "profile"}
+    cmd.AddCommand(newProfileSaveCmd())
+    return cmd
+}
+func newProfileSaveCmd() *cobra.Command {
+    return &cobra.Command{Use: "save <name> [--<flag> <value> ...]"}
+}
+''',
+            })
+
+            files, use, _ = find_command_source(cli_dir, ["save"])
+            self.assertEqual([f.name for f in files], ["save_cmd.go"])
+            self.assertEqual(use, "save <url>")
+
+            files, use, _ = find_command_source(cli_dir, ["profile", "save"])
+            self.assertEqual([f.name for f in files], ["profile.go"])
+            self.assertEqual(use, "save <name> [--<flag> <value> ...]")
+
+    def test_constructor_collection(self):
+        with tempfile.TemporaryDirectory() as td:
+            cli_dir = _write_cli(Path(td), {
+                "auth.go": '''package cli
+import "github.com/spf13/cobra"
+func newAuthCmd() *cobra.Command {
+    cmd := &cobra.Command{Use: "auth"}
+    cmd.AddCommand(newAuthLoginCmd())
+    cmd.AddCommand(newAuthLogoutCmd())
+    return cmd
+}
+func newAuthLoginCmd() *cobra.Command {
+    return &cobra.Command{Use: "login <token>"}
+}
+func newAuthLogoutCmd() *cobra.Command {
+    return &cobra.Command{Use: "logout"}
+}
+''',
+            })
+            ctors = collect_command_constructors(cli_dir)
+            self.assertEqual(set(ctors), {"newAuthCmd", "newAuthLoginCmd", "newAuthLogoutCmd"})
+            self.assertEqual(ctors["newAuthCmd"].use, "auth")
+            self.assertEqual(set(ctors["newAuthCmd"].children), {"newAuthLoginCmd", "newAuthLogoutCmd"})
+            self.assertEqual(ctors["newAuthLoginCmd"].use, "login <token>")
+
+    def test_root_children_discovery(self):
+        with tempfile.TemporaryDirectory() as td:
+            cli_dir = _write_cli(Path(td), {
+                "root.go": '''package cli
+import "github.com/spf13/cobra"
+func Execute() error {
+    rootCmd := &cobra.Command{Use: "x-pp-cli"}
+    rootCmd.AddCommand(newAuthCmd())
+    rootCmd.AddCommand(newDoctorCmd())
+    return rootCmd.Execute()
+}
+''',
+                "auth.go": '''package cli
+import "github.com/spf13/cobra"
+func newAuthCmd() *cobra.Command { return &cobra.Command{Use: "auth"} }
+''',
+                "doctor.go": '''package cli
+import "github.com/spf13/cobra"
+func newDoctorCmd() *cobra.Command { return &cobra.Command{Use: "doctor"} }
+''',
+            })
+            self.assertEqual(set(find_root_children(cli_dir)), {"newAuthCmd", "newDoctorCmd"})
+
+    def test_unresolvable_path_returns_none(self):
+        with tempfile.TemporaryDirectory() as td:
+            cli_dir = _write_cli(Path(td), {
+                "root.go": '''package cli
+import "github.com/spf13/cobra"
+func Execute() error {
+    rootCmd := &cobra.Command{Use: "x"}
+    rootCmd.AddCommand(newFooCmd())
+    return rootCmd.Execute()
+}
+''',
+                "foo.go": '''package cli
+import "github.com/spf13/cobra"
+func newFooCmd() *cobra.Command { return &cobra.Command{Use: "foo"} }
+''',
+            })
+            file, use, _ = resolve_command_path(cli_dir, ["nonexistent"])
+            self.assertIsNone(file)
+            self.assertIsNone(use)
+
+    def test_constructor_with_func_typed_param(self):
+        """Retro #303 review item #6: CONSTRUCTOR_RE must match
+        constructors whose signatures include function-typed
+        parameters like `func(int) error`. Without nested-paren
+        handling the regex stops at the first `)` of the inner
+        func type and silently drops the constructor from the
+        constructor map."""
+        with tempfile.TemporaryDirectory() as td:
+            cli_dir = _write_cli(Path(td), {
+                "callback.go": '''package cli
+import "github.com/spf13/cobra"
+func newCallbackCmd(handler func() error, fallback func(int) string) *cobra.Command {
+    return &cobra.Command{Use: "callback"}
+}
+''',
+            })
+            from verify_skill import collect_command_constructors
+            ctors = collect_command_constructors(cli_dir)
+            self.assertIn("newCallbackCmd", ctors,
+                "regex must handle func() and func(int) parameter types")
+            self.assertEqual(ctors["newCallbackCmd"].use, "callback")
+
+    def test_collect_command_constructors_is_cached(self):
+        """Retro #303 review item #5: collect_command_constructors
+        is wrapped with lru_cache so verify-skill's per-recipe
+        find_command_source loop doesn't re-scan internal/cli/*.go
+        for every recipe. The cache key is the cli_dir Path."""
+        from verify_skill import collect_command_constructors
+        with tempfile.TemporaryDirectory() as td:
+            cli_dir = _write_cli(Path(td), {
+                "foo.go": '''package cli
+import "github.com/spf13/cobra"
+func newFooCmd() *cobra.Command { return &cobra.Command{Use: "foo"} }
+''',
+            })
+            first = collect_command_constructors(cli_dir)
+            second = collect_command_constructors(cli_dir)
+            self.assertIs(first, second,
+                "second call must return the cached object, not rescan")
+
+    def test_legacy_fallback_when_no_root_addcommand(self):
+        """When the CLI doesn't follow the standard rootCmd.AddCommand
+        pattern (no root.go, or different convention), the legacy
+        specificity heuristic still finds something usable."""
+        with tempfile.TemporaryDirectory() as td:
+            cli_dir = _write_cli(Path(td), {
+                # No root.go — just a single command file
+                "search.go": '''package cli
+import "github.com/spf13/cobra"
+func newSearchCmd() *cobra.Command {
+    return &cobra.Command{Use: "search <query>"}
+}
+''',
+            })
+            files, use, _ = find_command_source(cli_dir, ["search"])
+            # Legacy fallback returns the file; not empty
+            self.assertEqual([f.name for f in files], ["search.go"])
+            self.assertEqual(use, "search <query>")
+
+
+if __name__ == "__main__":
+    unittest.main()
diff --git a/scripts/verify-skill/verify_skill.py b/scripts/verify-skill/verify_skill.py
index d266ab4c..7f3a41b0 100755
--- a/scripts/verify-skill/verify_skill.py
+++ b/scripts/verify-skill/verify_skill.py
@@ -43,6 +43,7 @@ import re
 import shlex
 import sys
 from dataclasses import dataclass, field
+from functools import lru_cache
 from pathlib import Path
 from typing import Iterable
 
@@ -117,18 +118,259 @@ def parse_use(use_str: str) -> tuple[str, int, int, bool]:
     return name, required, optional, variadic
 
 
+# CONSTRUCTOR_RE matches `func newXxxCmd(...) *cobra.Command {`. The
+# parameter list pattern `\([^()]*(?:\([^()]*\)[^()]*)*\)` handles one
+# level of nested parens — the case that matters in practice is a
+# function-typed parameter like `func newFooCmd(handler func() error)
+# *cobra.Command`. Without the nested-paren handling the regex stops
+# at the first `)` (the closer of `func()`) and the constructor is
+# silently dropped from the constructor map. Two-level nesting (e.g.
+# `func()` inside another `func()`) would still fail; flag for the
+# legacy fallback when that pathological shape appears.
+CONSTRUCTOR_RE = re.compile(
+    r'^func\s+(new[A-Z]\w*Cmd)\s*'
+    r'\([^()]*(?:\([^()]*\)[^()]*)*\)'
+    r'\s*\*cobra\.Command\s*\{',
+    re.MULTILINE,
+)
+ADDCMD_CHILD_RE = re.compile(r'\.AddCommand\s*\(\s*(new[A-Z]\w*Cmd)\s*\(')
+ROOT_ADDCMD_RE = re.compile(r'rootCmd\.AddCommand\s*\(\s*(new[A-Z]\w*Cmd)\s*\(')
+
+
+def _extract_function_body(text: str, start_offset: int) -> str | None:
+    """Given the offset just after the opening `{` of a function body,
+    return the body text (excluding the closing `}`). Tracks string and
+    comment state so braces inside string literals or comments don't
+    confuse the depth counter. Returns None if the body is unclosed.
+    """
+    depth = 1
+    i = start_offset
+    n = len(text)
+    in_string: str | None = None  # holds the active string opener: '"', '`', or "'"
+    in_line_comment = False
+    in_block_comment = False
+    while i < n and depth > 0:
+        c = text[i]
+        if in_line_comment:
+            if c == '\n':
+                in_line_comment = False
+            i += 1
+            continue
+        if in_block_comment:
+            if c == '*' and i + 1 < n and text[i + 1] == '/':
+                in_block_comment = False
+                i += 2
+                continue
+            i += 1
+            continue
+        if in_string is not None:
+            if c == '\\' and in_string != '`' and i + 1 < n:
+                # Skip the escaped char (still inside the string)
+                i += 2
+                continue
+            if c == in_string:
+                in_string = None
+            i += 1
+            continue
+        if c == '/' and i + 1 < n:
+            if text[i + 1] == '/':
+                in_line_comment = True
+                i += 2
+                continue
+            if text[i + 1] == '*':
+                in_block_comment = True
+                i += 2
+                continue
+        if c in ('"', '`', "'"):
+            in_string = c
+            i += 1
+            continue
+        if c == '{':
+            depth += 1
+        elif c == '}':
+            depth -= 1
+        i += 1
+    if depth != 0:
+        return None
+    return text[start_offset:i - 1]
+
+
+@dataclass
+class CommandConstructor:
+    name: str
+    file: Path
+    use: str
+    args_info: tuple | None
+    children: list[str] = field(default_factory=list)
+
+
+@lru_cache(maxsize=None)
+def collect_command_constructors(cli_dir: Path) -> dict[str, CommandConstructor]:
+    """Scan internal/cli/*.go for `func newXxxCmd(...) *cobra.Command`
+    declarations. For each, capture the Use string, the cobra.Args
+    validator, and the list of child constructor names called via
+    `<var>.AddCommand(newYyyCmd(...))` within the function body.
+
+    Result maps constructor name → CommandConstructor.
+
+    Cached per cli_dir for the lifetime of the process. verify-skill
+    invokes find_command_source once per recipe in SKILL.md (typically
+    5-15) and the source tree doesn't change mid-run, so the
+    file-system scan only needs to happen once. Callers must not
+    mutate the returned dict (it's the cache's storage).
+    """
+    src = cli_dir / "internal/cli"
+    if not src.exists():
+        return {}
+    constructors: dict[str, CommandConstructor] = {}
+    for go_file in src.glob("*.go"):
+        if go_file.name.endswith("_test.go"):
+            continue
+        try:
+            text = go_file.read_text()
+        except Exception:
+            continue
+        for m in CONSTRUCTOR_RE.finditer(text):
+            fn_name = m.group(1)
+            body = _extract_function_body(text, m.end())
+            if body is None:
+                continue
+            use_match = USE_RE.search(body)
+            if not use_match:
+                continue
+            args_match = ARGS_RE.search(body)
+            args_info = (args_match.group(1), args_match.group(2)) if args_match else None
+            children = list(dict.fromkeys(
+                child.group(1) for child in ADDCMD_CHILD_RE.finditer(body)
+            ))
+            constructors[fn_name] = CommandConstructor(
+                name=fn_name,
+                file=go_file,
+                use=use_match.group(1),
+                args_info=args_info,
+                children=children,
+            )
+    return constructors
+
+
+@lru_cache(maxsize=None)
+def find_root_children(cli_dir: Path) -> list[str]:
+    """Return the constructor names called from `rootCmd.AddCommand(...)`
+    anywhere in internal/cli/*.go. The ordering follows source order, but
+    the result is deduplicated.
+
+    Cached per cli_dir; callers must not mutate the returned list.
+    See collect_command_constructors for rationale."""
+    src = cli_dir / "internal/cli"
+    if not src.exists():
+        return []
+    seen: dict[str, None] = {}
+    for go_file in src.glob("*.go"):
+        if go_file.name.endswith("_test.go"):
+            continue
+        try:
+            text = go_file.read_text()
+        except Exception:
+            continue
+        for m in ROOT_ADDCMD_RE.finditer(text):
+            seen.setdefault(m.group(1), None)
+    return list(seen)
+
+
+def resolve_command_path(
+    cli_dir: Path,
+    cmd_path: list[str],
+    constructors: dict[str, CommandConstructor] | None = None,
+    root_children: list[str] | None = None,
+):
+    """Walk the AddCommand graph to find the canonical declaring file for
+    cmd_path. Returns (file, use_str, args_info) or (None, None, None) if
+    the path can't be resolved (unknown command, unconventional CLI).
+
+    This is the durable replacement for the old specificity-based
+    disambiguation in find_command_source. It picks the file based on
+    actual command-tree structure rather than guessing from `Use:` token
+    counts. See retro #301 finding F1.
+    """
+    if not cmd_path:
+        return None, None, None
+    if constructors is None:
+        constructors = collect_command_constructors(cli_dir)
+    if root_children is None:
+        root_children = find_root_children(cli_dir)
+    if not constructors or not root_children:
+        return None, None, None
+
+    current = None
+    for fn_name in root_children:
+        info = constructors.get(fn_name)
+        if info is None:
+            continue
+        leaf, _, _, _ = parse_use(info.use)
+        if leaf == cmd_path[0]:
+            current = info
+            break
+    if current is None:
+        return None, None, None
+
+    for token in cmd_path[1:]:
+        next_info = None
+        for child_fn in current.children:
+            child = constructors.get(child_fn)
+            if child is None:
+                continue
+            leaf, _, _, _ = parse_use(child.use)
+            if leaf == token:
+                next_info = child
+                break
+        if next_info is None:
+            return None, None, None
+        current = next_info
+
+    return current.file, current.use, current.args_info
+
+
 def find_command_source(cli_dir: Path, cmd_path: list[str]):
-    """Locate source file(s) whose cobra.Command matches this path.
-
-    Returns (go_files, use_str, args_info) where go_files is a list.
-    Why a list: CLIs sometimes declare the same Use in two files (historical
-    artifact or generator + transcendence both shipping a version of the
-    same command). Cobra only registers one at runtime, but we don't know
-    which without parsing root.go's AddCommand calls. Returning all matching
-    files lets callers union their flags when checking declarations.
+    """Locate the source file whose cobra.Command matches this path.
+
+    Returns (go_files, use_str, args_info) where go_files is a list (kept
+    list-shaped for backwards compatibility — most callers iterate it for
+    `flag_declared_in` lookups).
+
+    Resolution strategy:
+
+      1. Walk the rootCmd.AddCommand graph (the durable approach added in
+         retro #301 F1). When the CLI follows the standard `func newXxxCmd`
+         + `<parent>.AddCommand(newXxxCmd(...))` convention, this returns
+         exactly one file per command path with no false positives, even
+         when two different commands share a leaf name (e.g.,
+         `recipe-goat-pp-cli save` vs `recipe-goat-pp-cli profile save`).
+
+      2. If the graph walk fails (unconventional CLI, missing rootCmd,
+         constructor functions not named `newXxxCmd`), fall back to a
+         legacy specificity heuristic that scans every Go file for any
+         `Use:` whose first token matches the leaf. The legacy path is
+         imperfect (it can pick the wrong file when leaves collide) but
+         keeps the tool useful on CLIs that don't follow the standard
+         convention.
     """
     if not cmd_path:
         return [], None, None
+
+    file, use_str, args_info = resolve_command_path(cli_dir, cmd_path)
+    if file is not None:
+        return [file], use_str, args_info
+
+    # Legacy fallback — kept to preserve behavior for unconventional CLIs.
+    return _legacy_find_command_source(cli_dir, cmd_path)
+
+
+def _legacy_find_command_source(cli_dir: Path, cmd_path: list[str]):
+    """Pre-retro-#301 specificity-based heuristic. Retained as a fallback
+    for CLIs whose command structure doesn't match the standard
+    `rootCmd.AddCommand(newXxxCmd(...))` pattern resolve_command_path
+    expects (e.g., commands constructed via local helpers, or files that
+    declare cobra.Commands without going through a `newXxxCmd` factory)."""
     leaf = cmd_path[-1]
     src = cli_dir / "internal/cli"
     if not src.exists():
@@ -157,17 +399,12 @@ def find_command_source(cli_dir: Path, cmd_path: list[str]):
     if not candidates:
         return [], None, None
 
-    # Multi-token paths: prefer filename-match (e.g., contacts_search.go for
-    # cmd_path ["contacts", "search"]).
     if len(cmd_path) >= 2:
         expected_basename = "_".join(cmd_path).replace("-", "_") + ".go"
         for spec, go_file, use_str, args_info in candidates:
             if go_file.name == expected_basename:
                 return [go_file], use_str, args_info
 
-    # Single-token paths (or no filename match): take all files at the
-    # highest specificity tier. For flag verification, any one of them
-    # declaring the flag counts. For Use string, pick the representative.
     candidates.sort(key=lambda c: -c[0])
     top_spec = candidates[0][0]
     top_files = [c[1] for c in candidates if c[0] == top_spec]
diff --git a/skills/printing-press/SKILL.md b/skills/printing-press/SKILL.md
index 171dcce8..232aa6a1 100644
--- a/skills/printing-press/SKILL.md
+++ b/skills/printing-press/SKILL.md
@@ -476,6 +476,28 @@ Before new research:
    If the user picks option 2, invoke `/printing-press-polish <api>` to improve the existing CLI.
    If the user picks option 3, display the prior research, then re-present options 1 and 2.
 
+   **MANDATORY when re-using prior research after a binary upgrade.** If the user picks "Generate a fresh CLI" (option 1) AND `PRESS_VERSION` from the manifest differs from the current binary's version (parse both via semver and compare; only fire when the leading minor or major segment changed — patch-level deltas don't trigger this), prompt the user once before kicking off Phase 1 research.
+
+   Construct the prompt's "what changed" list from these category buckets — the categories are stable across versions; the specific machine deltas inside each category are not. Read `docs/CHANGELOG.md` (or run `git log --oneline v<PRESS_VERSION>..v<CURRENT> -- internal/`) and tag each notable change to one of these buckets:
+
+   | Category | Affects prior-brief assumption about... |
+   |---|---|
+   | **Transport / reachability** | Which sources are reachable, what auth/clearance is needed, which clients (stdlib, Surf, browser-clearance) the brief assumed |
+   | **Scoring rubrics** | What Phase 1.5/scorecard dimensions the brief targets, whether prior "high-priority" features still rank as such |
+   | **Auth modes** | Whether brief's auth choice (api-key, cookie, composed, oauth) is still the right pick, whether new modes unlock new endpoints |
+   | **MCP surface** | Whether brief's MCP shape (endpoint-mirror vs intent vs code-orchestration) matches the latest emit defaults |
+   | **Discovery** | Whether browser-sniff / crowd-sniff workflows changed, whether prior gate decisions are still valid |
+
+   For the prompt itself, list only the buckets that have at least one notable change between the two versions. If the CHANGELOG / git log is unavailable, list all five buckets generically and let the user decide.
+
+   > "The prior `<api>` was generated with printing-press v`<PRESS_VERSION>`. The current binary is v`<CURRENT>`. Categories where the machine has changed since then: `<applicable buckets>`. Each can invalidate prior research assumptions. Re-validate the prior brief against the current machine before reusing it?"
+
+   Options:
+   1. **Yes, re-validate the prior research** — fold the validation into Phase 1 (briefly re-probe reachability for previously-blocked sources, confirm scoring still classifies the prior CLI's pattern correctly, etc.) before reusing the brief.
+   2. **No, reuse the prior research as-is** — proceed with the brief verbatim, even if the underlying machine assumptions are stale.
+
+   The prompt forces the user to acknowledge the version delta and explicitly accept (or refuse) re-validation. Skip it entirely on first generation, on same-version regenerations, or when no prior manifest exists.
+
    If no CLI exists in the library and no lock is active, skip this step and proceed normally.
 
 5. **API Key Gate** — Check whether this API requires authentication, then handle accordingly.
@@ -1161,6 +1183,7 @@ For each tool, fill in what you know from the research. Stars and command_count
 8. `recipes` are 3–5 worked examples rendered in SKILL.md. Each has a title, a real command, and a one-line explanation. Prefer recipes that exercise novel features. **At least one recipe must pair `--agent` with `--select`** — using dotted paths (e.g. `--select events.shortName,events.competitions.competitors.team.displayName`) when the response is deeply nested. APIs like ESPN, HubSpot, and Linear return tens of KB per call; without a `--select` recipe, agents burn context parsing verbose payloads. Pick a command known to return a large or deeply nested response and show the narrowing pattern.
 9. `trigger_phrases` are natural-language phrases a user might say that should invoke this CLI's skill. Include 3–5 domain-specific phrases (e.g. for a finance CLI: "quote AAPL", "check my portfolio", "options for TSLA") and 2 generic phrases ("use <api-name>", "run <api-name>"). Domain verbs vary — don't just template "use X" variants.
 10. All `narrative` fields are optional. Omit fields you can't populate honestly rather than emit filler. The generator falls back to generic content gracefully.
+11. **Avoid hardcoded counts in narrative copy when the count tracks a runtime list.** A number embedded in `headline` or `value_prop` ("across N trusted sources", "from N retailers", "queries N vendors") propagates into root.go's Short/Long, the README, the SKILL, the MCP tools description, and `which.go` — every output surface that reads the narrative. When the underlying registry grows or shrinks, the count goes stale across all of those surfaces simultaneously, and a single-line edit to add a source requires hunting down ~10 hardcoded copies. Prefer plural-without-count phrasing ("across the major sources", "from a curated set of retailers") or describe the breadth qualitatively ("dozens of vendors") rather than committing to a specific integer. If a count is load-bearing for the value prop, keep the brief's narrative count-free and have the printed-CLI's README/SKILL author write the count once into a single hand-edited paragraph after generation — accepting that it will need a manual update whenever the registry changes.
 
 Also write discovery pages if browser-sniff was used. The generator reads these from `$API_RUN_DIR/discovery/browser-sniff-report.md` (which the browser-sniff gate already writes there). No additional action needed for discovery pages -- they are already in the right location.
 

← 574fc27d chore(main): release 2.3.7 (#299)  ·  back to Cli Printing Press  ·  docs(cli): refresh README for launch (Apr 28) (#306) 0d4d2cba →