Skip to content

Extraction changed three behaviours: whole-repo check returns a false green, review loses its workspace root, help loses its defaults #4

Closed

Description

@schickling-assistant

The extraction of the checker into crates/intent changed three behaviours relative to the
implementation it was lifted from. One of them makes a documented whole-repository invocation
return a false green. All three were found by an independent review of the consuming change and
re-derived at source; none is a regression introduced after the merge.

1. Whole-repository check silently stops enforcing meta-VRS decision shape (high)

The contract documents whole-repository scope for check [path]. The original resolved the
decision directory with a fallback:

let direct = root.join(".decisions");
if direct.is_dir() { direct } else { root.join("context/vrs/.decisions") }

crates/intent/src/lib.rs keeps only root.join(".decisions").

Repro: place a malformed decision at repo/context/vrs/.decisions/0001-bad.md and run
check repo --profile local --json. Expected a blocking VRS.ENF.meta-decision-shape
diagnostic; actual is that repo/.decisions is absent, the decision-shape pass is skipped, and
no other recursive Markdown pass performs the shape check. The result is a pass.

No test covers it: every helper on both sides passes repo/context/vrs directly, so the ported
tests cannot reach this path. A green port audit of those tests is not evidence about this
branch.

Either restore the fallback, or narrow the documented scope deliberately and say so in the
contract — but the current state documents one behaviour and implements another.

2. review_workspace loses its context/vrs sentinel (medium)

The original found the repository root from either a .git directory or the nearest ancestor
containing context/vrs. The extracted version keeps only the .git probe and otherwise falls
back to the corpus root — which is then passed as the review backend's --cwd and used as the
target-artifact escape boundary.

In a vendored or exported source tree with no .git, review is handed the corpus root instead
of the repository root, hiding repository context from the reviewer and rejecting target
artifacts elsewhere in the repository. Ordinary git worktrees mask this entirely, which is why
differential runs did not surface it.

3. Displayed defaults disappeared from --help and completions (medium)

To let the standalone tool and its embedding host carry different defaults, the arguments moved
from clap default_value to Option<PathBuf>, with defaults applied inside run_with.

Bare invocation still resolves correctly, but check|graph|review|review-fixtures --help no
longer advertises the documented defaults, and generated completion metadata no longer contains
them. No help or completion test covers this, so nothing fails.

Findings 2 and 3 are reported as described by the review and have not been independently
reproduced.

Related

The rename decision covering this corpus rules that the *.vN schema strings move along with
everything else. Those strings are emitted by the binary, so that is a code change here plus an
assertion change in the consumer — it is not a documentation-only pass, and it should not be
folded into an identifier-rename commit. Tracked separately from this issue.

Posted on behalf of @schickling
field value
agent_name unknown
agent_tool Claude Code
agent_tool_version 2.1.220
agent_runtime Claude Code 2.1.220
agent_session_lookup unavailable
tooling_profile dotfiles@unknown-dirty

Activity

  1. schickling-assistant commented on Aug 8, 2026

    @schickling-assistant
    ContributorAuthor

    Correcting one claim in this issue, and recording what did and did not reproduce. Addressed by #6.

    Finding 2 — half of what I wrote is wrong

    I reported that losing the context/vrs sentinel makes review "reject target artifacts elsewhere in the repo". That consequence does not reproduce, and I should not have asserted it — I relayed it from a review rather than deriving it, and it does not survive contact with the code.

    Target files come from markdown_files(invocation.root), which descends from the corpus root only, and the workspace is always that root or an ancestor. So every target starts with the workspace under either resolution, and the escape check cannot fire for review.

    What does reproduce is the mechanism: without the sentinel, a corpus in a tree with no .git resolves to the corpus root, and that value becomes the review backend's --cwd — so the reviewer loses the repository around the corpus. That is the real defect, and it is fixed.

    Finding 1 — fixed, and a contract question is still open

    The fallback is restored and the false green is gone. The new test invokes check at the repository root and fails on unmodified code with diagnostics: [] — a clean pass on a tree holding a malformed decision.

    But the contract question this issue raised is not settled by the fix. context/cli/spec.md:174 names the rule's subject as context/vrs/.decisions/, which reads as narrower than whole-repository scope. The fallback was restored because it is strictly additive — consulted only when <root>/.decisions is absent, naming a path a differently laid out repository does not have — so it can only add enforcement and is safe under either reading. Whether the documented scope should instead be narrowed remains open and is deliberately not resolved by #6.

    Finding 3 — fixed for the standalone binary only, by design

    --help renders from whichever command tree the caller builds. #6 supplies the mechanism and fixes the shipped binary; an embedding host gains the defaults only once it adopts the new helpers, which is a change in the host's own repository. The host half needs an owner and is not closed.

    Two follow-ups worth their own issues

    1. check_root still skips the decision-shape pass silently when no decision directory is found anywhere. The fallback fixes the reported case, but "found nothing" still exits 0 and is indistinguishable from a clean tree. Making that observable needs a new diagnostic class.
    2. The reusable engine is still coupled to the clap request types, contrary to the checked-in boundary. Not a contained change.

    On how this was found

    Worth recording, because it generalises: the original port audit compared the ported tests for assertion strength, found them byte-identical, and passed. That audit was structurally incapable of catching finding 1 — every helper on both sides passes the corpus path directly, so no test on either side ever exercised the whole-repository path. Byte-identical test bodies prove assertion strength and say nothing about behaviour on a path the tests never reach.

    Posted on behalf of @schickling
    field value
    agent_name unknown
    agent_tool Claude Code
    agent_tool_version 2.1.220
    agent_runtime Claude Code 2.1.220
    agent_session_lookup unavailable
    tooling_profile dotfiles@unknown-dirty
  2. schickling-assistant commented on Aug 9, 2026

    @schickling-assistant
    ContributorAuthor

    Correction and closure accounting

    Two things about this issue are wrong as it stands, and both are mine.

    1. The downstream-exposure framing was overstated — measured, not argued

    This issue's framing led to a claim, which I propagated, that a green check on the consuming
    repository was non-evidence for decision-shape enforcement while it pinned a pre-fix revision. That
    was measured and it does not hold.

    An A/B on identical host code with only the pin differing, with a malformed decision planted in the
    corpus, caught it on both pins — 4 VRS.ENF.meta-decision-shape diagnostics, exit 1, either way.
    The mechanism: the host passes the corpus root explicitly
    (intent::run_with(vrs, &intent::Defaults::corpus_root("context/vrs"))), and
    root_or_default(arg) = arg.unwrap_or_else(|| corpus_root), so the fallback this issue is about is
    never consulted from the host. Every enforcement path in the consuming repository names the corpus
    rather than a repository root, so none of them was exposed.

    The regression is real, and narrower than described: aim the standalone binary at a repository
    root and the shape pass really is absent (0 diagnostics of that rule pre-fix, 4 post-fix). Even there
    it is not a false green — that run exits 1 on ~955 other diagnostics. "Returns a false green" in
    the title is accurate only for a minimal tree with no other findings, which is the test harness, not
    a populated repository.

    2. This issue was auto-closed, and two findings had not actually been resolved

    Closure came from a merge reference, not from a judgement that all three findings were done. Both
    residues are now tracked:

    Leaving this issue closed, since both open parts now have their own homes and reopening it would only
    re-close on the next merge that references it.

    Posted on behalf of @schickling
    field value
    agent_name unknown
    agent_tool Claude Code
    agent_tool_version 2.1.220
    agent_runtime Claude Code 2.1.220
    agent_session_lookup unavailable
    tooling_profile dotfiles@unknown-dirty
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions