Skip to content

feat(#800): add file-issue CLI command with dedup guard - #822

Open
fullsend-ai-coder[bot] wants to merge 2 commits into
mainfrom
agent/800-retro-dedup-guard
Open

feat(#800): add file-issue CLI command with dedup guard#822
fullsend-ai-coder[bot] wants to merge 2 commits into
mainfrom
agent/800-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.

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 #800

Post-script verification

  • Branch is not main/master (agent/800-retro-dedup-guard)
  • Secret scan passed (gitleaks — ba77dbdbbd17e9bb18c47bf6efa023c98b697158..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` CLI command that creates GitHub issues
with a built-in duplicate detection guard. Before filing, the command
queries the GitHub Search API for issues recently created by the same
author in the target repo. If an existing issue has a similar title
(measured by Jaccard word-overlap coefficient), the command skips
creation and returns the existing issue URL instead.

This prevents the duplicate filing observed when multiple concurrent
retro agent runs identify the same improvement and file nearly
identical issues within a short time window.

Changes:
- Add SearchIssues to forge.Client interface with IssueSearchOptions
- Implement SearchIssues for GitHub (Search API), GitLab (stub), and
  FakeClient
- Add fullsend file-issue CLI command with --creator, --dedup-window,
  --dry-run flags and JSON output
- Add title similarity via Jaccard coefficient with stop-word filtering
- Add comprehensive tests for dedup logic, similarity, and forge method

Note: pre-commit could not run in sandbox (network restricted).

Closes #800
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 2, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 3:13 AM UTC · Completed 3:32 AM UTC
Commit: 6258746 · View workflow run →

@fullsend-ai-review

fullsend-ai-review Bot commented Aug 2, 2026

Copy link
Copy Markdown

Review

Findings

Medium

  • [logic-error] internal/cli/fileissue.go:148--dry-run does not prevent side effects during duplicate detection. When a duplicate is found in Phase 1 (dedup search), the code posts a comment on the existing issue via CreateIssueComment before returning — even when --dry-run is set. The dryRun guard only gates issue creation (Phase 2 at line 167), not the comment posting in Phase 1. This violates the expected read-only semantics of --dry-run.
    Remediation: Wrap the CreateIssueComment call in an if !dryRun { ... } guard.

  • [interface-contract-consistency] internal/forge/forge.go:190IssueSearchOptions.State field comment says "open", "closed", or "" for any state, implying empty string means "match any state." However, the GitHub implementation defaults empty State to "open" (treating "" as "open", not "any"), and the FakeClient ignores State entirely — three different behaviors for the same contract.
    Remediation: Update the State field comment to // "open", "closed", or "" (defaults to "open") to match the GitHub implementation.

Low

  • [missing-test] internal/cli/fileissue_test.go — No test for the interaction between --dry-run and duplicate detection. TestFileIssueWithDedup_DryRun uses an empty FakeClient with no open issues, so it never exercises the code path where a duplicate is found during a dry run. A test with pre-populated issues and dryRun=true would catch the medium-severity bug above.

  • [validation-gap] internal/cli/fileissue.go:27 — Issue Add post-script dedup guard for concurrent retro proposals #800 suggests a 10–15 minute lookback window, but defaultDedupWindow is 30 minutes. The wider window is configurable via --dedup-window and may be a reasonable safety margin, but a code comment explaining the rationale for the 30-minute default would help future readers.

  • [stale-plan-doc] docs/plans/gitlab-cron-polling-implementation.md:176 — The "Full method mapping" table enumerating all forge.Client methods for GitLab does not include the new SearchIssues method (which returns ErrNotSupported). Low priority since plan docs are point-in-time records.

  • [options-struct-precedent] internal/forge/forge.go:183IssueSearchOptions is the first options struct in the forge.Client interface; all other methods use positional parameters. The struct approach is justified by the number of fields (5) and is a reasonable evolution of the API shape.


Next steps:

  • /fs-fix — agent addresses review findings automatically
  • /fs-fix <your instruction> — agent fixes with your specific guidance
  • Push commits directly — review re-runs automatically on push
  • /fs-fix-stop — disable automatic fix runs for this PR
Previous run

Review

Findings

High

  • [missing-requirement] internal/cli/fileissue.go:155 — Issue Add post-script dedup guard for concurrent retro proposals #800 explicitly requires: "If a match is found, skip issue creation and instead add a comment on the existing issue noting the additional evidence." The implementation skips creation and returns the duplicate URL, but does not add a comment to the existing issue. The forge.Client interface already provides CreateIssueComment for this purpose.
    Remediation: Add a client.CreateIssueComment(ctx, owner, repo, issue.Number, commentBody) call when a duplicate is detected (before returning the early FileIssueResult), or update issue Add post-script dedup guard for concurrent retro proposals #800 to clarify that the comment behavior is deferred to a follow-up.

Medium

  • [stdout-corruption] internal/cli/fileissue.go:43 — The ui.Printer and the JSON result both write to os.Stdout. The printer emits human-readable progress lines (Header, StepStart, StepDone, StepInfo, StepWarn) throughout execution, then json.NewEncoder(os.Stdout).Encode(result) writes the JSON. Any consumer parsing stdout for the JSON output documented in Long ("Output is JSON with fields: created, url, number, duplicate_of") will receive interleaved printer text before the JSON.
    Remediation: Write printer output to os.Stderr (ui.New(os.Stderr)), keeping os.Stdout exclusively for the JSON result.

  • [fail-open] internal/cli/fileissue.go — When SearchIssues returns an error, the error is logged as a warning and execution falls through to issue creation, bypassing the dedup guard entirely. While the PR description documents this as intentional ("Search failures are non-fatal"), the primary purpose of this command is duplicate prevention — silently skipping the duplicate check undermines user expectations when the search fails due to rate limiting, network errors, or API issues.
    Remediation: Consider providing a --strict flag that treats search failures as fatal, or at minimum document the fail-open behavior in the --help output.

  • [scope-creep] internal/cli/fileissue.go — Issue Add post-script dedup guard for concurrent retro proposals #800 asks for "a post-filing dedup guard in the retro post-script." The PR delivers a general-purpose fullsend file-issue CLI command that goes beyond the targeted post-script fix into new general-purpose infrastructure. While this is arguably better engineering (testable, reusable), it expands scope beyond the issue's explicit authorization.

  • [pattern-violation] internal/cli/fileissue.go — Direct gh.New(token) call bypasses the newGitHubLiveClient() helper pattern, which handles GitHub Enterprise Server base URL configuration via GITHUB_API_URL environment variable. While gh.New(token) is used in many existing CLI files, newGitHubLiveClient exists specifically for GHES compatibility.
    Remediation: Replace client := gh.New(token) with client := newGitHubLiveClient(token, "").

  • [stale-doc] docs/cli/README.md — The new file-issue CLI command is not listed in the CLI overview's "Additional commands" table, which currently documents run, lock, and scan.
    Remediation: Add file-issue to the Additional commands table.

  • [stale-doc] docs/guides/dev/cli-internals.md — The CLI command tree does not include the new file-issue command.
    Remediation: Add file-issue with its flags to the command tree.

Low

  • [missing-pagination] internal/forge/github/github.goSearchIssues fetches only a single page (per_page=100) without pagination, unlike other listing methods in the same file.
  • [injection-vuln] internal/forge/github/github.go — Search query parameters (Owner, Repo, Creator, State) are interpolated into the GitHub search query string without validation. Values containing spaces could inject additional search qualifiers, though the impact is limited to search result manipulation (read-only).
  • [input-validation] internal/forge/forge.goIssueSearchOptions.State is a free-form string with no enum constraint. Consider constraining to known values ("open", "closed").
  • [scope-alignment] internal/cli/fileissue.go — Issue Add post-script dedup guard for concurrent retro proposals #800 proposes a 10–15 minute lookback window; the implementation defaults to 30 minutes. The --dedup-window flag makes this configurable.
  • [naming-convention] internal/cli/fileissue.goTitlesSimilar is exported but appears to be used only within the cli package. Consider making it unexported (titlesSimilar).
  • [naming-convention] internal/forge/forge.goSearchIssues introduces the Search verb, which is not used elsewhere in the Client interface (List, Get, Create, etc.). Semantically appropriate for the GitHub Search API but differs from established naming.
  • [test-inadequate] internal/forge/fake.goFakeClient.SearchIssues ignores Creator and Since filter parameters. This matches the existing fake pattern in the codebase but reduces test fidelity.
  • [code-organization] internal/forge/github/github_comment_test.goSearchIssues tests are placed in github_comment_test.go, which is for comment operations. Consider a separate test file for issue search operations.
  • [missing-doc] docs/contributing/forge-abstraction.md — The new SearchIssues method is not mentioned in the forge abstraction guide.
  • [missing-doc] skills/filing-issues/SKILL.md — The filing-issues skill doesn't cross-reference the new file-issue CLI command.

Labels: PR adds a new CLI command (file-issue) implementing a feature from issue #800


Next steps:

  • /fs-fix — agent addresses review findings automatically
  • /fs-fix <your instruction> — agent fixes with your specific guidance
  • Push commits directly — review re-runs automatically on push
  • /fs-fix-stop — disable automatic fix runs for this PR

fullsend-ai-review[bot]

This comment was marked as outdated.

@guyoron1

guyoron1 commented Aug 2, 2026

Copy link
Copy Markdown
Owner

/fs-fix

@fullsend-ai-coder

fullsend-ai-coder Bot commented Aug 2, 2026

Copy link
Copy Markdown
Author

🤖 Finished Fix · ✅ Success · Started 6:48 AM UTC · Completed 6:58 AM UTC
Commit: 6258746 · View workflow run →

- Add CreateIssueComment call when duplicate is detected, fulfilling
  the issue #800 requirement to note additional evidence on existing
  issues
- Switch ui.Printer to os.Stderr so JSON output on stdout is not
  corrupted by interleaved progress lines
- Replace gh.New(token) with newGitHubLiveClient(token, "") for
  GitHub Enterprise Server compatibility
- Make TitlesSimilar unexported (titlesSimilar) since it is only
  used within the cli package
- Add file-issue to CLI docs (README.md and cli-internals.md)
- Add test for comment-failure-is-non-fatal behavior

Addresses review feedback on #822
@fullsend-ai-coder

Copy link
Copy Markdown
Author

🔧 Fix agent — iteration 1 (human-triggered)

Addressed 6 of 17 review findings with code and documentation fixes. The high-severity missing-requirement (no comment on duplicate) is resolved by adding a CreateIssueComment call. The stdout-corruption bug is fixed by routing printer output to stderr. The pattern-violation is fixed by using newGitHubLiveClient. Two stale-doc findings are resolved by adding file-issue to CLI docs. The naming-convention finding is resolved by making TitlesSimilar unexported. Disagreed with 11 findings (fail-open is intentional, scope-creep is a design decision, and the remaining are low-severity items suitable for follow-up).

Fixed (6):

  1. missing-requirement: no comment added to existing issue on duplicate detection (internal/cli/fileissue.go): Added CreateIssueComment call when a duplicate is detected, fulfilling the issue Add post-script dedup guard for concurrent retro proposals #800 requirement to add a comment on the existing issue noting the additional evidence. Comment failure is non-fatal to avoid blocking the dedup result. Added test for comment verification and comment-failure-is-non-fatal behavior.
  2. stdout-corruption: printer and JSON both write to os.Stdout (internal/cli/fileissue.go): Changed ui.New(os.Stdout) to ui.New(os.Stderr) so JSON output on stdout is not corrupted by interleaved printer progress lines.
  3. pattern-violation: direct gh.New(token) bypasses newGitHubLiveClient helper (internal/cli/fileissue.go): Replaced gh.New(token) with newGitHubLiveClient(token, "") for GitHub Enterprise Server (GHES) compatibility via GITHUB_API_URL. Removed unused gh import alias.
  4. stale-doc: file-issue not listed in CLI README (docs/cli/README.md): Added file-issue to the Additional commands table in the CLI overview.
  5. stale-doc: file-issue not listed in CLI internals command tree (docs/guides/dev/cli-internals.md): Added file-issue with all its flags to the CLI command tree.
  6. naming-convention: TitlesSimilar is exported but only used within the cli package (internal/cli/fileissue.go): Made TitlesSimilar unexported (titlesSimilar) since it is only used within the cli package. Updated all references in tests.

Disagreed (11):

  1. fail-open: search failure silently falls through to issue creation: The fail-open behavior is explicitly documented in the PR description as intentional design ('Search failures are non-fatal'). The primary failure mode this guards against (rate limiting, unsupported forge) is better served by proceeding with creation than by blocking issue filing entirely. Adding a --strict flag would expand scope beyond addressing a review finding.
  2. scope-creep: general-purpose CLI command vs targeted post-script fix: This is a design decision made by the PR author, not a code defect. A reusable CLI command is arguably better engineering (testable via forge.Client interface, usable beyond retro). Changing the architecture would be a major scope change for the fix agent.
  3. missing-pagination: SearchIssues fetches only a single page: The function docstring explicitly documents 'Returns at most one page of results sorted by creation time descending.' For the dedup use case (recent issues by a specific creator), 100 results is more than sufficient.
  4. injection-vuln: search query parameters interpolated without validation: The impact is limited to search result manipulation (read-only). Values come from CLI flags, not untrusted user input. Adding validation is a reasonable follow-up but not a correctness issue for this PR.
  5. input-validation: IssueSearchOptions.State is a free-form string: This matches the existing pattern in the forge interface where string fields are not enum-constrained. Adding an enum constraint would be a cross-cutting change. Reasonable follow-up.
  6. scope-alignment: default dedup window is 30 minutes vs issue Add post-script dedup guard for concurrent retro proposals #800 proposal of 10-15 minutes: The --dedup-window flag makes this configurable. A wider default is more conservative (catches more potential duplicates). The issue proposed 10-15 minutes as a suggestion, not a hard requirement.
  7. naming-convention: SearchIssues introduces the Search verb not used elsewhere in Client interface: SearchIssues is semantically correct for the GitHub Search API (/search/issues), which is fundamentally different from the list/get operations. Using 'List' would be misleading.
  8. test-inadequate: FakeClient.SearchIssues ignores Creator and Since filter parameters: This matches the existing FakeClient pattern throughout the codebase. The FakeClient is intentionally simple. Adding filter fidelity would be a cross-cutting change.
  9. code-organization: SearchIssues tests in github_comment_test.go: Moving tests between files is a low-value churn change. The tests are in the correct package and run correctly.
  10. missing-doc: forge-abstraction.md does not mention SearchIssues: The forge abstraction guide describes architectural principles and rules, not individual methods. Adding SearchIssues would be inconsistent with the document's scope.
  11. missing-doc: filing-issues skill does not cross-reference file-issue CLI command: The skills/ directory is a protected path. The fix agent cannot modify files under skills/.

Tests: passed

Next steps:

  • /fs-review — request a re-review of the changes
  • /fs-fix <your instruction> — run another fix pass with specific guidance
  • Push commits directly — review re-runs automatically on push
    Updated by fullsend fix agent

@fullsend-ai-review

fullsend-ai-review Bot commented Aug 2, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 7:00 AM UTC · Completed 7:19 AM UTC
Commit: c862d15 · View workflow run →

@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.

Comment thread internal/cli/fileissue.go
})
if err != nil {
// Non-fatal: if the search fails (e.g., rate limit, unsupported
// forge), fall through to creation rather than blocking filing.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[medium] logic-error

--dry-run does not prevent side effects during duplicate detection. When a duplicate is found, the code posts a comment on the existing issue via CreateIssueComment before returning — even when --dry-run is set. The dryRun guard only gates issue creation (Phase 2), not the comment posting in Phase 1.

Suggested fix: Wrap the CreateIssueComment call in an if !dryRun { ... } guard.

Comment thread internal/forge/forge.go
Repo string // repository name
Creator string // filter by author login (empty = any)
Since time.Time // only issues created on or after this time
State string // "open", "closed", or "" for any state

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[medium] interface-contract-consistency

IssueSearchOptions.State field comment says open/closed/empty-for-any-state, but the GitHub implementation defaults empty State to open, and the FakeClient ignores State entirely — three different behaviors for the same contract.

Suggested fix: Update the State field comment to match the GitHub implementation: empty defaults to open.

@@ -0,0 +1,342 @@
package cli

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[low] missing-test

No test for the interaction between --dry-run and duplicate detection. TestFileIssueWithDedup_DryRun uses an empty FakeClient with no open issues, so it never exercises the code path where a duplicate is found during a dry run.

Comment thread internal/cli/fileissue.go
}

// defaultDedupWindow is the lookback duration for dedup searches.
const defaultDedupWindow = 30 * time.Minute

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[low] validation-gap

Issue #800 suggests a 10-15 minute lookback window, but defaultDedupWindow is 30 minutes. The wider window is configurable via --dedup-window, but a code comment explaining the rationale for 30 minutes would help.

Comment thread internal/forge/forge.go
Labels []string
}

// IssueSearchOptions configures a search for issues. Used by

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[low] options-struct-precedent

IssueSearchOptions is the first options struct in the forge.Client interface; all other methods use positional parameters. The struct approach is justified by the number of fields.

@github-actions

github-actions Bot commented Sep 4, 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 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add post-script dedup guard for concurrent retro proposals

1 participant