Repository navigation
Require opt-in before adopting cited GitHub issue items - #169
Conversation
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configuration
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Reviewer's GuideThe PR changes GitHub issue import identity resolution to adopt one eligible existing item citing the exact issue, while preserving structured ownership precedence and adding strict ambiguity/no-write safeguards, opt-out support, distinct adoption accounting, comprehensive CLI/atomic/gated regression coverage, and validation documentation. Sequence diagram for a gated citation adoption importsequenceDiagram
participant CLI
participant Importer
participant Tracker
participant GitHub
CLI->>Importer: runImport
Importer->>GitHub: fetch issue
Importer->>Tracker: read existing items
Importer->>Importer: matchImportedIssue
alt one exact citation candidate
Importer->>Importer: build adopt plan
Importer->>Importer: verifyImportIdempotency
Importer->>Tracker: upsert existing item with provenance
Tracker-->>Importer: landing item updated
Importer-->>CLI: adopted count and gate receipt
else multiple citation candidates
Importer-->>CLI: ambiguous_citation with candidate IDs
Note over Importer,Tracker: No tracker write occurs
end
State diagram for GitHub issue import identitystateDiagram-v2
[*] --> StructuredMatch
StructuredMatch --> Reused: tag, URL, or provider ownership
StructuredMatch --> CitationSearch: no established owner
CitationSearch --> NewItem: no exact citation
CitationSearch --> Adopted: one eligible citation
CitationSearch --> Blocked: multiple citations
CitationSearch --> NewItem: --no-adopt-cited
Reused --> Upserted
Adopted --> Upserted
NewItem --> Upserted
Blocked --> [*]
Upserted --> [*]
Flow diagram for cited GitHub issue adoptionflowchart TD
A["Prepare GitHub issue import"] --> B{"Structured identity match?"}
B -->|Yes| C["Reuse established owner"]
B -->|No| D{"Citation adoption enabled?"}
D -->|No| E["Create new item"]
D -->|Yes| F["Find exact URL or owner/repo#N citation"]
F --> G{"Candidate count"}
G -->|0| E
G -->|1| H["Adopt existing item"]
G -->|2 or more| I["Refuse batch: ambiguous_citation"]
C --> J["Normal upsert and provenance fields"]
H --> J
E --> J
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
unbraind
left a comment
There was a problem hiding this comment.
VERDICT: BLOCK
FINDINGS:
- [blocking] import-fidelity.ts:68 — Citation matching never checks fix/track intent — an untagged closed item containing only
Related: acme/widgets#30is adopted, letting import overwrite its content and reopen it when issue 30 is open. - [blocking] import-fidelity.ts:71 — The matcher accepts any matching reference without excluding items citing several issues — importing only issue 30 adopts a roadmap containing
acme/widgets#30 and acme/widgets#31, bypassing the batch conflicting-target check and allowing issue 30 to close the roadmap.
SUMMARY: The PR adds default citation adoption, ambiguity refusal, adoption receipts, and an opt-out while retaining structured matching precedence. The inspected tests cover positive adoption, retries, ambiguity, and batch target conflicts, but omit incidental mentions and single-issue imports against multi-issue items; tests were not run.
Review by a read-only codex review agent (gpt-6.1-sol), posted by the orchestrator; the PR returns to its author agent for the fixes.
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
unbraind
left a comment
There was a problem hiding this comment.
VERDICT: BLOCK
FINDINGS:
- [blocking] import-fidelity.ts:81 — The regex ignores second-issue URLs ending in
/, soFixes acme/widgets#30 and https://github.com/acme/widgets/issues/31/incorrectly permits adoption for issue 30, allowing the multi-issue item's content to be overwritten and its closed status reopened.
SUMMARY: The PR restricts adoption to intent-bearing single-issue items and reports incidental citations separately. Tests cover the earlier failures, precedence and ambiguity, but omit the trailing-slash bypass reproduced with the head matcher.
Re-review by a read-only codex review agent (gpt-6.1-sol), posted by the orchestrator; back to the author agent.
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
unbraind
left a comment
There was a problem hiding this comment.
VERDICT: BLOCK
FINDINGS:
- [blocking] import-fidelity.ts:78 — Closing Markdown emphasis markers remain inside URL tokens, hiding a second issue reference —
Fixes acme/widgets#30 and **https://github.com/acme/widgets/issues/31**incorrectly selects adoption, allowing import to overwrite the multi-issue item's content and status.
SUMMARY: The PR adopts existing single-issue items while preserving structured ownership precedence and requiring citation intent. The reference matrix and intent/precedence checks pass in isolation, but bold and italic second-URL regressions fail.
Third re-review by a read-only codex review agent (gpt-6.1-sol), posted by the orchestrator; back to the author agent with a fail-safe design change.
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
unbraind
left a comment
There was a problem hiding this comment.
VERDICT: BLOCK
FINDINGS:
- [blocking] import-fidelity.ts:122 — Same-number references to another repository can still permit adoption —
Fixes acme/widgets#30 and **https://github.com/other/widgets/issues/30**passes the numeric veto while the strict parser misses the bold URL, allowing import to overwrite a multi-issue item's content and status.
SUMMARY: The PR reuses existing single-issue items through citation adoption with conservative exclusion and explicit intent checks. Tests cover extensive reference shapes and non-git trackers but omit Markdown-wrapped foreign references sharing the target number.
Fourth re-review by a read-only codex review agent (gpt-6.1-sol), posted by the orchestrator; back to the author agent.
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
unbraind
left a comment
There was a problem hiding this comment.
VERDICT: BLOCK
FINDINGS:
- [blocking] import-fidelity.ts:104 — Encoded second references bypass both parsers:
Fixes acme/widgets#30 and [other](https://github.com/other/widgets/issues/31)returnsvia: "citation"(as doesissues%2F31), allowing import to overwrite or reopen a multi-issue item.
SUMMARY: The PR reuses existing untagged items through conservative citation matching and normal import updates. Tests cover many literal reference decorations but miss encoded references; theIssue: <url>positive control still adopts.
Fifth re-review by a read-only codex review agent (gpt-6.1-sol), posted by the orchestrator. Design decision for the next round: normalize, allowlist anchored forms, and make adoption opt-in.
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
CodeQL flagged the anchored issue-label URL pattern as a polynomial regular expression: the optional query group and the trailing [.)]* tail could both match the same characters. The allowed tail (>, then . or ), then spaces/tabs) is now trimmed by hand before an anchored pattern without overlapping quantifiers. A test pins the accepted and refused tails and a 50,000-character hostile input.
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
With --adopt-cited, an untagged item whose description said 'Fixes acme/widgets#30' while its github_url named issue 31 was adopted for #30, overwriting the identity of #31. Items with structured ownership (a GitHub issue URL in github_url, provenance or external links, a github_number, provider provenance, or a native 'GitHub issue <url>' declaration) are now reported as citation candidates with reason structured_owner and are never adopted from prose.
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
…tion An untagged item whose external_ref was gh:acme/widgets#31 could still be adopted for #30 from a 'Fixes acme/widgets#30' description. Any non-empty external_ref (gh:, linear:, jira:, ado: or other providers) now marks the item as already owned, so prose never re-owns it.
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
source_owner (and provider identity fields) were still not treated as ownership, so an item natively owned by Linear could be adopted and reopened from a 'Fixes acme/widgets#30' description. The adoption guard now recognizes every source identity.ts uses: github_* fields, github_link_only false, external_ref, source_owner, linear_/jira_/ado_ identity fields, provider provenance, native declarations and GitHub issue URLs in links.
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
…adoption guard The owner matcher accepts a githuburl alias that the adoption guard did not treat as ownership. Any github-prefixed field (except github_link_only) now counts as ownership, and the owner-link field pattern is a single constant used by both checks so they cannot drift.
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
…doption identity.ts recognizes GH:owner/repo#N case-insensitively, while the adoption filter only parsed lowercase provenance tags, so an item tagged GH:other/widgets#31 could be adopted for another issue. Any well-formed GitHub identity tag, in any case, now marks the item as owned.
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
unbraind
left a comment
There was a problem hiding this comment.
Orchestrator review (built-in Claude review agent, verified by the orchestrator; bots are not waited for since 2026-10-22):
Pre-merge review: eleven read-only codex review rounds (gpt-6.1-sol) plus orchestrator fixes for rounds 6-10.
Rounds 6-10 each found one structured-ownership source that citation adoption did not respect (github_url conflict, core external_ref, source_owner and provider identity fields, the githuburl alias, uppercase GH: tags). The guard now mirrors every ownership source identity.ts and the owner matcher recognize, and both share one owner-link field constant. Each fix has a regression test that fails on revert. CodeQL's polynomial-regex alert on the issue-label URL tail is fixed by linear trimming. Adoption stays opt-in and off by default in the CLI, SDK and reusable workflow.
Final round:
VERDICT: MERGE
FINDINGS:
- [non-blocking] README.md:988 — The claim that plain imports may write earlier issues before rejecting later ambiguity is stale — all imports now plan the entire batch before writing.
SUMMARY: The PR requires explicit opt-in for citation adoption and protects existing structured ownership, including mixed-case GitHub identity tags. The reviewed tests cover defaults, ownership guards, normalization, workflow propagation and non-git persistence; I did not execute them.
GitHub issue import now keeps normal create behavior unless citation adoption is explicitly enabled. The default path reports read-only
citation_candidatesso agents can tag the right item. The primary prevention is creating issue items withgh:owner/repo#Ntags.Encoded second references such as
[other](https://github.com/other/widgets/issues/31)andissues%2F31bypassed the earlier raw/strict parsers. This changes the ownership contract instead of enabling automatic adoption for arbitrary prose.--adopt-cited, SDKadoptCited: true, reusable sync inputadopt-cited: true. Every default is false. Preview, gated import and repeat verification receive the same setting. Default imports neither adopt nor refuse fallback ambiguity; established structured-identity collision checks remain unchanged.entities@8.1.0; existing pins and thresholds are unchanged.Issue: <url>/Tracks: <url>. Unanchored descriptions, body fix phrases and Related mentions are candidates only.Tag/structured URL/provider matching keeps precedence. With opt-in, one eligible owner receives an
adoptplan and normal upsert; two eligible owners refuse the batch with typedambiguous_citation, candidate IDs and a tagging remedy. Closed/canceled issue owners remain eligible. Dry-run reports adopts, receipts reportadopted, and gated idempotency counts one landing item. Default hints never change the referenced item's fields or status.Issue:item; real workflow shells execute preview, import and repeat for omitted/false/true input/and%2F0x53ce71a9: 4,096 repository/number/decoration/encoding samples across three fields, 12,288 cases; earlier matrices remainAll four mandatory PM-linked commands passed with no failures/skips: adoption 44/44, regex/performance 8/8, executable workflow 4/4, packed CLI 1/1. An additional complete selector/fidelity sweep passed 80/80. Typecheck, builds, lint, docstrings, zero duplication, privacy, strict tracker health and changelog generation/check passed locally. Scoped coverage found no uncovered branches in the new normalization/exclusion/allowlist functions.
Behavioral revert proofs use current tests and real persistence in an isolated source copy:
0b4bc29matchImportedIssuenode --test --test-name-pattern='explicit opt-in (false|defaults)' test/cited-issue-adoption.test.tscitesIssueand its prior raw helpernode --test --test-name-pattern='normalized allowlist' test/cited-issue-adoption.test.tsnode --test --test-name-pattern='reusable workflow.*(explicit opt-in|adoption input)' test/import-gate.test.tsThe workflow proof packs a real copy of current compiled output: true loses adoption, the input contract fails, and the two default executions still pass. Fixed selections pass. Proofs and prior review evidence are recorded on the PM item.
Saved public tracker archives were replayed read-only with live GitHub issue/timeline reads:
f72250e4c5ee8dfafaf57ee8afe20ddd3cbc993e, issue 30pm-review-withdrawn-lineagee1fc83a81507ca6a91f1675b74863c050f1a89f1, issue 307d7479ae77c13d01c54b2b4560ad54091ffed788, issues 154/153/136/53/81All modes report zero skips; pm-context retains three
multiple_issueshints. All 393/393 original tracker files retain identical SHA-256 hashes after both modes. These are saved revisions, not a claim about latest repository mains. Validation details include the adopt entry and exact commands.Exact-head npm/Bun CI release gates and CodeQL are pending. Their completed run links and counts will replace this paragraph before handoff.
Commit
6adb234contains code, reusable workflow, regression tests, README/validation/caller docs, regenerated changelog and PM history together. Commit51dea17removes an unnecessary overlapping trailing match flagged by CodeQL in Markdown definition normalization, with no rule suppression or threshold changes. All linked checks passed again after that cleanup. Commit894dbdbkeeps distinct normalized/source/joined views but scans equal views once, scans marker positions before reading adjacent repositories, and skips only identity formatting transforms. This addresses the unchanged 50 ms native ownership budget (85.562 ms before optimization in CI; local fixed 22.892 ms). Restoring only the three changed performance functions from51dea17makes the selected native probe fail at 112.322 ms, 1 failure / 0 passes / 7 filtered. No threshold was relaxed. Current Node source checks pass 52/52 and native local Bun regex checks 8/8, with no uncovered new-helper branches. The public replay was re-run unchanged after this optimization. Independent re-review remains pending; skipped or unavailable reviewers do not establish approval.PM: pm-github-cited-issue-adoption, left in progress with the claim released. Existing overlap: import fidelity, identity diagnosis/repair. Incident: pm-review-withdrawn-lineage.
Summary by Sourcery
Require explicit opt-in for conservative GitHub issue citation adoption while preserving normal creates and surfacing actionable candidate hints by default.
New Features:
Bug Fixes:
Enhancements:
Build:
CI:
Documentation:
Tests:
Chores: