Skip to content

feat(#5991): add fullsend issues get/post-comment commands - #6144

Merged
ralphbean merged 6 commits into
mainfrom
agent/5991-issues-get-post-comment
Aug 14, 2026
Merged

feat(#5991): add fullsend issues get/post-comment commands#6144
ralphbean merged 6 commits into
mainfrom
agent/5991-issues-get-post-comment

Conversation

@fullsend-ai-coder

Copy link
Copy Markdown
Contributor

Summary

Add fullsend issues get and fullsend issues post-comment CLI commands that read and write issue content across GitHub, GitLab, and Jira via tracker.Client (#5988). The --tracker flag selects the backend, replacing direct gh/glab calls in agent scripts with a single forge-agnostic command path.

The existing top-level fullsend post-comment is preserved as a deprecated backward-compatible alias for GitHub, so current callers (e.g. fullsend-ai/agents triage scripts) continue to work.

Related Issue

Depends on #5988 (tracker.Client interface) and #5989 (Jira tracker.Client implementation).

Changes

  • internal/cli/issues.go: New fullsend issues command group with get (JSON output of issue + comments) and post-comment (sticky comment lifecycle via tracker.Client) subcommands
  • internal/cli/tracker_client.go: Factory function that constructs the appropriate tracker.Client for each backend (GitHub/GitLab via forge adapter, Jira via JiraClient adapter)
  • internal/cli/postcomment.go: Added Deprecated field to cobra.Command so the old fullsend post-comment prints a migration notice
  • internal/cli/root.go: Register newIssuesCmd() in the root command
  • docs/guides/dev/cli-internals.md: Document the new issues command tree and mark post-comment as deprecated
  • Tests: 27 new tests covering flag registration, marker search, sticky comment create/update/dry-run, and tracker client construction for all three backends

Testing

  • All 27 new tests pass (go test -race ./internal/cli/ -run "TestNewIssues|TestFindMarked|TestPostTracker|TestNewTracker|TestResolveJira")
  • Existing post-comment and root command tests still pass
  • go vet clean
  • go build ./cmd/fullsend/ compiles successfully
  • Secret scan clean

Closes #5991

Post-script verification

  • Branch is not main/master (agent/5991-issues-get-post-comment)
  • Secret scan passed (gitleaks — 0257b76bd8546cdd64b665ec996bf16806158516..HEAD)
  • PR body secret scan passed (gitleaks — no-git)

Add a new `fullsend issues` command group with `get` and
`post-comment` subcommands that read and write issue content
across GitHub, GitLab, and Jira via `tracker.Client`.

The `--tracker` flag selects the backend (github, gitlab, jira).
For GitHub/GitLab, `--project` is "owner/repo". For Jira,
`--project` is the Jira project key (e.g. "PROJ").

`fullsend issues get` outputs issue title, body, labels, and
comments as JSON. `fullsend issues post-comment` implements
sticky comment behavior (find-by-marker, collapse history,
create-or-update) using tracker.Client directly.

The existing top-level `fullsend post-comment` command is
preserved as a deprecated backward-compatible alias that always
targets GitHub. It prints a deprecation notice pointing callers
to `fullsend issues post-comment --tracker github`.

A tracker client factory (newTrackerClient) constructs the
appropriate tracker.Client for each backend: ForgeClient
adapter for GitHub/GitLab, JiraClient adapter for Jira.

Note: pre-commit could not run in the sandbox (network error
during hook initialization). The post-script runs it
authoritatively.

Closes #5991
@fullsend-ai-coder
fullsend-ai-coder Bot requested a review from a team as a code owner August 12, 2026 15:23
@fullsend-ai-coder fullsend-ai-coder Bot added the ready-for-review Triggers review agent dispatch label Aug 12, 2026
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 12, 2026

Copy link
Copy Markdown

🤖 Review · ⚠️ Cancelled · Started 3:24 PM UTC · Ended 3:24 PM UTC

Commit: 1f4ccce · View workflow run →

@github-actions

github-actions Bot commented Aug 12, 2026

Copy link
Copy Markdown

Site preview

Preview: https://ec4e6475-site.fullsend-ai.workers.dev

Commit: 86356ca39634ae0f464d15602724f23b99bc9899

@fullsend-ai-review

fullsend-ai-review Bot commented Aug 12, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 3:25 PM UTC · Completed 3:41 PM UTC

Commit: 1f4ccce · View workflow run →

@codecov

codecov Bot commented Aug 12, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 87.93651% with 38 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
internal/cli/issues.go 90.09% 13 Missing and 8 partials ⚠️
internal/tracker/fake_jira.go 71.42% 7 Missing and 3 partials ⚠️
internal/cli/tracker_client.go 88.67% 3 Missing and 3 partials ⚠️
internal/config/defaults.go 0.00% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

@fullsend-ai-review

fullsend-ai-review Bot commented Aug 12, 2026

Copy link
Copy Markdown

Review

Findings

Low

  • [config validation gap] internal/config/config.go:521 — The perRepoConfig.Tracker field is not validated during Validate(). This is consistent with the Forge field (which also skips Validate() checks), and resolveTracker validates the value at CLI invocation time via validateTrackerName. A config.yaml containing tracker: servicenow will parse without error, only failing when the CLI command is run.
    Remediation: Consider adding tracker validation to perRepoConfig.Validate() for parity with Runtime/Inference.Provider validation.

  • [scope divergence from issue authorization] internal/cli/issues.go:63 — Issue Add fullsend issues get/fullsend issues post-comment, deprecate top-level fullsend post-comment #5991 specifies --forge but the PR implements --tracker. The implementation provides documented architectural justification (forge ≠ tracker: a GitHub-hosted repo may track issues in Jira). The code, docs, and tests are internally consistent with the --tracker naming; only the issue description needs updating.
    Remediation: Update issue Add fullsend issues get/fullsend issues post-comment, deprecate top-level fullsend post-comment #5991 to document the design decision to use --tracker instead of --forge.

  • [marker spoofing / missing author verification] internal/cli/issues.go:469findMarkedTrackerComment does not filter by comment author, unlike sticky.FindMarkedComment which accepts a botUser parameter. The godoc on postTrackerStickyComment and the user-facing documentation (docs/guides/user/issues-commands.md, Trust model section) explicitly document this as a conscious tradeoff for trusted CI environments where tracker.Client lacks GetAuthenticatedUser.

  • [secrets exposure via CLI flag] internal/cli/issues.go:140 — The --token flag accepts API tokens as command-line arguments visible via /proc/<pid>/cmdline. Environment variable fallback exists for all trackers. Consistent with the existing post-comment command pattern.

  • [test fidelity] internal/tracker/jira_client_test.go:222 — The refactoring from unexported fakeJiraClient to exported FakeJiraClient changed UpdateComment behavior: the new fake performs the full jira.MarkdownToADF round-trip (more realistic), adding an implicit dependency on MarkdownToADF succeeding for test inputs.

  • [edge case] internal/cli/issues.go:272testBody == "" cannot distinguish "no override" from "inject empty body." The empty-body path is covered at the postTrackerStickyComment level (TestPostTrackerStickyComment_EmptyBody).

  • [error handling] internal/cli/issues.go:381 — When LoadConfig returns a non-PerRepoConfigReader (e.g., org-mode config), resolveTracker falls through to the generic --tracker is required error rather than a more specific message.

  • [test gap] internal/cli/issues_test.go — No test exercises runIssuesGet with a config-based tracker default (the production path). The post-comment equivalent (TestRunIssuesPostComment_TrackerFromConfig) exists, and both share the same resolveTracker function which is well-tested directly.

Previous run

Review

Findings

Low

  • [config validation gap] internal/config/config.go:521 — The perRepoConfig.Tracker field is not validated during Validate(). This is consistent with the Forge field (which also skips Validate() checks), and resolveTracker validates the value at CLI invocation time via validateTrackerName. A config.yaml containing tracker: servicenow will parse without error, only failing when the CLI command is run.
    Remediation: Consider adding tracker validation to perRepoConfig.Validate() for parity with Runtime/Inference.Provider validation.

  • [scope divergence from issue authorization] internal/cli/issues.go:100 — Issue Add fullsend issues get/fullsend issues post-comment, deprecate top-level fullsend post-comment #5991 specifies --forge but the PR implements --tracker. The implementation provides documented architectural justification (forge ≠ tracker: a GitHub-hosted repo may track issues in Jira). The code, docs, and tests are internally consistent with the --tracker naming; only the issue description needs updating.
    Remediation: Update issue Add fullsend issues get/fullsend issues post-comment, deprecate top-level fullsend post-comment #5991 to document the design decision to use --tracker instead of --forge.

  • [marker spoofing / missing author verification] internal/cli/issues.go:432findMarkedTrackerComment does not filter by comment author, unlike sticky.FindMarkedComment which accepts a botUser parameter. The godoc on postTrackerStickyComment and the user-facing documentation (docs/guides/user/issues-commands.md, Trust model section) explicitly document this as a conscious tradeoff for trusted CI environments where tracker.Client lacks GetAuthenticatedUser.

  • [secrets exposure via CLI flag] internal/cli/issues.go:103 — The --token flag accepts API tokens as command-line arguments visible via /proc/<pid>/cmdline. Environment variable fallback exists for all trackers. Consistent with the existing post-comment command pattern.

  • [test fidelity] internal/tracker/jira_client_test.go:222 — The refactoring from unexported fakeJiraClient to exported FakeJiraClient changed UpdateComment behavior: the new fake performs the full jira.MarkdownToADF round-trip (more realistic), adding an implicit dependency on MarkdownToADF succeeding for test inputs.

  • [edge case] internal/cli/issues.go:272testBody == "" cannot distinguish "no override" from "inject empty body." The empty-body path is covered at the postTrackerStickyComment level (TestPostTrackerStickyComment_EmptyBody).

  • [error handling] internal/cli/issues.go:381 — When LoadConfig returns a non-PerRepoConfigReader (e.g., org-mode config), resolveTracker falls through to the generic --tracker is required error rather than a more specific message.

  • [test gap] internal/cli/issues_test.go — No test exercises runIssuesGet with a config-based tracker default (the production path). The post-comment equivalent (TestRunIssuesPostComment_TrackerFromConfig) exists, and both share the same resolveTracker function which is well-tested directly.

Previous run (2)

Review

Findings

Medium

  • [input validation gap] internal/cli/issues.go:360resolveTracker returns the tracker name as-is without normalizing case or validating against the recognized set (github, gitlab, jira). In runIssuesPostComment, the un-normalized value gates Jira-specific protections (marker validation at line 310, MaxSize cap at line 338) via trackerName == TrackerJira. A case-mismatched input like --tracker JIRA skips both protections and then fails at newTrackerClient with a confusing "unsupported tracker" error instead of a clear validation message.
    Remediation: Normalize the tracker name to lowercase in resolveTracker (e.g., return strings.ToLower(trackerFlag), nil) or validate against the known set early.

  • [scope divergence from issue authorization] internal/cli/issues.go:94 — Issue Add fullsend issues get/fullsend issues post-comment, deprecate top-level fullsend post-comment #5991 specifies the flag name as --forge (matching the existing convention used by fullsend run, fullsend repos install, fullsend lock, fullsend reconcile-status, and fullsend poll), but the PR implements it as --tracker. The issue body says: "fullsend issues get --forge {github,gitlab,jira} ..." and "--forge is required unless a default is supplied via config." While tracker.Client is the internal Go interface name, CLI flags are user-facing and diverging from the established --forge convention creates an inconsistency for CLI users.
    Remediation: Rename --tracker to --forge on both subcommands to match the issue specification and existing CLI convention, or update the issue to record a deliberate decision to diverge.

Low

  • [marker spoofing / missing author verification] internal/cli/issues.go:404findMarkedTrackerComment does not filter by comment author, unlike sticky.FindMarkedComment which accepts a botUser parameter. An untrusted user could plant a comment containing the marker string, causing the bot to edit that user's comment instead of creating its own. The godoc on postTrackerStickyComment explicitly documents this as a conscious tradeoff because tracker.Client lacks a GetAuthenticatedUser method, and usage is scoped to trusted CI environments.

  • [secrets exposure via CLI flag] internal/cli/issues.go:156 — The --token flag accepts API tokens as command-line arguments visible via /proc/<pid>/cmdline. In CI environments (ephemeral, single-tenant runners) this is typically not exploitable, and environment variable fallback exists. Consistent with the existing post-comment command pattern.

  • [naming-convention] internal/cli/tracker_client.go:14 — Constants TrackerGitHub, TrackerGitLab, TrackerJira are exported but live in an internal/ package consumed only locally. Every other string constant in the cli package is unexported (maxContextScanDepth, metricsFile, defaultAgentsRepoOwner, etc.).
    Remediation: Rename to trackerGitHub, trackerGitLab, trackerJira.

  • [naming-convention] internal/cli/tracker_client.go:97resolveGitLabTrackerToken uses an inconsistent naming pattern compared to the symmetric resolveGitHubToken and resolveJiraToken. The Tracker infix only appears in the GitLab variant (justified by collision avoidance with existing resolveGitLabToken() in forge_client.go).

  • [missing-config-field-doc] docs/guides/infrastructure/layered-config-reference.md:71 — The config field reference table does not include the new tracker field added to perRepoConfig. This field provides a default tracker backend for fullsend issues commands.
    Remediation: Add a row to the table for the tracker field.

  • [missing-cli-command-doc] docs/guides/dev/cli-internals.md:9 — The developer-facing ASCII tree documents the issues command with subcommands, but there is no user-facing guide explaining when and how to use fullsend issues get and fullsend issues post-comment for custom agents posting to Jira or GitLab.
    Remediation: Consider adding a user-facing guide entry in docs/guides/user/.

Previous run (3)

Review

Findings

Medium

  • [scope divergence from issue authorization] internal/cli/issues.go:145 — Issue Add fullsend issues get/fullsend issues post-comment, deprecate top-level fullsend post-comment #5991 specifies the flag name as --forge (matching the existing convention used by fullsend run, fullsend repos install, fullsend lock, fullsend reconcile-status, and fullsend poll), but the PR implements it as --tracker. The issue body says: "fullsend issues get --forge {github,gitlab,jira} ..." and "--forge is required unless a default is supplied via config." While tracker.Client is the internal Go interface name, CLI flags are user-facing and diverging from the established --forge convention creates an inconsistency for CLI users.
    Remediation: Rename --tracker to --forge on both subcommands to match the issue specification and existing CLI convention.

  • [marker spoofing / missing author verification] internal/cli/issues.go:376findMarkedTrackerComment does not filter by comment author, unlike sticky.FindMarkedComment which accepts a botUser parameter. An untrusted user could plant a comment containing the marker string, causing the bot to edit that user's comment instead of creating its own. The PR's godoc documents this as a conscious tradeoff for trusted CI environments where tracker.Client lacks GetAuthenticatedUser.
    Remediation: Add GetAuthenticatedUser to tracker.Client, or document the trust requirement in CLI help text (not just godoc).

Low

  • [secrets exposure via CLI flag] internal/cli/issues.go:148 — The --token flag accepts API tokens as command-line arguments visible via /proc/<pid>/cmdline. In CI environments (ephemeral, single-tenant runners) this is typically not exploitable, and environment variable fallback exists. Consistent with the existing post-comment command pattern.

  • [naming-convention] internal/cli/tracker_client.go:14 — Constants TrackerGitHub, TrackerGitLab, TrackerJira are exported but live in an internal/ package consumed only locally. Every other string constant in the cli package is unexported (maxContextScanDepth, metricsFile, defaultAgentsRepoOwner, etc.).
    Remediation: Rename to trackerGitHub, trackerGitLab, trackerJira.

  • [naming-convention] internal/cli/tracker_client.go:97resolveGitLabTrackerToken uses an inconsistent naming pattern compared to the symmetric resolveGitHubToken and resolveJiraToken. The Tracker infix only appears in the GitLab variant (justified by collision avoidance with existing resolveGitLabToken() in forge_client.go).

  • [missing-doc-entry] docs/guides/dev/cli-internals.md:658 — The Key Source Files Reference table does not include entries for internal/cli/issues.go (322 lines) or internal/cli/tracker_client.go (118 lines), though it lists smaller files like root.go.
    Remediation: Add rows for the new files to the table.

Previous run (4)

Review

Findings

Medium

  • [stale-doc] docs/guides/user/jira-integration.md:39 — The "Event semantics — input only" section states "No output is posted to Jira" and references Add Jira comment write support and implement tracker.Client for Jira #5989 as unimplemented future work describing "Jira comment write support and a tracker.Client for Jira." This PR adds fullsend issues post-comment --tracker jira and tracker.NewJiraClient, providing exactly the CLI primitive and tracker client described as not yet existing.
    Remediation: Update the section to acknowledge the new CLI primitive and update the Add Jira comment write support and implement tracker.Client for Jira #5989 reference to note that tracker.Client Jira support has landed, while agent-pipeline integration remains pending.

Low

  • [missing input validation] internal/cli/issues.go:127runIssuesGet does not validate that --number is a positive integer. A user can pass --number 0 or --number -1, producing confusing tracker API errors. By contrast, runIssuesPostComment at line 239 explicitly checks number <= 0.
    Remediation: Add if cfg.number <= 0 validation guard at the start of runIssuesGet, matching the pattern in runIssuesPostComment.

  • [JSON output inconsistency] internal/cli/issues.go:152 — When an issue has no labels, issue.Labels is nil, causing JSON output to serialize "labels": null instead of "labels": []. The Comments field is explicitly initialized with make(), producing "comments": [] even when empty. This inconsistency can confuse downstream JSON consumers.
    Remediation: Initialize Labels to an empty slice when nil before assigning to the result struct.

  • [marker spoofing / missing author verification] internal/cli/issues.go:337findMarkedTrackerComment does not filter by comment author, unlike sticky.FindMarkedComment which accepts a botUser parameter. The PR's godoc documents this as a conscious tradeoff for trusted CI environments where tracker.Client lacks GetAuthenticatedUser.
    Remediation: Add GetAuthenticatedUser to tracker.Client, or pass an author filter using tracker.Comment.Author against a known bot username.

Previous run (5)

Review

Findings

Medium

  • [security-adjacent logic gap / marker spoofing] internal/cli/issues.go:256findMarkedTrackerComment does not filter by comment author, unlike the existing sticky.FindMarkedComment which accepts a botUser parameter and skips comments not authored by the bot. An untrusted user could plant a comment containing the marker string, causing the bot to edit that user's comment instead of creating its own. The PR's godoc comment on postTrackerStickyComment (lines 186–191) explicitly documents this as a conscious tradeoff — the tracker.Client interface lacks GetAuthenticatedUser, and the command targets trusted CI environments. Worth tracking if the command is later exposed to untrusted callers.
    Remediation: Add GetAuthenticatedUser to tracker.Client, or pass an author filter using tracker.Comment.Author against a known bot username.

  • [stale-doc] docs/guides/user/jira-integration.md:34 — The "Event semantics — input only" section states "No output is posted to Jira" and references Add Jira comment write support and implement tracker.Client for Jira #5989 as unimplemented future work describing "Jira comment write support and a tracker.Client for Jira." This PR adds fullsend issues post-comment --tracker jira and tracker.NewJiraClient, providing exactly the CLI primitive and tracker client described as not yet existing.
    Remediation: Update the section to acknowledge the new CLI primitive and update the Add Jira comment write support and implement tracker.Client for Jira #5989 reference to note that tracker.Client Jira support has landed, while agent-pipeline integration remains pending.

Low

  • [missing input validation] internal/cli/issues.go:78newIssuesGetCmd does not validate that --number is a positive integer. A user can pass --number 0 or --number -1, producing confusing tracker API errors. By contrast, newIssuesPostCommentCmd at line 141 explicitly checks number <= 0.
    Remediation: Add if number <= 0 validation guard in newIssuesGetCmd's RunE.

  • [null vs empty JSON output] internal/cli/issues.go:93 — When an issue has no labels, issue.Labels may be nil, causing JSON output to contain "labels": null instead of "labels": []. The Comments field avoids this by using make() at line 97.
    Remediation: Initialize Labels to empty slice when nil: if issue.Labels == nil { result.Labels = []string{} }.

  • [naming-convention] internal/cli/tracker_client.go:14 — Constants TrackerGitHub, TrackerGitLab, TrackerJira are exported but live in an internal/ package where they are only consumed locally. Other string constants in the package are unexported.
    Remediation: Make constants unexported: trackerGitHub, trackerGitLab, trackerJira.

  • [naming-convention] internal/cli/tracker_client.go:97resolveGitLabTrackerToken uses an inconsistent naming pattern compared to the symmetric resolveGitHubToken and resolveJiraToken. The "Tracker" infix only appears in the GitLab variant (justified by collision avoidance with existing resolveGitLabToken() in forge_client.go).


Labels: PR adds new CLI commands under internal/cli/ (Go) and updates CLI documentation

@fullsend-ai-review fullsend-ai-review Bot added requires-manual-review Review requires human judgment go Pull requests that update go code feature Feature-category issue awaiting human prioritization labels Aug 12, 2026
@ralphbean

Copy link
Copy Markdown
Member

/fs-fix address the findings and make sure CI is fixed and that we pass codecov

@fullsend-ai-coder

fullsend-ai-coder Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor Author

🤖 Finished Fix · ✅ Success · Started 3:54 PM UTC · Completed 4:16 PM UTC

Commit: 1f4ccce · View workflow run →

Refactor RunE logic into standalone runIssuesGet and runIssuesPostComment
functions using config structs with testClient overrides, following the
established pattern (repos.go, admin.go). Add 9 new tests covering:
- runIssuesGet: happy path, no-comments, issue-not-found
- runIssuesPostComment: create, update, invalid-number, empty-marker,
  dry-run, file-based body

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

Copy link
Copy Markdown
Contributor Author

🔧 Fix agent — iteration 1 (human-triggered)

Addressed codecov/patch failure by refactoring issues.go RunE functions into testable standalone functions with config structs (matching repo conventions), and added 9 new tests bringing function coverage from 42-52% to 92-94%. The behaviour test failure is a flaky test also failing on main, unrelated to this PR.

Fixed (2):

  1. codecov/patch coverage below threshold (internal/cli/issues.go): Refactored RunE functions into standalone runIssuesGet and runIssuesPostComment functions using config structs with testClient/testWriter/testPrinter overrides, following the repo's established pattern (repos.go, admin.go). This makes the command execution paths testable without requiring real API tokens.
  2. insufficient test coverage for issues get/post-comment (internal/cli/issues_test.go): Added 9 new tests: TestRunIssuesGet (happy path with JSON output verification), TestRunIssuesGet_NoComments, TestRunIssuesGet_IssueNotFound, TestRunIssuesPostComment_Create, TestRunIssuesPostComment_Update, TestRunIssuesPostComment_InvalidNumber, TestRunIssuesPostComment_EmptyMarker, TestRunIssuesPostComment_DryRun, and TestRunIssuesPostComment_FromFile. Coverage for newIssuesGetCmd rose from 42.9% to 92.3%, newIssuesPostCommentCmd from 51.6% to 94.1%.

Disagreed (1):

  1. behaviour test failure: The behaviour test failure is a flaky test that also fails on main (confirmed via gh api: run 31552214657 on main had the same behaviour job failure). The PR's changes are limited to CLI commands under internal/cli/ and do not affect the behaviour test infrastructure (dispatch, triage, fork features). Re-running CI should resolve this.

Tests: passed

Decision points
  • Chose config struct + testClient override pattern over mocking or interface injection (alternatives: Dependency injection via constructor parameter, Interface-based mock, Testing through cobra command execution with real tokens; rationale: The config struct with test override fields is the established pattern in this codebase (reposMigrateConfig, installConfig). It keeps the public API unchanged while enabling testability.)

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 12, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 4:17 PM UTC · Completed 4:29 PM UTC

Commit: c16d78e · View workflow run →

@ralphbean

ralphbean commented Aug 12, 2026

Copy link
Copy Markdown
Member

Ran a functional test against Jira staging with issues post-comment --tracker jira — comment creation and the find/update-by-marker lookup both worked.

Two things came up along the way that feel more like Jira/ADF format quirks than bugs in this PR, so I filed them as follow-ups instead of blocking here:

Nothing here looks like it needs to change in this PR, just flagging for visibility.

@ralphbean

Copy link
Copy Markdown
Member

/fs-fix remaining findings

@fullsend-ai-coder

fullsend-ai-coder Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor Author

🤖 Finished Fix · ✅ Success · Started 7:32 PM UTC · Completed 7:42 PM UTC

Commit: c16d78e · View workflow run →

Add --number validation in runIssuesGet matching runIssuesPostComment,
initialize nil Labels to empty slice for consistent JSON output,
and update jira-integration.md to reflect that tracker.Client Jira
support and the CLI primitive have landed while agent-pipeline
integration remains pending.

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

Copy link
Copy Markdown
Contributor Author

🔧 Fix agent — iteration 2 (human-triggered)

Addressed 3 of 4 review findings: added --number validation in runIssuesGet, fixed nil labels JSON serialization, and updated stale jira-integration.md docs. Disagreed with 1 finding (marker spoofing author verification is out of scope — tracker.Client interface design decision belonging to #5988).

Fixed (3):

  1. stale-doc: jira-integration.md says no Jira write support exists (docs/guides/user/jira-integration.md): Updated 'Event semantics — input only' section and 'Repo configuration' paragraph to acknowledge that the CLI primitive (fullsend issues post-comment --tracker jira) and tracker.Client Jira implementation have landed, while clarifying that the built-in agent pipeline does not use them yet. Updated Add Jira comment write support and implement tracker.Client for Jira #5989 reference and removed the 'Until both land' language in favor of scoping to Add JIRA support to the triage agent #2264 as the remaining blocker.
  2. missing input validation: runIssuesGet does not validate --number is positive (internal/cli/issues.go): Added 'if cfg.number <= 0' validation guard at the start of runIssuesGet, matching the existing pattern in runIssuesPostComment. Added TestRunIssuesGet_InvalidNumber test covering 0, -1, and -100.
  3. JSON output inconsistency: nil labels serialize as null instead of [] (internal/cli/issues.go): Initialize Labels to an empty slice when nil before assigning to the result struct, so JSON output produces '"labels": []' instead of '"labels": null'. Added TestRunIssuesGet_NilLabelsOutputAsEmptyArray test that verifies the raw JSON output.

Disagreed (1):

  1. marker spoofing / missing author verification in findMarkedTrackerComment: The tracker.Client interface (defined in the dependency PR Introduce tracker.Client: a forge-agnostic issue-content interface for GitHub/GitLab #5988) intentionally omits GetAuthenticatedUser. Adding it would expand the interface contract across all three backends (GitHub, GitLab, Jira) and is a cross-PR design decision. The current code already documents this tradeoff in the godoc on postTrackerStickyComment (the command targets trusted CI environments). If the command is later exposed to untrusted callers, author verification can be added as a follow-up.

Tests: passed

Decision points

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 12, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 7:44 PM UTC · Completed 7:59 PM UTC

Commit: 394b193 · View workflow run →

fullsend-ai-review[bot]

This comment was marked as outdated.

@waynesun09 waynesun09 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Multi-agent review squad pass (Claude + Grok reviewers, findings cross-verified against source). Posting only the new medium+ findings — everything already covered in earlier review rounds (marker author-check tradeoff, #6148/#6150 follow-ups, resolved items) is deliberately not re-raised. 4 inline comments follow: one high (Jira 32 KiB hard failure vs sticky's 65,000-byte cap), three medium (escaped-marker idempotency break + untested Jira round-trip, --jira-email optional vs poll.go's required-email rationale, config-default tracker from #5991 scope silently dropped).

Comment thread internal/cli/issues.go
Comment thread internal/cli/issues.go
Comment thread internal/cli/tracker_client.go
Comment thread internal/cli/issues.go Outdated
@ralphbean ralphbean added the fullsend-fix Enables automatic bot-triggered fix runs on human-authored PRs label Aug 13, 2026
@ralphbean
ralphbean requested a review from waynesun09 August 13, 2026 18:55
@ralphbean

Copy link
Copy Markdown
Member

/fs-fix fix the input validation gap issue, but ignore the scope creep finding. dismiss that. fix marker spoofing, but ignore secrets exposure. fix both naming-convention issues and both docs gaps issues.

@fullsend-ai-coder

fullsend-ai-coder Bot commented Aug 14, 2026

Copy link
Copy Markdown
Contributor Author

🤖 Finished Fix · ✅ Success · Started 1:55 PM UTC · Completed 2:11 PM UTC

Commit: 372f4e6 · View workflow run →

- Normalize --tracker to lowercase and validate against known set in
  resolveTracker, fixing the input validation gap where case-mismatched
  input (e.g. --tracker JIRA) skipped tracker-specific protections
- Unexport TrackerGitHub/TrackerGitLab/TrackerJira constants to match
  cli package conventions (all other string constants are unexported)
- Rename resolveGitHubToken → resolveGitHubTrackerToken and
  resolveJiraToken → resolveJiraTrackerToken for symmetry with the
  existing resolveGitLabTrackerToken
- Document marker-spoofing trust model in post-comment CLI help text
  (not just godoc), noting the trusted-CI-only assumption
- Add tracker field to layered-config-reference.md per-field table
- Add user-facing guide docs/guides/user/issues-commands.md explaining
  when and how to use fullsend issues get/post-comment
- Add issues.go and tracker_client.go to cli-internals.md key source
  files table

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

Copy link
Copy Markdown
Contributor Author

🔧 Fix agent — iteration 3 (human-triggered)

Addressed 6 of 8 review findings per human instruction. Fixed: input validation gap (case normalization + known-set validation in resolveTracker), marker spoofing (documented trust model in CLI help text and user guide), both naming-convention issues (unexported constants, symmetric function names), and both docs gaps (config reference table + new user-facing guide). Dismissed 2 per human instruction: scope divergence (--tracker vs --forge) and secrets exposure.

Fixed (6):

  1. input validation gap in resolveTracker (internal/cli/issues.go): Added validateTrackerName that normalizes tracker name to lowercase via strings.ToLower and validates against the known set (github, gitlab, jira). Case-mismatched input like --tracker JIRA now resolves correctly instead of skipping Jira-specific protections and failing with a confusing error in newTrackerClient.
  2. marker spoofing / missing author verification (internal/cli/issues.go): Documented the trust model in the post-comment command's Long CLI help text, noting that marker-based lookup does not verify comment author and is safe only in trusted CI environments. Also documented in the new user-facing guide. Adding GetAuthenticatedUser to tracker.Client is deferred as it would be a broader interface change belonging to Introduce tracker.Client: a forge-agnostic issue-content interface for GitHub/GitLab #5988.
  3. naming-convention: exported constants in internal package (internal/cli/tracker_client.go): Unexported TrackerGitHub/TrackerGitLab/TrackerJira to trackerGitHub/trackerGitLab/trackerJira, matching the cli package convention where all other string constants are unexported. Updated all references in issues.go, issues_test.go, and tracker_client_test.go.
  4. naming-convention: resolveGitLabTrackerToken inconsistent naming (internal/cli/tracker_client.go): Added the Tracker infix to the other two functions for symmetry: resolveGitHubToken → resolveGitHubTrackerToken and resolveJiraToken → resolveJiraTrackerToken. All three now follow the same naming pattern and clearly belong to the tracker client code path, avoiding confusion with resolveGitLabToken() in forge_client.go.
  5. missing-config-field-doc: tracker field not in config reference table (docs/guides/infrastructure/layered-config-reference.md): Added tracker field to both the per-field merge rules table and the code defaults table, plus a new subsection explaining its scalar override semantics and distinction from the forge field.
  6. missing-cli-command-doc: no user-facing guide for fullsend issues commands (docs/guides/user/issues-commands.md): Created a user-facing guide explaining when and how to use fullsend issues get and post-comment for custom agents posting to Jira or GitLab. Covers flags, examples, config-based default tracker, trust model, environment variables, and Jira marker constraints. Added sidebar entry in docs/.vitepress/config.ts. Also added issues.go and tracker_client.go to the Key Source Files table in cli-internals.md.

Disagreed (2):

  1. scope divergence from issue authorization (--tracker vs --forge): Human instruction explicitly says to dismiss this finding as scope creep. The --tracker flag name was agreed in the issue comments and is consistent with the internal tracker.Client interface naming.
  2. secrets exposure via CLI flag: Human instruction explicitly says to ignore this finding. The --token flag pattern is consistent with the existing post-comment command and environment variable fallback exists.

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 14, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 2:12 PM UTC · Completed 2:27 PM UTC

Commit: 8f4470c · View workflow run →

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review fullsend-ai-review Bot added ready-for-merge All reviewers approved — ready to merge and removed requires-manual-review Review requires human judgment labels Aug 14, 2026
@ralphbean
ralphbean enabled auto-merge August 14, 2026 15:19
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 14, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 3:20 PM UTC · Completed 3:34 PM UTC

Commit: 86356ca · 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.

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

  • internal/config/config.go:521: [low] config validation gap

The perRepoConfig.Tracker field is not validated during Validate(). This is consistent with the Forge field (which also skips Validate() checks), and resolveTracker validates the value at CLI invocation time via validateTrackerName. A config.yaml containing tracker: servicenow will parse without error, only failing when the CLI command is run.

Suggested fix: Consider adding tracker validation to perRepoConfig.Validate() for parity with Runtime/Inference.Provider validation.

  • internal/cli/issues.go:63: [low] scope divergence from issue authorization

Issue #5991 specifies --forge but the PR implements --tracker. The implementation provides documented architectural justification (forge != tracker: a GitHub-hosted repo may track issues in Jira). The code, docs, and tests are internally consistent with the --tracker naming; only the issue description needs updating.

Suggested fix: Update issue #5991 to document the design decision to use --tracker instead of --forge.

  • internal/cli/issues.go (file-level): Line 469 · [low] marker spoofing / missing author verification

findMarkedTrackerComment does not filter by comment author, unlike sticky.FindMarkedComment which accepts a botUser parameter. The godoc on postTrackerStickyComment and the user-facing documentation (docs/guides/user/issues-commands.md, Trust model section) explicitly document this as a conscious tradeoff for trusted CI environments where tracker.Client lacks GetAuthenticatedUser.

  • internal/cli/issues.go:140: [low] secrets exposure via CLI flag

The --token flag accepts API tokens as command-line arguments visible via /proc//cmdline. Environment variable fallback exists for all trackers. Consistent with the existing post-comment command pattern.

  • internal/tracker/jira_client_test.go:222: [low] test fidelity

The refactoring from unexported fakeJiraClient to exported FakeJiraClient changed UpdateComment behavior: the new fake performs the full jira.MarkdownToADF round-trip (more realistic), adding an implicit dependency on MarkdownToADF succeeding for test inputs.

  • internal/cli/issues.go:272: [low] edge case

testBody == "" cannot distinguish 'no override' from 'inject empty body.' The empty-body path is covered at the postTrackerStickyComment level (TestPostTrackerStickyComment_EmptyBody).

  • internal/cli/issues.go:381: [low] error handling

When LoadConfig returns a non-PerRepoConfigReader (e.g., org-mode config), resolveTracker falls through to the generic '--tracker is required' error rather than a more specific message.

@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Aug 14, 2026
@ralphbean
ralphbean added this pull request to the merge queue Aug 14, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Aug 14, 2026
@ralphbean
ralphbean added this pull request to the merge queue Aug 14, 2026
Merged via the queue into main with commit 8e66ce3 Aug 14, 2026
16 checks passed
@ralphbean
ralphbean deleted the agent/5991-issues-get-post-comment branch August 14, 2026 16:53
@fullsend-ai-retro

fullsend-ai-retro Bot commented Aug 14, 2026

Copy link
Copy Markdown

🤖 Finished Retro · ✅ Success · Started 4:55 PM UTC · Completed 5:10 PM UTC

Commit: 86356ca · View workflow run →

@fullsend-ai-retro

Copy link
Copy Markdown

Retro: PR #6144fullsend issues get/post-comment commands

Timeline

PR #6144 added fullsend issues get and fullsend issues post-comment CLI commands for forge-agnostic issue interaction (GitHub, GitLab, Jira). The code agent opened the PR on Aug 12 with 1767 additions across 18 files. Over ~2 days, the PR went through 4 /fs-fix invocations (one infrastructure failure), a substantial manual human commit addressing Jira-specific edge cases, 6 review iterations, and 3 merge queue attempts before merging on Aug 14.

Key events:

  • Code agent produced initial implementation (Aug 12 15:23)
  • Fix agent successfully refactored for testability and raised coverage from ~42% to ~94% (Aug 12 15:52)
  • Fix agent addressed 3 of 4 surface-level review findings (Aug 12 19:30)
  • waynesun09's multi-agent review squad found 1 high + 3 medium Jira-specific findings that the standard review agent missed entirely (Aug 12 20:25)
  • Fix agent invocation for waynesun09's findings failed due to shellcheck_py wheel build HTTP 503 (Aug 12 20:44)
  • Human (ralphbean) manually implemented all 4 of waynesun09's findings in commit 372f4e6 — cross-package architectural work spanning 11 files and 6 packages (Aug 13)
  • Fix agent addressed remaining low-severity findings per selective human instructions (Aug 14 13:54)
  • Merge queue failed twice (GCP WIF HTTP 429 rate limiting; cancelled downstream workflow) before succeeding on third attempt (Aug 14 15:30–16:53)

Review quality

The standard review agent (fullsend-ai-review) found 6 findings (2 medium, 4 low) — all surface-level issues like naming conventions, missing docs entries, and secrets exposure via CLI flags. waynesun09's review squad found 4 deeper findings that the review agent missed entirely:

  1. (high) Jira's 32 KiB markdown limit vs sticky's 65,000-byte default — comments in the 32–65 KiB band would pass sticky's trim but fail at jira.MarkdownToADF
  2. (medium) Jira's ADF round-trip silently escapes marker characters, breaking sticky idempotency on re-runs
  3. (medium) Making --jira-email optional contradicts the existing buildJiraClient pattern in poll.go, reintroducing a silent-401 failure mode
  4. (medium) The issue spec (Add fullsend issues get/fullsend issues post-comment, deprecate top-level fullsend post-comment #5991) required a config-based default tracker, but the implementation required --tracker unconditionally

All four required cross-package reasoning or domain-specific knowledge about Jira's behavior — a pattern consistent with existing issue #898 and #1525 (review agent misses cross-file impact analysis that review squads catch). This PR provides additional evidence that the gap is most acute when new code integrates with external service APIs that have undocumented constraints.

Evidence for existing issues

Human note for retro agent

ralphbean explicitly requested a follow-up issue for marker spoofing in findMarkedTrackerComment — see proposal below.

Proposals filed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature Feature-category issue awaiting human prioritization fullsend-fix Enables automatic bot-triggered fix runs on human-authored PRs go Pull requests that update go code ready-for-merge All reviewers approved — ready to merge ready-for-review Triggers review agent dispatch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add fullsend issues get/fullsend issues post-comment, deprecate top-level fullsend post-comment

2 participants