TimZander / TimZander/claude

deep-review: a repo with no origin silently drops an explicitly passed story reference

Open
#228 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Python
Stars
0
Forks
1
Avg merge
1d 3h
Merged PRs (30d)
7

Description

Follow-up from #220 / #224. Found during round-4 review; deliberately not fixed there, because that branch already carries one contract change.

Problem

In a repository with no origin remote, an explicitly passed story reference is dropped without a word.

$ cd /repo-with-no-origin
$ resolve-pr.sh --args "https://github.com/TimZander/claude/issues/42"
KIND=none
WORKITEM_KIND=none
REFERENCE_REFUSED=false

The user named a story. The script neither accepted it nor refused it — and REFERENCE_REFUSED=false tells the consumer that nothing was referenced. commands/deep-review.md then reports "no linked story found," and the review grades the diff against nothing.

This is the precise failure the locality guard exists to prevent. The README's own framing:

A refusal must never look like an absence.

Root cause

reference_url_is_local() is tri-state:

return meaning
0 reference is local — accept
1 reference is foreign — refuse, set REFERENCE_REFUSED=true
2 our own origin is unparsable or absent — cannot tell

The tri-state was introduced in #224 to stop blaming the user when the fault is ours: before it, an unreadable origin made every reference look foreign. That fix was correct, but the 2 case now falls through every call site's if/elif and lands in neither branch. The reference is discarded and no key records that it existed.

So the guard swapped one failure mode (false refusal) for its mirror (silent absence) rather than reaching a third state.

Note this is not an exotic case: no-origin is the ordinary local/offline repo, and git init with no remote is the first thing a new plugin or spike looks like.

Options

  1. Accept when origin is unknowable. Cheapest. With no origin there is no "wrong repo" to be confused with, so the number cannot resolve to someone else's story in the way the guard was built to prevent. Slightly looser, and wrong the moment a remote is added later.
  2. Report it distinctly — e.g. REFERENCE_REFUSED=unverifiable — so the review can say "you named a story; I could not confirm it belongs to this repo" and still fetch it, flagged. Costs one enum value and one more consumer branch in deep-review.md.

Preference is (2). It preserves the invariant the whole feature rests on, and "can't tell" is already a first-class answer elsewhere in this feature — the acceptance-criteria checklist has exactly the same three-way shape.

Acceptance criteria

  1. In a repo with no origin, an explicitly passed issue/work-item reference is not silently discarded — the resolver output distinguishes it from the no-reference-given case.
  2. commands/deep-review.md reports the distinction, and never renders it as "no linked story found."
  3. The same holds for an origin whose URL is present but unparsable — not only for a missing remote.
  4. scripts/test_resolve-pr.sh covers both shapes. Neither is exercised today.

Notes

scripts/test_resolve-pr.sh has a NO_REMOTE_REPO fixture already, but no test asserts this behaviour through it — which is why the suite was green at 233/0 with the gap present.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Read reference_url_is_local() and its call sites in resolve-pr.sh, then inspect commands/deep-review.md for how REFERENCE_REFUSED is rendered. Run scripts/test_resolve-pr.sh with the existing NO_REMOTE_REPO fixture and an unparsable-origin case; done means both references remain distinguishable from no reference and deep-review never reports them as absent.

Written by the indexing model from the issue text.

Assessment

Tech stack
git, shell
Domain
cli, testing
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
70/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.