feat(#849): add file-issue CLI command with dedup guard - #871
feat(#849): add file-issue CLI command with dedup guard#871fullsend-ai-coder[bot] wants to merge 2 commits into
Conversation
Add a `fullsend file-issue` CLI command that wraps issue creation
with a built-in dedup guard, preventing concurrent retro agent
runs from filing duplicate issues. When 23 workspace-update PRs
were closed simultaneously on 2026-07-24, 5 retro agents
independently filed near-identical issues within a 2-minute window.
The command solves the TOCTOU race by searching for recent issues
with similar titles before creating new ones:
- Add `SearchIssues` to `forge.Client` interface with
`IssueSearchOptions` (owner, repo, creator, since, state)
- Implement for GitHub (Search API), GitLab (stub), FakeClient
- Title similarity uses Jaccard word-overlap coefficient with
stop-word filtering (threshold: 0.6)
- JSON output: `{created, url, number, duplicate_of}`
- Search failures are non-fatal (fall through to creation)
- Flags: --creator, --dedup-window, --dry-run
Note: pre-commit could not run in sandbox (network restricted);
the post-script runs it authoritatively on the runner.
Closes #849
|
🤖 Finished Review · ✅ Success · Started 9:48 AM UTC · Completed 10:03 AM UTC |
ReviewFindingsMedium
Low
Next steps:
Previous runReviewFindingsMedium
Low
Next steps:
Previous run (2)ReviewFindingsMedium
Low
Labels: PR adds a new file-issue CLI command implementing a dedup guard feature |
fullsend-ai-review
Bot
added
requires-manual-review
|
/fs-review |
|
🤖 Finished Review · ✅ Success · Started 10:31 AM UTC · Completed 10:49 AM UTC |
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
fullsend-ai-review
Bot
removed
the
requires-manual-review
|
/fs-fix |
|
🤖 Finished Fix · ✅ Success · Started 10:51 AM UTC · Completed 11:00 AM UTC |
- Fix API contract: update IssueSearchOptions.State doc to reflect that empty defaults to "open" (not "any state") - Fix edge case: remove "add" from stop-word list so action verbs contribute to similarity scoring; opposite-action titles (e.g., "Add X" vs "Remove X") are no longer falsely matched as duplicates - Change >= to > for threshold comparison to avoid boundary false positives at exactly 0.6 - Add input validation: reject Creator values containing spaces, colons, or quotes that could inject search qualifiers - Add dedup_skipped field to FileIssueResult JSON output so callers can detect when the dedup guard was not enforced - Add explanatory comment for the 0.6 similarity threshold - Fix self-contradictory comment on fileIssueWithDedup - Add test for opposite-action edge case and dedup_skipped assertions Addresses review feedback on #871
🔧 Fix agent — iteration 1 (human-triggered)Addressed all 6 review findings: fixed API contract doc, removed action verb from stop words and tightened threshold comparison, added Creator input validation, added dedup_skipped output field, documented threshold rationale, and fixed contradictory comment. Fixed (6):
Tests: passedNext steps:
|
|
🤖 Finished Review · ✅ Success · Started 11:03 AM UTC · Completed 11:23 AM UTC |
| } | ||
|
|
||
| // Build the search query string per GitHub Search syntax. | ||
| parts := []string{fmt.Sprintf("repo:%s/%s", opts.Owner, opts.Repo), "is:issue"} |
There was a problem hiding this comment.
[medium] injection
SearchIssues validates opts.Creator with containsSearchSyntax() but does not validate opts.Owner, opts.Repo, or opts.State before interpolating them into the GitHub Search query string. While the current CLI call site constrains these values, the method is on the forge.Client interface and may be called by future code with unsanitized inputs.
Suggested fix: Apply containsSearchSyntax() validation to opts.Owner, opts.Repo, and opts.State. Validate State against an allowlist of 'open'/'closed'.
|
|
||
| since := time.Now().UTC().Add(-dedupWindow) | ||
| existing, err := client.SearchIssues(ctx, forge.IssueSearchOptions{ | ||
| Owner: owner, |
There was a problem hiding this comment.
[low] race-condition
TOCTOU race condition in dedup guard. The check-then-create pattern has no atomicity guarantee. Concurrent agents can all pass the search phase before any creates. This residual race is inherent to distributed dedup without server-side locking. The design trade-off is intentional.
| }) | ||
| if err != nil { | ||
| // Non-fatal: if the search fails (e.g., rate limit, unsupported | ||
| // forge), fall through to creation rather than blocking filing. |
There was a problem hiding this comment.
[low] fail-open
Dedup guard intentionally fails open on search error (falls through to creation). Documented in code and signaled via dedup_skipped JSON field. Intentional design trade-off.
| // stripping punctuation. A coefficient strictly above | ||
| // defaultSimilarityThreshold is considered a match. | ||
| func titlesSimilar(a, b string) bool { | ||
| return titleSimilarity(a, b) > defaultSimilarityThreshold |
There was a problem hiding this comment.
[low] edge-case
titlesSimilar uses strict > (not >=) for the 0.6 threshold, but the comment on defaultSimilarityThreshold says titles sharing >=60% are near-certain rephrasings. Minor doc/code mismatch; the test suite covers this boundary intentionally.
|
|
||
| Output is JSON with fields: created, url, number, duplicate_of.`, | ||
| RunE: func(cmd *cobra.Command, args []string) error { | ||
| printer := ui.New(os.Stderr) |
There was a problem hiding this comment.
[low] output-destination-consistency
Uses ui.New(os.Stderr) correctly (JSON to stdout, status to stderr), but the codebase convention includes a clarifying inline comment when using os.Stderr.
Suggested fix: Add comment: // status to stderr, JSON output to stdout
| // SearchIssues queries for issues matching an IssueSearchOptions filter. | ||
| // Used for dedup checks before filing (e.g., retro proposals). Returns | ||
| // at most one page of results sorted by creation time descending. | ||
| SearchIssues(ctx context.Context, opts IssueSearchOptions) ([]Issue, error) |
There was a problem hiding this comment.
[low] api-contract-completeness
SearchIssues uses an options-struct pattern while all other forge.Client methods pass owner, repo positionally. This creates an API style inconsistency.
Suggested fix: Consider SearchIssues(ctx, owner, repo string, opts IssueSearchOptions) to keep Owner/Repo positional.
| // SearchIssues queries for issues matching an IssueSearchOptions filter. | ||
| // Used for dedup checks before filing (e.g., retro proposals). Returns | ||
| // at most one page of results sorted by creation time descending. | ||
| SearchIssues(ctx context.Context, opts IssueSearchOptions) ([]Issue, error) |
There was a problem hiding this comment.
[low] interface-method-grouping
Blank lines around SearchIssues break the contiguous grouping of issue methods in the interface definition.
Suggested fix: Remove blank lines before and after SearchIssues so it sits contiguously with other issue methods.
| // SearchIssues queries for issues matching an IssueSearchOptions filter. | ||
| // Used for dedup checks before filing (e.g., retro proposals). Returns | ||
| // at most one page of results sorted by creation time descending. | ||
| SearchIssues(ctx context.Context, opts IssueSearchOptions) ([]Issue, error) |
There was a problem hiding this comment.
[low] api-contract-documentation
Doc comment says 'at most one page of results' without specifying page size. The GitHub implementation hard-codes per_page=100. Callers cannot know the max result count from the interface alone.
Suggested fix: Document 'at most 100 results' in the interface doc comment.
|
This pull request has been automatically marked as stale because it has not had any activity in the last month. It will be closed in 2 weeks if no further activity occurs. Remove the |
1 participant
Summary
Adds a
fullsend file-issueCLI command that wraps issue creation with a built-in dedup guard, preventing concurrent retro agent runs from filing duplicate issues. When multiple concurrent retro agents independently identify the same improvement, only one issue is created — the others detect the duplicate via title similarity search and skip filing.Related Issue
Closes #849
Changes
SearchIssuesmethod to theforge.Clientinterface withIssueSearchOptions(owner, repo, creator, since, state)SearchIssuesfor GitHub (via the Search API/search/issues), GitLab (stub returningErrNotSupported), andFakeClientfullsend file-issueCLI command with--repo,--title,--body,--label,--token,--creator,--dedup-window,--dry-runflags{created, url, number, duplicate_of}Testing
normalizeWordstests (stop word removal, punctuation, number preservation)SearchIssuestests: result parsing, PR exclusion, default statego vetpassesgo build ./...passesmake lint— pre-commit could not run in sandbox (network restricted); the post-script runs it authoritativelyChecklist
!for breaking changes)Closes #849
Post-script verification
agent/849-retro-dedup-guard)c887fc47c59d8b87b5282ca44044d2933d30df41..HEAD)