Skip to content

chore: add "jira" as a valid forge platform key - #6272

Merged
ralphbean merged 3 commits into
mainfrom
chore/add-jira-forge-key
Aug 18, 2026
Merged

chore: add "jira" as a valid forge platform key#6272
ralphbean merged 3 commits into
mainfrom
chore/add-jira-forge-key

Conversation

@ralphbean

Copy link
Copy Markdown
Member

Summary

Test plan

  • TestValidForgePlatform updated and passing
  • Full ./internal/harness/ test suite passing
  • CLI forge-related tests passing

🤖 Generated with Claude Code

Temporary addition to unblock the triage-agent PoC for JIRA issues.
The code agent remains blocked on a longer-term forge abstraction
design currently under debate in #6237.

Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Ralph Bean <rbean@redhat.com>
@ralphbean
ralphbean requested a review from a team as a code owner August 17, 2026 02:18
@ralphbean ralphbean added the fullsend-fix Enables automatic bot-triggered fix runs on human-authored PRs label Aug 17, 2026
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Allow "jira" as a forge platform key for harness validation

✨ Enhancement 🧪 Tests 🕐 10-20 Minutes

Grey Divider

AI Description

• Add "jira" to the recognized forge platform keys to allow --forge jira.
• Document this as a temporary unblock for the triage-agent JIRA PoC.
• Extend harness validation tests to cover the new forge key.
Diagram

graph TD
  CLI["fullsend CLI (--forge)"] --> VFP["ValidForgePlatform()"] --> Keys["validForgeKeys map"]
  Cfg["ForgeConfig"] --> VFP
  Tests["TestValidForgePlatform"] --> VFP
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Registry-based forge providers (interface + registration)
  • ➕ Avoids hard-coded key allow-lists; each forge registers itself
  • ➕ Scales better as more platforms are added
  • ➖ Larger refactor; not aligned with this PR’s stated goal (temporary unblock)
  • ➖ Would require design agreement across agents/commands
2. Config-driven allow-list (env/config file)
  • ➕ Unblocks new keys without code changes
  • ➕ Enables experimentation without changing binaries
  • ➖ Shifts validation to runtime; risk of unsupported combinations
  • ➖ Still doesn’t solve the longer-term abstraction problem
3. Feature-flagged acceptance of "jira"
  • ➕ Makes the temporary nature explicit and easy to roll back
  • ➕ Limits exposure to PoC usage
  • ➖ Adds flag plumbing and branching for a very small change
  • ➖ May be unnecessary if this is already an internal-only harness concern

Recommendation: Given the explicit intent to be a temporary unblock, this PR’s minimal change (add a single allow-listed key plus test coverage) is appropriate. Keep the inline comment pointing to the ongoing abstraction discussion, and ensure follow-up work replaces the allow-list with a proper forge/provider abstraction once the design in #6237 lands.

Files changed (2) +5 / -0

Enhancement (1) +4 / -0
forge.goAdd "jira" to supported forge platform keys (temporary) +4/-0

Add "jira" to supported forge platform keys (temporary)

• Extends the 'validForgeKeys' allow-list to recognize "jira" as a valid forge platform key. Adds contextual comments documenting the temporary nature and linking to the longer-term abstraction discussion.

internal/harness/forge.go

Tests (1) +1 / -0
harness_test.goTest that "jira" is accepted by ValidForgePlatform +1/-0

Test that "jira" is accepted by ValidForgePlatform

• Updates 'TestValidForgePlatform' to assert that "jira" is treated as a recognized forge platform key, alongside the existing "github" and "gitlab" cases.

internal/harness/harness_test.go

@fullsend-ai-review

fullsend-ai-review Bot commented Aug 17, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 2:19 AM UTC · Completed 2:32 AM UTC

Commit: 302cd85 · View workflow run →

@codecov

codecov Bot commented Aug 17, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@qodo-code-review

qodo-code-review Bot commented Aug 17, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Jira forge routed to GitHub ✗ Dismissed 🐞 Bug ≡ Correctness
Description
Because "jira" is now considered a valid forge key, --forge jira can flow into code paths that
only special-case GitLab and otherwise assume GitHub. This can cause status notifications and
reconcile-status to mint/use GitHub credentials and GitHub-specific behavior even though the user
explicitly selected "jira".
Code

internal/harness/forge.go[R30-33]

+	// "jira" is a temporary addition to unblock the triage-agent PoC for
+	// JIRA issues. The code agent remains blocked on a longer-term forge
+	// abstraction design (see https://github.com/fullsend-ai/fullsend/pull/6237).
+	"jira": true,
Relevance

●●● Strong

Team often adds validation/guards when new platform keys could trigger wrong code paths.

PR-#5643
PR-#6144

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The PR makes ValidForgePlatform("jira") return true, which lets detectForgePlatform accept and
return "jira". Status notification setup and status reconciliation only special-case "gitlab"
and otherwise execute the GitHub setup path, so "jira" is treated as GitHub in those flows.

internal/harness/forge.go[27-40]
internal/cli/run.go[3270-3329]
internal/cli/reconcilestatus.go[63-82]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The PR adds `jira` to the accepted forge keys, which allows `--forge jira` through validation. However, CLI status-related code paths only branch on `forgePlatform == "gitlab"` and treat all other values as GitHub, so `jira` currently triggers GitHub token minting / GitHub client setup.

## Issue Context
`--forge` is being used both to select harness `forge:` overrides and (implicitly) to choose forge-specific runtime integrations (status notifications / reconciliation). Accepting `jira` without adding explicit handling makes it easy to silently run the wrong provider path.

## Fix Focus Areas
- internal/cli/run.go[3303-3329]
- internal/cli/reconcilestatus.go[63-82]
- internal/harness/forge.go[27-39]

## Proposed fix
- In `setupStatusNotifier`, switch explicitly on `forgePlatform`:
 - `case "gitlab"`: existing GitLab path
 - `case "github", ""`: existing GitHub path
 - `default` (includes `"jira"`): return a clear error like `status notifications unsupported for forge %q; use --forge github|gitlab for status notifications`.
 This will cause `fullsend run` to disable status notifications with an explicit warning (current behavior already warns on notifier setup error).

- In `reconcile-status`, similarly error out when `forgePlatform` is neither `gitlab` nor `github`/empty, since reconciliation only supports GitHub/GitLab.

- (Optional) Add a small unit/integration test asserting `--forge jira` disables status notifications (run) and is rejected for reconcile-status, if tests exist for these code paths.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context sources
✅ Compliance rules (platform): 54 rules

Grey Divider

Tip of the day
💡 Did you know, you can keep summaries lean with Finding overflow, which tucks the rest behind 'View more'

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread internal/harness/forge.go
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 17, 2026

Copy link
Copy Markdown

Review

Findings

Low

  • [consumer completeness] internal/cli/forge_client.go:39 — Adding "jira" to validForgeKeys allows --forge jira to pass harness validation, but downstream switch statements in newForgeClient, FetchRemoteScaffold, and forgeURL only handle "github" and "gitlab". The primary consumer path (fullsend run) does not reach these switches — it passes the platform string into ComposeOpts.ForgePlatform without calling newForgeClient. The repos install --forge jira path is gated by a separate repos.IsValidForge() map that does not include "jira". The practical risk is narrow — limited to hypothetical future callers.
    Remediation: Consider adding a comment in newForgeClient's default branch noting that "jira" is a valid forge key but intentionally unsupported for client creation until the forge abstraction design lands.

  • [scope-authorization-missing] — No linked issue authorizing this change. The PR body references PR docs!: add ADR 0088 for CEL-guarded overlays in the harness schema #6237 as the broader design work and describes this as a minimal unblock for the triage-agent PoC.
    Remediation: Link the issue that requested JIRA forge support, or create one documenting the triage-agent PoC requirements.

  • [stale-enumeration] docs/guides/user/running-agents-locally.md:212 — The --forge flag documentation states valid values are github, gitlab but this PR adds jira as a third valid platform for fullsend run.
    Remediation: Update to include jira in the list of valid forge platforms.

  • [stale-enumeration] docs/guides/user/running-agents-locally.md:255 — The --forge flag documentation for the run command states valid values are github or gitlab.
    Remediation: Update to include jira.

  • [stale-enumeration] docs/guides/dev/cli-internals.md:94 — The --forge flag documentation for the run command in the developer CLI reference lists github, gitlab only.
    Remediation: Update to include jira.

Previous run

Review

Findings

Low

  • [consumer completeness] internal/cli/forge_client.go:39 — Adding "jira" to validForgeKeys allows --forge jira to pass harness validation, but downstream switch statements in newForgeClient, FetchRemoteScaffold, and forgeURL only handle "github" and "gitlab". The primary consumer path (fullsend run) does not reach these switches — it passes the platform string into ComposeOpts.ForgePlatform without calling newForgeClient. The repos install --forge jira path is gated by a separate repos.IsValidForge() map that was not modified, so it would reject "jira" before reaching newForgeClient. The practical risk is narrow — limited to hypothetical future callers — but no compile-time or test-time guard prevents them from hitting the gap.
    Remediation: Consider adding a comment in newForgeClient's default branch noting that "jira" is a valid forge key but intentionally unsupported for client creation until the forge abstraction design lands.

  • [scope-authorization-missing] — No linked issue authorizing this change. The PR body references PR docs!: add ADR 0088 for CEL-guarded overlays in the harness schema #6237 as the broader design work and describes this as a minimal unblock for the triage-agent PoC.
    Remediation: Link the issue that requested JIRA forge support, or create one documenting the triage-agent PoC requirements.

  • [scope-label-mismatch] — PR is labeled fullsend-fix but adds new platform support (jira forge key). The PR title uses the chore: prefix, suggesting a chore label would be more accurate.
    Remediation: Consider whether type/chore would be more appropriate.


Labels: PR modifies internal harness forge configuration (internal/harness/forge.go)

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 component/harness Agent harness, config, and skills loading labels Aug 17, 2026

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

Review-only sweep: 2 findings inline (design/schema conflict on the forge/tracker split, and a test-coverage gap in the new assertions).

Comment thread internal/harness/forge.go
Comment thread internal/harness/harness_test.go
Address review feedback: update TestValidate_ForgeUnrecognizedKey to
assert the full valid-keys string including "jira", and add
TestValidateForge_JiraBlock exercising validateForge/ResolveForge with
a forge.jira: block.

Signed-off-by: Ralph Bean <rbean@redhat.com>
Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Ralph Bean <rbean@redhat.com>

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

LGTM — small, well-tested addition of "jira" as a valid forge key.

@ralphbean
ralphbean enabled auto-merge August 17, 2026 20:06
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 18, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 4:05 AM UTC · Completed 4:23 AM UTC

Commit: 7c48cdf · View workflow run →

@ralphbean
ralphbean added this pull request to the merge queue Aug 18, 2026
Merged via the queue into main with commit ac0355c Aug 18, 2026
17 of 18 checks passed
@ralphbean
ralphbean deleted the chore/add-jira-forge-key branch August 18, 2026 17:32
@fullsend-ai-retro

fullsend-ai-retro Bot commented Aug 18, 2026

Copy link
Copy Markdown

🤖 Finished Retro · ❌ Failure · Started 5:35 PM UTC · Completed 5:35 PM UTC

Commit: 7c48cdf · View workflow run →

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

Labels

component/harness Agent harness, config, and skills loading fullsend-fix Enables automatic bot-triggered fix runs on human-authored PRs ready-for-merge All reviewers approved — ready to merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants