Skip to content

feat(#848): add file-issue CLI command with dedup guard - #869

Open
fullsend-ai-coder[bot] wants to merge 1 commit into
mainfrom
agent/848-retro-dedup-guard
Open

feat(#848): add file-issue CLI command with dedup guard#869
fullsend-ai-coder[bot] wants to merge 1 commit into
mainfrom
agent/848-retro-dedup-guard

Conversation

@fullsend-ai-coder

Copy link
Copy Markdown

Summary

Adds 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 nearly identical issues within a 2-minute window. This command solves that race by searching for recent issues with similar titles before creating.

Related Issue

Closes #848

Changes

  • Add SearchIssues to the forge.Client interface with IssueSearchOptions (owner, repo, creator, since, state)
  • Implement SearchIssues for GitHub (via the Search API /search/issues), GitLab (stub returning ErrNotSupported), and FakeClient
  • Add fullsend file-issue CLI command with --creator, --dedup-window, --dry-run flags
  • 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 — the command falls through to creation rather than blocking filing

Testing

  • Title similarity tests (identical, near-identical, rephrased, different, case-insensitive, edge cases)
  • normalizeWords tests (stop word removal, punctuation, number preservation)
  • Dedup integration tests: duplicate found, no duplicate, no creator, search failure fallthrough, dry run, distinct proposals pass, different repos never suppress
  • GitHub SearchIssues tests: result parsing, PR exclusion, default state
  • go vet passes
  • go build ./... passes
  • make lint — pre-commit could not run in sandbox (network restricted); the post-script runs it authoritatively

Checklist

  • PR title follows Conventional Commits (correct type, ! for breaking changes)
  • Commits are signed off (DCO) — autonomous agent, exempt
  • Tests added for new logic

Closes #848

Post-script verification

  • Branch is not main/master (agent/848-retro-dedup-guard)
  • Secret scan passed (gitleaks — c887fc47c59d8b87b5282ca44044d2933d30df41..HEAD)
  • PR body secret scan passed (gitleaks — no-git)
  • Pre-commit hooks passed (authoritative run on runner)
  • Tests ran inside sandbox

Add a `fullsend file-issue` command that wraps issue creation
with a built-in dedup guard to prevent concurrent retro agents
from filing duplicate issues. When 23 workspace-update PRs
closed simultaneously on 2026-07-24, 5 retros filed nearly
identical issues within a 2-minute window due to a TOCTOU
race in the existing dedup check.

The command searches for recent issues by the same author in
the target repo using the GitHub Search API. Title similarity
is measured via Jaccard word-overlap coefficient (threshold
0.6) after normalizing to lowercase and filtering stop words.
When a duplicate is found, the command skips creation and adds
a comment on the existing issue with the additional evidence.

Changes:
- Add SearchIssues to forge.Client interface with
  IssueSearchOptions (owner, repo, creator, since, state)
- Implement SearchIssues for GitHub (Search API), GitLab
  (stub returning ErrNotSupported), and FakeClient
- Add file-issue CLI command with --creator, --dedup-window,
  --dry-run flags and JSON output
- Search failures are non-fatal (fall through to creation)
- Comprehensive tests for similarity, normalization, dedup
  integration, and GitHub API interaction

Note: pre-commit could not run in sandbox (network
restricted); the post-script runs it authoritatively.

Closes #848
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 2, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 9:45 AM UTC · Completed 10:02 AM UTC
Commit: 1903a53 · View workflow run →

@fullsend-ai-review

fullsend-ai-review Bot commented Aug 2, 2026

Copy link
Copy Markdown

Review

Findings

Medium

  • [api-contract] internal/forge/github/github.go:2577IssueSearchOptions.State is documented as "open", "closed", or "" for any state but the SearchIssues implementation defaults empty State to "open" (line 2577). A caller passing State="" expecting all issues (per the documented contract) would silently receive only open issues. The current sole caller (fileIssueWithDedup) passes State: "open" explicitly, so no active code path is affected. Fix the doc comment to say empty defaults to "open", or change the implementation to omit the state qualifier when empty.

Low

  • [test-coverage-gap] internal/forge/gitlab/issue.go:361 — GitLab SearchIssues stub returns forge.ErrNotSupported but has no corresponding test in the existing TestErrNotSupported_* test groups that systematically cover analogous stubs.

  • [injection-vuln] internal/forge/github/github.go:2569SearchIssues interpolates opts.Owner, opts.Repo, opts.Creator, and opts.State into the GitHub Search query string via fmt.Sprintf. Since the Search API interprets space-separated tokens as additional qualifiers, values containing spaces could alter search scope. Current call path uses CLI-flag-sourced values (low exploitability). Consider validating inputs contain only expected characters for defense in depth.

  • [content-injection] internal/cli/fileissue.go:200 — The duplicate comment body interpolates the title parameter into Markdown bold (> **%s**). If the title contained Markdown syntax, it would render under the bot's identity. Risk mitigated by CLI flag origin. Consider wrapping in backticks instead of bold.

  • [fail-open] internal/cli/fileissue.go:187 — The dedup guard is fail-open by design: SearchIssues failure falls through to issue creation. Under sustained API errors, concurrent agents would bypass dedup. The trade-off (preserving filing ability over strict dedup) is intentional and tested.

  • [naming-coherence] internal/cli/fileissue.go:200 — Duplicate comment uses agent-specific language ("another agent independently proposed") in a generically named command. Minor naming/abstraction mismatch.

  • [documentation-style] internal/cli/fileissue.go:158 — Function comment for fileIssueWithDedup includes confusing meta-commentary ("exported-name-style (lowercase, unexported)"). Simplify to describe behavior only.

  • [documentation-style] internal/cli/fileissue.go:64 — Const declarations use block doc-comments while similar constants in the codebase (e.g., maxBodyBytes in readbody.go) use inline comments.

  • [type-documentation] internal/cli/fileissue.go:56FileIssueResult type comment is terse compared to similar result types in the package.

  • [stale-reference] docs/superpowers/specs/2026-05-04-retro-agent-design.md:154 — Retro agent design spec references gh issue create for filing issues. Still accurate until the retro post-script integration lands, but should be updated at that point.

Previous run

Review

Findings

Medium

  • [scope-exceeded] internal/cli/fileissue.go — Issue Add post-script dedup guard for concurrent retro proposals #848 authorized a "post-script dedup guard for concurrent retro proposals" with implementation option 1 being a post-script lookback search. This PR implements the dedup logic as a standalone CLI command (fullsend file-issue) rather than modifying the retro post-script directly. The standalone command is architecturally sound and more reusable, but the scope differs from what was authorized — the retro post-script still uses gh issue create and would need a separate follow-up PR to integrate the new command. Consider updating issue Add post-script dedup guard for concurrent retro proposals #848 to document the scope change.

  • [stale-doc] docs/agents/retro.md:14 — The retro agent documentation does not mention fullsend file-issue or dedup protection. Note: the retro post-script has not been updated in this PR to use the new command, so the docs accurately describe the current implementation. This becomes actionable when the post-script integration follows.

  • [stale-doc] docs/superpowers/specs/2026-05-04-retro-agent-design.md:154 — The retro design spec references gh issue create for issue filing. Same caveat as above — the spec is currently accurate since the post-script hasn't been changed yet. Update when integration follows.

Low

  • [test-inadequate] internal/forge/fake.go:1284FakeClient.SearchIssues ignores the Creator and Since fields, so unit tests cannot verify that dedup search is scoped to the correct author and time window. The live implementation does have httptest-based tests (TestSearchIssuesReturnsResults in github_comment_test.go) that verify the query string contains the expected author: and created:>= qualifiers, mitigating this concern.

  • [edge-case] internal/cli/fileissue.go:167titleSimilarity returns 1.0 when both titles normalize to zero words (e.g., both are stop-words-only like "add the" and "add to"). Unlikely in practice since real issue titles contain content words.

  • [edge-case] internal/cli/fileissue.go:120 — TOCTOU window between the dedup search and issue creation. Two concurrent agents could both pass the dedup check and both create issues. Acknowledged as a best-effort design tradeoff per issue Add post-script dedup guard for concurrent retro proposals #848.

  • [injection] internal/forge/github/github.go:2577SearchIssues interpolates IssueSearchOptions fields (Creator, State) into the GitHub Search query string without sanitizing for qualifier injection. A Creator value with spaces could inject additional search qualifiers. Low risk since the current callsite passes CLI flag values and GitHub usernames cannot contain spaces.

@fullsend-ai-review fullsend-ai-review Bot added the requires-manual-review Review requires human judgment label Aug 2, 2026
@guyoron1

guyoron1 commented Aug 2, 2026

Copy link
Copy Markdown
Owner

/fs-review

@fullsend-ai-review

fullsend-ai-review Bot commented Aug 2, 2026

Copy link
Copy Markdown

🤖 Review · ❌ Terminated · Started 10:31 AM UTC · Ended 10:51 AM UTC
Commit: 1903a53 · View workflow run →

@guyoron1

guyoron1 commented Aug 2, 2026

Copy link
Copy Markdown
Owner

/fs-fix

@fullsend-ai-review fullsend-ai-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the review comment for full details.


Note: The following inline comments could not be posted on the diff (GitHub returned 422) and are included here instead:

  • internal/forge/github/github.go:2577: [medium] api-contract

IssueSearchOptions.State is documented as '"open", "closed", or "" for any state' but the SearchIssues implementation defaults empty State to "open". A caller passing State="" expecting all issues would silently receive only open issues. Current sole caller passes State: "open" explicitly, so no active code path is affected.

Suggested fix: Fix the doc comment to say empty defaults to "open", or change the implementation to omit the state qualifier when empty.

  • internal/forge/gitlab/issue.go (file-level): Line 361 · [low] test-coverage-gap

GitLab SearchIssues stub returns forge.ErrNotSupported but has no corresponding test in the existing TestErrNotSupported_* test groups.

  • internal/forge/github/github.go:2569: [low] injection-vuln

SearchIssues interpolates opts.Owner, opts.Repo, opts.Creator, and opts.State into the GitHub Search query string. Spaces in these values would be interpreted as additional qualifiers. Current call path uses CLI-flag-sourced values (low exploitability). Consider validating inputs for defense in depth.

  • internal/cli/fileissue.go:200: [low] content-injection

The duplicate comment body interpolates the title parameter into Markdown bold. If title contained Markdown syntax, it would render under the bot's identity. Risk mitigated by CLI flag origin.

  • internal/cli/fileissue.go:187: [low] fail-open

Dedup guard is fail-open by design: SearchIssues failure falls through to issue creation. Under sustained API errors, concurrent agents would bypass dedup. Trade-off is intentional and tested.

  • internal/cli/fileissue.go:200: [low] naming-coherence

Duplicate comment uses agent-specific language ('another agent independently proposed') in a generically named command.

  • internal/cli/fileissue.go:158: [low] documentation-style

Function comment for fileIssueWithDedup includes confusing meta-commentary ('exported-name-style (lowercase, unexported)'). Simplify to describe behavior only.

  • internal/cli/fileissue.go:64: [low] documentation-style

Const declarations use block doc-comments while similar constants in the codebase use inline comments.

  • internal/cli/fileissue.go:56: [low] type-documentation

FileIssueResult type comment is terse compared to similar result types in the package.

@fullsend-ai-review

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 10:31 AM UTC · Completed 10:51 AM UTC
Commit: 1903a53 · View workflow run →

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown

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 stale label to reset the inactivity timer.

@github-actions github-actions Bot added the stale label Sep 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

requires-manual-review Review requires human judgment stale

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add post-script dedup guard for concurrent retro proposals

1 participant