Skip to content

feat(#1074): add Jira-source overlay to code harness - #1075

Open
fullsend-ai-coder[bot] wants to merge 11 commits into
mainfrom
agent/1074-jira-source-code-overlay
Open

feat(#1074): add Jira-source overlay to code harness#1075
fullsend-ai-coder[bot] wants to merge 11 commits into
mainfrom
agent/1074-jira-source-code-overlay

Conversation

@fullsend-ai-coder

Copy link
Copy Markdown
Contributor

Summary

  • Port the code harness from the deprecated forge: block to overlays: with CEL when: expressions, matching the pattern established in the triage harness (Port triage agent from forge to overlays method #1013)
  • Add a Jira-source overlay (event.source.system == "jira") that fetches issue content via fullsend issues get --tracker jira on the runner, writes it to a context file, and copies it into the sandbox without exposing Jira credentials
  • Update the code-implementation skill to read the prepared .issue-context.json when FULLSEND_SOURCE_TRACKER=jira instead of calling forge APIs

Design

The Jira overlay is ordered before the forge overlays so its pre_script overrides the default forge pre-script. Post-script selection remains based on runtime.forge, so a Jira→GitHub run still pushes and creates a PR through GitHub. Both the Jira-source and target-forge overlays match and merge for a Jira-triggered run (merge-all-matching behavior).

Jira credentials (JIRA_USER_EMAIL, JIRA_TOKEN, JIRA_BASE_URL) are scoped to env.runner only — they never enter the sandbox. The sandbox receives the issue content as a JSON file at /sandbox/workspace/.issue-context.json and a FULLSEND_SOURCE_TRACKER=jira signal.

Testing

  • Added pre-code-jira-test.sh with 12 test cases covering:
    • Jira→GitHub and Jira→GitLab compositions succeed
    • Invalid Jira URL patterns rejected
    • Non-atlassian.net hosts rejected
    • JIRA_BASE_URL mismatch rejected
    • Trailing-slash normalization
    • fullsend CLI failure handling
    • Empty/invalid JSON response handling
    • Missing credential validation
    • Context file content matches fetched JSON
  • All existing pre-code-test.sh and post-code-test.sh tests pass unchanged
  • Bundle check (make check-bundle) passes

Closes #1074

Post-script verification

  • Branch is not main/master (agent/1074-jira-source-code-overlay)
  • Secret scan passed (gitleaks — 20bd058ec31d8cd9a6c34bb9d04b5247b0440fce..HEAD)
  • PR body secret scan passed (gitleaks — no-git)

Replace the deprecated forge: block in harness/code.yaml with
overlays: entries using CEL when: expressions.  Add a Jira-source
overlay guarded by event.source.system == "jira" that:

- overrides pre_script with a Jira-aware pre-code-jira.sh that
  fetches the issue via fullsend issues get --tracker jira,
  validates the Jira URL and credentials, and writes the issue
  context to /tmp/jira-issue-context.json
- copies the context file into the sandbox at
  /sandbox/workspace/.issue-context.json via host_files, keeping
  Jira credentials on the runner only
- sets FULLSEND_SOURCE_TRACKER=jira in the sandbox so the agent
  knows to read the prepared context file

Post-script selection remains based on runtime.forge so GitHub
targets still push via GitHub and GitLab targets via GitLab.
Both the Jira-source and target-forge overlays match and merge
for a Jira-triggered run.

Update the code-implementation skill to check for
FULLSEND_SOURCE_TRACKER=jira and read .issue-context.json
instead of calling gh issue view or the GitLab API.

Add pre-code-jira-test.sh covering URL validation, credential
checks, context file creation, and error paths for both
Jira->GitHub and Jira->GitLab compositions.

Closes #1074
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 27, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 10:11 PM UTC · Completed 10:53 PM UTC

Commit: affef07 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high

@fullsend-ai-review fullsend-ai-review Bot added the risk/elevated PR risk: elevated label Aug 27, 2026
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 27, 2026

Copy link
Copy Markdown

Risk Assessment: elevated (3/5)

Details

Large blast radius (5), 11 protected path changes (5), and CI workflow modification (4) drive Tier 1 to 2.63; high churn (4), multi-author contention (5), and heavy fix/revert history (5) in Tier 2 produce 3.14; Tier 3 remains low (1.67) as this is an additive-only Jira overlay with clear issue scope, yielding a weighted composite of 2.59 that rounds to 3 (elevated), consistent with the prior assessment.

Previous run

Risk Assessment: elevated (3/5)

Details

Large blast radius (5), 11 protected path changes (5), and CI workflow modification (4) drive Tier 1 up, while high churn (4) and multi-author contention (5) in Tier 2 reinforce elevated risk; Tier 3 is low (1.83) due to additive-only overlay with clear issue scope, but preserving prior score of 3 as Tier 1 signals are unchanged and Tier 2 high-signal dimensions support elevated level.

Previous run (2)

Risk Assessment: elevated (3/5)

Details

Anchored to prior score of 3. Tier 1 signals unchanged: large blast radius (5), 11 protected path changes (5), and CI workflow modification (4) drive risk up, offset by bot authorship (1), no security-sensitive files (1), and no dependency changes (1). Only 3 files changed since prior review — all documentation and configuration updates. Tier 2 churn and multi-author contention signals remain consistent. Tier 3: well-specified linked issue with clear acceptance criteria and CEL-guarded activation reduces deployment risk.

Previous run (3)

Risk Assessment: elevated (3/5)

Details

Anchored to prior score of 3. Tier 1 signals unchanged: large blast radius (5), 11 protected path changes (5), and CI workflow modification (4) drive risk up, offset by bot authorship (1), no security-sensitive files (1), and no dependency changes (1). Tier 2 confirms high churn and multi-author contention across existing files. Bulk of change is new Jira-source scripts, limiting regression risk. Tier 3: well-specified issue with clear acceptance criteria and CEL-guarded activation reduces deployment risk.

Previous run (4)

Risk Assessment: elevated (3/5)

Details

Anchored to prior score of 3. Tier 1 signals unchanged: large blast radius (5), 5 protected path changes (5), and CI workflow modification (4) drive risk up, offset by bot authorship (1), no security-sensitive files (1), and no dependency changes (1). Tier 2 shows high churn and multi-author contention across existing files. Bulk of change is new scripts, limiting regression risk. Tier 3: well-specified issue with clear acceptance criteria reduces ambiguity.

Previous run (5)

Risk Assessment: elevated (3/5)

Details

Anchored to prior score of 3. Tier 1 signals unchanged: large blast radius (5), 5 protected path changes (5), and CI workflow modification (4) drive risk up, offset by bot authorship, no security-sensitive files, and no dependency changes. High churn on harness/code.yaml and SKILL.md (Tier 2) adds historical risk, but the bulk of the change is new scripts. Well-specified issue with clear acceptance criteria (Tier 3) reduces ambiguity.

Previous run (6)

Risk Assessment: elevated (3/5)

Details

Anchored to prior score of 3. Tier 1 signals unchanged: large blast radius (5), 5 protected path changes (5), and CI workflow modification (4) drive risk up, offset by bot authorship, no security-sensitive files, and no dependency changes. Tier 2 confirms high churn and multi-author contention on existing files. Good issue alignment and largely additive change sustain the prior elevated assessment.

Previous run (7)

Risk Assessment: elevated (3/5)

Details

Tier 1 signals unchanged from prior assessment — large blast radius with 5 protected path changes and CI workflow modifications drive risk up, while high churn and multi-author contention on existing files elevate Tier 2, but the change is largely additive new scripts by a bot with clear issue alignment and no security-sensitive changes, sustaining the prior elevated score.

Previous run (8)

Risk Assessment: elevated (3/5)

Details

Large blast radius with 5 protected path changes and CI workflow modifications drive Tier 1 up, while high churn and multi-author contention on existing files elevate Tier 2, but the change is largely additive new scripts by a bot with clear issue alignment.

@fullsend-ai-review

fullsend-ai-review Bot commented Aug 27, 2026

Copy link
Copy Markdown

Review

Findings

High

  • [logic-error] scripts/post-code.src.sh:233 — Jira+GitLab composition will fail at runtime. When FULLSEND_FORGE=gitlab and event.source.system=jira, the Jira overlay (ordered last, last-writer-wins) sets ISSUE_URL to the Jira browse URL (e.g., https://<site>.atlassian.net/browse/PROJ-42). The GitLab validation block calls forge_validate_issue_url on ISSUE_URL, which checks against the GitLab URL pattern (^https://[a-zA-Z0-9._-]+(/[a-zA-Z0-9._-]+)+/-/issues/[0-9]+$). The Jira URL will never match, causing exit 1. Additionally, GITLAB_HOST derivation from ISSUE_URL would extract the Jira host instead of the GitLab host, breaking all subsequent GitLab API calls. EXTERNAL_WORK_ITEM is already set to true before this block, so it can be used to gate the behavior.
    Remediation: When EXTERNAL_WORK_ITEM=true and FULLSEND_FORGE=gitlab, skip the forge_validate_issue_url check on ISSUE_URL and derive GITLAB_HOST from CI_SERVER_HOST instead.

Medium

  • [protected-path] harness/code.yaml, scripts/lib/post-failure-report.lib.sh, scripts/post-code-test.sh, scripts/post-code.sh, scripts/post-code.src.sh, scripts/post-fix.sh, scripts/pre-code-jira-test.sh, scripts/pre-code-jira.sh, scripts/pre-code-jira.src.sh, scripts/validate-code-output.sh, skills/code-implementation/SKILL.md — 11 of 14 changed files are under protected paths (harness/, scripts/, skills/). The PR links to issue code agent: read Jira source issues through an event overlay #1074 and the description explains the rationale for modifying governance and infrastructure files. Human approval is always required for protected-path changes.

Low

  • [code-duplication] scripts/pre-code-jira.src.sh:129 — Pre-commit tool resolution block (lines 129–183) is a verbatim copy from pre-code.src.sh. The codebase has an established lib/*.lib.sh pattern (e.g., code-ops.lib.sh, prescript-output.lib.sh). The inline comment acknowledges the duplication ("This section is shared with the forge pre-script") but does not extract it.
    Remediation: Extract the shared pre-commit resolution logic to scripts/lib/precommit-resolve.lib.sh and source it from both pre-code.src.sh and pre-code-jira.src.sh.

  • [edge-case] scripts/pre-code-jira.src.sh:107 — The Jira pre-script omits the existing-PR check present in the forge pre-script. A detailed comment block documents this as intentional: forge_list_prs_for_issue searches for closing keywords targeting forge issue numbers (#N), which would produce false negatives for Jira-sourced issues that use keys (PROJ-42). A Jira-aware existing-PR check would require cross-system linking integration.

  • [Cross-work-item injection check weakening] scripts/post-code.src.sh:603 — The external-work-item PR ownership check uses grep -qwF to match WORK_ITEM_URL in the PR body. The -w (whole-word) flag mitigates the substring false-positive concern from the prior review — grep -wF 'PROJ-1' will not match PROJ-12 because the trailing digit is a word character. URL structure introduces a theoretical edge case with query parameters or fragments appended to the URL, but the primary defense (branch namespace agent/<WORK_ITEM_KEY>-*) remains unaffected.


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

Medium

  • [protected-path] harness/code.yaml, scripts/lib/post-failure-report.lib.sh, scripts/post-code-test.sh, scripts/post-code.sh, scripts/post-code.src.sh, scripts/post-fix.sh, scripts/pre-code-jira-test.sh, scripts/pre-code-jira.sh, scripts/pre-code-jira.src.sh, scripts/validate-code-output.sh, skills/code-implementation/SKILL.md — 11 of 14 changed files are under protected paths (harness/, scripts/, skills/). The PR links to issue code agent: read Jira source issues through an event overlay #1074 and the description explains the rationale for modifying governance and infrastructure files. Human approval is always required for protected-path changes.

Low

  • [code-duplication] scripts/pre-code-jira.src.sh:129 — Pre-commit tool resolution block (lines 129–183) is a verbatim copy from pre-code.src.sh. The codebase has an established lib/*.lib.sh pattern (e.g., code-ops.lib.sh, prescript-output.lib.sh). The inline comment acknowledges the duplication ("This section is shared with the forge pre-script") but does not extract it.
    Remediation: Extract the shared pre-commit resolution logic to scripts/lib/precommit-resolve.lib.sh and source it from both pre-code.src.sh and pre-code-jira.src.sh.

  • [edge-case] scripts/pre-code-jira.src.sh:107 — The Jira pre-script omits the existing-PR check present in the forge pre-script. A detailed comment block documents this as intentional: forge_list_prs_for_issue searches for closing keywords targeting forge issue numbers (#N), which would produce false negatives for Jira-sourced issues that use keys (PROJ-42). A Jira-aware existing-PR check would require cross-system linking integration.

  • [Cross-work-item injection check weakening] scripts/post-code.src.sh:597 — The external-work-item PR ownership check uses grep -qwF to match WORK_ITEM_URL in the PR body. The -w (whole-word) flag mitigates the substring false-positive concern from the prior review — grep -wF 'PROJ-1' will not match PROJ-12 because the trailing digit is a word character. URL structure introduces a theoretical edge case with query parameters or fragments appended to the URL, but the primary defense (branch namespace agent/<WORK_ITEM_KEY>-*) remains unaffected.


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 (2)

Review

Findings

Medium

  • [env-var-contract-violation] harness/code.yaml:68CODE_ALLOWED_TARGET_BRANCHES was removed from top-level env.runner. AGENTS.md section 8 explicitly cites this variable as a canonical passthrough example that "must stay as ${VAR} passthrough." The post-code script reads it with ${CODE_ALLOWED_TARGET_BRANCHES:-} to enforce branch restrictions — if the harness no longer passes it through, branch restriction silently becomes a no-op. The removal also renders the AGENTS.md example at line 160 factually incorrect. See also: [scope-creep] — the removal is incidental to the forge→overlays migration and not authorized by issue code agent: read Jira source issues through an event overlay #1074.
    Remediation: Restore CODE_ALLOWED_TARGET_BRANCHES: "${CODE_ALLOWED_TARGET_BRANCHES}" in the top-level env.runner block, or update AGENTS.md section 8 to reference a different canonical passthrough example if the removal is intentional.

  • [logic-error] skills/code-implementation/SKILL.md:207 — SKILL.md tells the sandbox agent to "derive the key from FULLSEND_WORK_ITEM_URL" in three places (R3 retry section, Step 1 identification, and work-item recording). However, FULLSEND_WORK_ITEM_URL is never declared in env.sandbox — the Jira overlay maps it to ISSUE_URL (ISSUE_URL: "${FULLSEND_WORK_ITEM_URL}"). The agent will fail to find the variable at runtime.
    Remediation: Replace all SKILL.md references from FULLSEND_WORK_ITEM_URL to ISSUE_URL, matching the sandbox env mapping in harness/code.yaml.

  • [protected-path] harness/code.yaml, scripts/lib/post-failure-report.lib.sh, scripts/post-code-test.sh, scripts/post-code.sh, scripts/post-code.src.sh, scripts/post-fix.sh, scripts/pre-code-jira-test.sh, scripts/pre-code-jira.sh, scripts/pre-code-jira.src.sh, scripts/validate-code-output.sh, skills/code-implementation/SKILL.md — 11 of 14 changed files are under protected paths (harness/, scripts/, skills/). The PR links to issue code agent: read Jira source issues through an event overlay #1074 and the description explains the rationale for modifying governance and infrastructure files. Human approval is always required for protected-path changes.

Low

  • [Cross-work-item injection check weakening] scripts/post-code.src.sh:597 — The external-work-item PR ownership check uses grep -qF substring match on the full WORK_ITEM_URL. A URL ending in /PROJ-1 is a substring of /PROJ-12, creating a false positive that weakens the defense-in-depth guard against cross-work-item commit injection. The primary defense (branch namespace agent/<WORK_ITEM_KEY>-*) is not affected.
    Remediation: Use line-anchored grep or append a word boundary to the pattern.

  • [naming-inconsistency] scripts/pre-code-jira.src.sh:33 — Helper function is named _sanitize_gha but the codebase convention is _gha_sanitize (used in 30+ files). The implementation also omits the ANSI-escape strip present in the canonical definition.
    Remediation: Rename to _gha_sanitize and add the ANSI-escape strip to match the canonical definition.

  • [code-duplication] scripts/pre-code-jira.src.sh:129 — Pre-commit tool resolution (lines 129–181) is a verbatim copy from pre-code.src.sh. Extracting to a shared library would follow the established lib/*.lib.sh pattern and prevent future drift.
    Remediation: Extract to scripts/lib/precommit-resolve.lib.sh.

  • [GHA-workflow-command-injection] scripts/pre-code-jira.src.sh:72 — The ::notice:: annotation interpolates ${ISSUE_URL}, ${PROJECT_KEY}, and ${ISSUE_KEY} without _sanitize_gha(). Values are regex-validated at line 41, constraining the character set to alphanumerics, dots, hyphens, and forward slashes — preventing practical injection. Defense-in-depth sanitization recommended for consistency.

  • [missing-validation] scripts/pre-code-jira.src.sh:28REPO_FULL_NAME is documented as required in the script header but not validated with a :? guard in the required-var block.

  • [edge-case] scripts/pre-code-jira.src.sh:107 — The Jira pre-script omits the existing-PR check present in the forge pre-script. Documented as intentional — forge_list_prs_for_issue would produce false negatives for Jira keys (PROJ-42 vs #N).

  • [style-inconsistency] scripts/post-code.src.sh:787ISSUE_REF_KEYWORD kept as an intermediary in production code but eliminated in the test helper build_pr_body. Both produce the same output, but the structural divergence reduces the test's value as a production mirror.


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 (3)

Review

Findings

Medium

  • [protected-path] harness/code.yaml, scripts/lib/post-failure-report.lib.sh, scripts/post-code-test.sh, scripts/post-code.sh, scripts/post-code.src.sh, scripts/post-fix.sh, scripts/pre-code-jira-test.sh, scripts/pre-code-jira.sh, scripts/pre-code-jira.src.sh, scripts/validate-code-output.sh, skills/code-implementation/SKILL.md — 11 of 14 changed files are under protected paths (harness/, scripts/, skills/). The PR links to issue code agent: read Jira source issues through an event overlay #1074 and the description explains the rationale for modifying governance and infrastructure files. Human approval is always required for protected-path changes.

  • [logic-error] skills/code-implementation/SKILL.md:224 — The SKILL.md instructions tell the agent to "derive the key from FULLSEND_WORK_ITEM_URL" but FULLSEND_WORK_ITEM_URL is not declared in the Jira overlay’s env.sandbox block and is therefore not available inside the sandbox. Only ISSUE_URL (set to the same value via ISSUE_URL: "") is available. The agent would fail to find the variable and fall back to unintended behavior.
    Remediation: Change SKILL.md references from FULLSEND_WORK_ITEM_URL to ISSUE_URL, or add FULLSEND_WORK_ITEM_URL to the Jira overlay’s env.sandbox block.

  • [env-var-contract-violation] harness/code.yaml:68CODE_ALLOWED_TARGET_BRANCHES was removed from the top-level env.runner block. AGENTS.md section 8 explicitly cites this as a canonical example of a computed passthrough that "must stay as passthrough". The post-code script reads it via so it still functions if users set it in their workflow env block, but the AGENTS.md contract is broken and the documentation is now factually wrong. See also: [scope-creep] and [stale-reference] findings at this location.
    Remediation: Restore CODE_ALLOWED_TARGET_BRANCHES: "" to the top-level env.runner block, or update AGENTS.md to remove the reference.

  • [scope-creep] harness/code.yaml:69 — Issue code agent: read Jira source issues through an event overlay #1074 authorizes adding the Jira-source overlay and migrating forge: to overlays:. The removal of CODE_ALLOWED_TARGET_BRANCHES from env.runner is incidental to the migration and not explicitly authorized. See also: [env-var-contract-violation] finding at this location.
    Remediation: Restore the passthrough line.

  • [stale-reference] AGENTS.md:160 — AGENTS.md cites CODE_ALLOWED_TARGET_BRANCHES as an example of a computed passthrough "in harness/code.yaml’s env.runner block". This PR removes that line, making the example factually incorrect after merge.
    Remediation: Replace the example with another passthrough that still exists, or restore the passthrough.

Low

  • [logic-error] skills/code-implementation/SKILL.md:246 — The SKILL.md fallback path calls gh issue view "" when FULLSEND_TRACKER=jira but .issue-context.json is missing. The Jira overlay sets ISSUE_NUMBER to empty string in the sandbox, so this executes gh issue view "" which errors. Safer than the prior state (which could hit an unrelated GitHub issue) but the fallback is non-functional.

  • [Cross-work-item injection check weakening] scripts/post-code.src.sh:598 — The external-work-item PR ownership check uses grep -qF substring match. A work-item URL ending in PROJ-1 is a substring of one ending in PROJ-12, creating a false positive that weakens the defense-in-depth guard against cross-work-item commit injection. The primary defense (branch namespace agent/<WORK_ITEM_KEY>-*) is not affected.
    Remediation: Use line-anchored grep or append a word boundary to the pattern.

  • [Host allowlist scope] scripts/pre-code-jira.src.sh:47 — The Jira host allowlist (*.atlassian.net) permits any subdomain. The JIRA_BASE_URL cross-check mitigates when set, but when unset the cross-check is skipped. Attack requires control over the event payload.

  • [GHA-workflow-command-injection] scripts/pre-code-jira.src.sh:72 — The ::notice:: annotation interpolates , , and `` without _sanitize_gha(). Values are regex-validated, constraining the character set. Defense-in-depth sanitization recommended for consistency.

  • [edge-case] scripts/pre-code-jira.src.sh:107 — The Jira pre-script omits the existing-PR check present in the forge pre-script. Documented as intentional — forge_list_prs_for_issue would produce false negatives for Jira keys (PROJ-42 vs #N).

  • [missing-validation] scripts/pre-code-jira.src.sh:28REPO_FULL_NAME is documented as required but not validated with a :? guard in the required-var block.

  • [naming-inconsistency] scripts/pre-code-jira.src.sh:33 — The helper function is named _sanitize_gha but the codebase convention is _gha_sanitize. The implementation also omits the ANSI-escape strip present in the canonical definition.

  • [code-duplication] scripts/pre-code-jira.src.sh:129 — Pre-commit tool resolution (lines ~129–182) is a verbatim copy from pre-code.src.sh. Extracting to a shared library would follow the established lib/*.lib.sh pattern.

  • [source-ordering] scripts/pre-code-jira.src.sh:127code-ops.lib.sh is sourced at line 127, just before the pre-commit tool section. In pre-code.src.sh, it is sourced near the top. Functionally correct but diverges from convention.

  • [style-inconsistency] scripts/post-code.src.sh:753ISSUE_REF_KEYWORD kept as an intermediary in production code but eliminated in the test helper build_pr_body. Both produce the same output, but the structural divergence reduces the test’s value as a production mirror.

  • [missing-documentation] docs/code.md:48CODE_ALLOWED_TARGET_BRANCHES description updated to remove the env.runner reference but does not explain how users should now set it.


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 (4)

Review

Findings

Medium

  • [protected-path] harness/code.yaml, scripts/pre-code-jira-test.sh, scripts/pre-code-jira.sh, scripts/pre-code-jira.src.sh, skills/code-implementation/SKILL.md — 5 of 8 changed files are under protected paths (harness/, scripts/, skills/). The PR links to issue code agent: read Jira source issues through an event overlay #1074 and the description explains the rationale for modifying governance and infrastructure files. Human approval is always required for protected-path changes.

Low

  • [logic-error] skills/code-implementation/SKILL.md:224 — The SKILL.md fallback path calls gh issue view "${ISSUE_NUMBER}" when FULLSEND_TRACKER=jira but .issue-context.json is missing. For Jira-sourced issues, ISSUE_NUMBER is the numeric portion of the Jira key (e.g., 42 from PROJ-42). Calling gh issue view 42 would hit an unrelated GitHub issue. The only path to a missing file at sandbox runtime is a silent host_files copy failure (optional: true), an infrastructure edge case.

  • [missing-validation] scripts/pre-code-jira.src.sh:28 — The script header documents REPO_FULL_NAME as a required environment variable, but it is not validated with a :? guard in the required-var block (lines 28–31). In practice, REPO_FULL_NAME is not directly consumed by any code path in the Jira pre-script (the pre-commit section uses forge_get_repo_dir() which falls back to workspace paths), so the impact is limited to documentation accuracy. Adding the guard would match the base pre-code.src.sh convention.

  • [Host allowlist scope] scripts/pre-code-jira.src.sh:47 — The Jira host allowlist (*.atlassian.net) permits any subdomain. The JIRA_BASE_URL cross-check (lines 56–64) mitigates credential exfiltration when JIRA_BASE_URL is set, but when it is unset (documented as optional), the cross-check is skipped and the base URL is derived from ISSUE_URL. The attack requires control over the event payload that sets FULLSEND_WORK_ITEM_URL, which is an elevated prerequisite. Consider making JIRA_BASE_URL required so the cross-check always runs.

  • [GHA-workflow-command-injection] scripts/pre-code-jira.src.sh:72 — The ::notice:: annotation on line 72 interpolates ${ISSUE_URL}, ${PROJECT_KEY}, and ${ISSUE_KEY} without _sanitize_gha(). The ::error:: on line 88 also interpolates ${ISSUE_KEY} unsanitized. All values are derived from regex-validated ISSUE_URL (line 42), constraining the character set to alphanumerics, dots, hyphens, and forward slashes — preventing practical injection. Defense-in-depth sanitization recommended for consistency with other sites in the same file.

  • [naming-inconsistency] scripts/pre-code-jira.src.sh:33 — The helper function is named _sanitize_gha but the codebase convention is _gha_sanitize (used in triage-ops.lib.sh, retro-ops.lib.sh, review-ops.lib.sh, scribe-ops.lib.sh, prioritize-ops.lib.sh, and gitlab-host-validation.lib.sh). The implementation also omits the ANSI-escape strip present in the canonical definition.

  • [code-duplication] scripts/pre-code-jira.src.sh:129 — Pre-commit tool resolution (lines ~129–182) is a verbatim copy from pre-code.src.sh. Extracting to a shared lib/precommit-tools.lib.sh would follow the established pattern (lib/prescript-output.lib.sh, lib/code-ops.lib.sh) and prevent future drift between the two copies.

  • [source-ordering] scripts/pre-code-jira.src.sh:127code-ops.lib.sh is sourced at line 127, just before the pre-commit tool section. In pre-code.src.sh, it is sourced near the top. The current ordering is functionally correct since the Jira-specific validation section does not use forge_* helpers, but diverges from the established convention.

  • [edge-case] scripts/pre-code-jira.src.sh:107 — The Jira pre-script omits the existing-PR check present in the forge pre-script. This is documented as intentional (lines 107–120): forge_list_prs_for_issue searches for closing keywords referencing forge issue numbers (#N), but Jira-sourced PRs use Jira keys (PROJ-42), so the check would produce false negatives.

  • [architectural-coherence] harness/code.yaml:138event.source.system == "jira" is a novel CEL expression in this repo (all existing overlays use runtime.forge). This is intentionally different from triage's runtime.forge == "jira" approach — for code, the forge remains GitHub/GitLab while Jira is only the work-item source. Confirm the fullsend runtime supports this CEL namespace.

Previous run (5)

Review

Findings

Medium

  • [protected-path] harness/code.yaml, scripts/pre-code-jira-test.sh, scripts/pre-code-jira.sh, scripts/pre-code-jira.src.sh, skills/code-implementation/SKILL.md — 5 of 8 changed files are under protected paths (harness/, scripts/, skills/). The PR links to issue code agent: read Jira source issues through an event overlay #1074 and the description explains the rationale for modifying governance and infrastructure files. Human approval is always required for protected-path changes.

Low

  • [logic-error] skills/code-implementation/SKILL.md:214 — The SKILL.md fallback path calls gh issue view "${ISSUE_NUMBER}" when FULLSEND_TRACKER=jira but .issue-context.json is missing. For Jira-sourced issues, this forge API call would fail or hit the wrong issue. The only path to a missing file at sandbox runtime is a silent host_files copy failure (optional: true), an infrastructure edge case. Consider logging a hard error instead of falling back to forge API when the tracker is Jira.

  • [edge-case] scripts/pre-code-jira.src.sh:98 — The Jira pre-script omits the existing-PR check present in the forge pre-script. This is documented as intentional (lines 98–116): forge_list_prs_for_issue searches for closing keywords referencing forge issue numbers (#N), but Jira-sourced PRs use Jira keys (PROJ-42), so the check would produce false negatives.

  • [naming-inconsistency] scripts/pre-code-jira.src.sh:33 — The helper function is named _sanitize_gha but the codebase convention is _gha_sanitize (used in triage-ops.lib.sh, retro-ops.lib.sh, review-ops.lib.sh, scribe-ops.lib.sh, prioritize-ops.lib.sh, and gitlab-host-validation.lib.sh). The implementation also omits the ANSI-escape strip present in the canonical definition.

  • [code-duplication] scripts/pre-code-jira.src.sh:117 — Pre-commit tool resolution (lines ~117–174) is a verbatim copy from pre-code.src.sh. Extracting to a shared lib/precommit-tools.lib.sh would follow the established pattern (lib/prescript-output.lib.sh, lib/code-ops.lib.sh) and prevent future drift between the two copies.

  • [source-ordering] scripts/pre-code-jira.src.sh:127code-ops.lib.sh is sourced at line 127, just before the pre-commit tool section. In pre-code.src.sh, it is sourced near the top. The current ordering is functionally correct since the Jira-specific validation section does not use forge_* helpers, but diverges from the established convention.

  • [architectural-coherence] harness/code.yamlevent.source.system == "jira" is a novel CEL expression in this repo (all existing overlays use runtime.forge). This is intentionally different from triage's runtime.forge == "jira" approach — for code, the forge remains GitHub/GitLab while Jira is only the work-item source. Confirm the fullsend runtime supports this CEL namespace.

Previous run (6)

Review

Findings

Medium

  • [env-variable-conflict] harness/code.yaml — When both the Jira overlay (event.source.system == "jira") and a forge overlay (runtime.forge == "github") match, both set ISSUE_URL in the runner env block — the Jira overlay sets it to "${FULLSEND_WORK_ITEM_URL}" and the GitHub overlay sets it to "${GITHUB_ISSUE_URL}". Because the GitHub overlay appears later in the overlays: array, its ISSUE_URL value would win under last-writer-wins merge semantics. For Jira-sourced events, GITHUB_ISSUE_URL is likely unset/empty, so ISSUE_URL becomes empty, causing the Jira pre-script to fail at ${ISSUE_URL:?ISSUE_URL must be set}. The triage harness avoids this by using runtime.forge == "jira" as a mutually exclusive condition; the code harness uses event.source.system == "jira" which co-fires with the forge overlay.
    Remediation: Either remove ISSUE_URL from the forge overlays’ runner env (it’s already set by the Jira overlay for Jira flows), rename the Jira overlay’s key to avoid collision, reorder the Jira overlay after the forge overlays, or document the overlay merge semantics.

  • [protected-path] harness/code.yaml, scripts/pre-code-jira-test.sh, scripts/pre-code-jira.sh, scripts/pre-code-jira.src.sh, skills/code-implementation/SKILL.md — 5 of 7 changed files are under protected paths (harness/, scripts/, skills/). The PR links to issue code agent: read Jira source issues through an event overlay #1074 and the description explains the rationale for modifying governance and infrastructure files. Human approval is always required for protected-path changes.

Low

  • [GHA-workflow-command-injection] scripts/pre-code-jira.src.sh:35ISSUE_URL is echoed unsanitized in a GHA ::error:: workflow command when the regex validation fails. If ISSUE_URL (sourced from ${FULLSEND_WORK_ITEM_URL}) contains an embedded newline followed by a GHA workflow command, echo emits the injected command on a separate line. The risk is low because ISSUE_URL comes from the fullsend platform’s normalized event (not raw user input), and the same unsanitized pattern exists in pre-code.src.sh.

  • [GHA-workflow-command-injection] scripts/pre-code-jira.src.sh:52JIRA_BASE_URL is echoed unsanitized in a GHA ::error:: workflow command when the cross-check fails. JIRA_BASE_URL is a CI-configured variable, not arbitrary user input. The trailing-slash stripping loop does not remove newlines, but the comparison context limits exploitability.

  • [logic-error] skills/code-implementation/SKILL.md:214 — The SKILL.md code example falls through to forge API calls (gh issue view) when FULLSEND_TRACKER=jira but the context file /sandbox/workspace/.issue-context.json is missing. The pre-script validates the context file before exiting 0, so the sandbox only starts when the file exists on the runner — the only path to a missing file at sandbox runtime is a silent host_files copy failure (optional: true), an infrastructure edge case. Consider logging a warning in the else branch when FULLSEND_TRACKER is jira to surface the unexpected state.

  • [edge-case] scripts/pre-code-jira.src.sh:98 — The Jira pre-script omits the existing-PR check present in the forge pre-script. This is documented as intentional (lines 98–113): forge_list_prs_for_issue searches for closing keywords referencing forge issue numbers (#N), but Jira-sourced PRs use Jira keys (PROJ-42), so the check would produce false negatives. A Jira-aware check requires cross-system linking out of scope for the initial overlay.

  • [code-duplication] scripts/pre-code-jira.src.sh:117 — Pre-commit tool resolution (lines ~117–174) is a verbatim copy from pre-code.src.sh. Extracting to a shared lib/precommit-tools.lib.sh would follow the established pattern (lib/prescript-output.lib.sh, lib/code-ops.lib.sh) and prevent future drift between the two copies.

  • [missing-doc] LOCAL.mdLOCAL.md has a “Testing triage with Jira” section but no corresponding section for testing the code agent with Jira-sourced issues. Users testing the Jira overlay locally would lack guidance on required env vars (FULLSEND_WORK_ITEM_URL, JIRA_USER_EMAIL, JIRA_TOKEN, JIRA_BASE_URL, FULLSEND_TRACKER) and overlay composition.


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 (7)

Review

Findings

Medium

  • [protected-path] harness/code.yaml — 5 of 7 changed files are under protected paths (harness/, scripts/, skills/): harness/code.yaml, scripts/pre-code-jira-test.sh, scripts/pre-code-jira.sh, scripts/pre-code-jira.src.sh, skills/code-implementation/SKILL.md. The PR links to issue code agent: read Jira source issues through an event overlay #1074 and the description explains the rationale for modifying governance and infrastructure files. Human approval is always required for protected-path changes.

Low

  • [logic-error] skills/code-implementation/SKILL.md:214 — The SKILL.md code example falls through to forge API calls (gh issue view) when FULLSEND_SOURCE_TRACKER=jira but the context file /sandbox/workspace/.issue-context.json is missing. The pre-script validates the context file before exiting 0, so the sandbox only starts when the file exists on the runner — the only path to a missing file at sandbox runtime is a silent host_files copy failure (optional: true), an infrastructure edge case. Consider logging a warning in the else branch when FULLSEND_SOURCE_TRACKER is jira to surface the unexpected state.

  • [edge-case] scripts/pre-code-jira.src.sh:98 — The Jira pre-script omits the existing-PR check present in the forge pre-script. This is documented as intentional (lines 98–113): forge_list_prs_for_issue searches for closing keywords referencing forge issue numbers (#N), but Jira-sourced PRs use Jira keys (PROJ-42), so the check would produce false negatives. A Jira-aware check requires cross-system linking out of scope for the initial overlay.

  • [code-duplication] scripts/pre-code-jira.src.sh:117 — Pre-commit tool resolution (lines ~117–174) is a verbatim copy from pre-code.src.sh. Extracting to a shared lib/precommit-tools.lib.sh would follow the established pattern (lib/prescript-output.lib.sh, lib/code-ops.lib.sh) and prevent future drift between the two copies.

  • [missing-doc] LOCAL.mdLOCAL.md has a "Testing triage with Jira" section but no corresponding section for testing the code agent with Jira-sourced issues. Users testing the Jira overlay locally would lack guidance on required env vars and overlay composition.

Previous run (8)

Review

Findings

High

  • [stale-doc] docs/code.md:171 — The "Multi-forge support" section describes the code harness as using forge.<platform> sections to configure platform-specific policies, skills, and env vars. This PR replaces the forge: block with overlays: entries using CEL when: expressions, making this section factually incorrect and misleading for users trying to understand or customize the code harness.
    Remediation: Update the section to reflect overlays: with when: expressions and mention the Jira-source overlay.

Medium

  • [edge-case] scripts/pre-code-jira.src.sh — The base pre-script (pre-code.sh) checks whether a human already has an open PR linked to the issue and skips the agent run to prevent stepping on human work. The Jira pre-script replaces it entirely via the overlay pre_script override but omits this check, so Jira-sourced runs will proceed even when a human PR already addresses the issue.
    Remediation: Add an existing-PR check using forge_list_prs_for_issue, or document why this check is intentionally omitted for Jira-sourced flows.

  • [stale-doc] docs/code.md:49FULLSEND_FORGE description says "Set automatically by the harness forge.<platform>.env section." After this PR, it is set via overlays: entries with when: conditions.
    Remediation: Update to reference overlay env blocks.

  • [missing-doc] docs/code.mdFULLSEND_SOURCE_TRACKER is introduced as a new env var in both runner and sandbox but is not documented in the Variables table.
    Remediation: Add FULLSEND_SOURCE_TRACKER to the Variables table.

  • [protected-path] harness/code.yaml — 5 of 6 changed files are under protected paths (harness/, scripts/, skills/): harness/code.yaml, scripts/pre-code-jira-test.sh, scripts/pre-code-jira.sh, scripts/pre-code-jira.src.sh, skills/code-implementation/SKILL.md. The PR links to issue code agent: read Jira source issues through an event overlay #1074 and the description explains the rationale. Human approval is always required for protected-path changes.

Low

  • [logic-error] harness/code.yaml — The Jira overlay does not set ISSUE_NUMBER in its sandbox env block; it relies on the companion forge overlay to supply it via merge-all-matching. See also: [scope-boundary] skills/code-implementation/SKILL.md:230 says "use the ISSUE_NUMBER environment variable (set by the harness)" — wording is imprecise for the Jira flow since the Jira overlay itself does not set it.

  • [test-inadequate] scripts/pre-code-jira-test.sh — The empty-issue-context-rejected test passes fullsend_exit=1, which causes the script to exit at the CLI-failure branch (line 78) before reaching the -s guard (line 85). The empty-file validation path has no test coverage.

  • [documentation-accuracy] scripts/pre-code-jira.src.sh:15 — Header lists JIRA_BASE_URL as a required environment variable, but code treats it as optional with ${JIRA_BASE_URL:-} and derives it from ISSUE_URL when unset.

  • [code-duplication] scripts/pre-code-jira.src.sh:99 — Pre-commit tool resolution (lines 99–156) is a verbatim copy from pre-code.src.sh. Extracting to a shared lib/*.lib.sh would match the established pattern.

  • [code-organization] harness/code.yaml — Jira overlay ordering (before forge overlays) is load-bearing for pre_script override semantics. Documented in a YAML comment but the dependency is implicit.

  • [naming-convention] scripts/pre-code-jira.src.sh — Notice annotation uses emoji-free format; sibling pre-code.src.sh uses emoji prefix in its ::notice:: annotations.

  • [error-handling-idiom] scripts/pre-code-jira.src.sh — Uses ${VAR:?} for required-variable checks; sibling pre-code.src.sh uses an accumulate-and-report pattern that surfaces all missing variables at once.


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.

@fullsend-ai-coder

fullsend-ai-coder Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor Author

🤖 Finished Fix · ✅ Success · Started 10:55 PM UTC · Completed 11:04 PM UTC

Commit: affef07 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $3.57

- Update docs/code.md: Multi-forge section now describes overlays with
  CEL when: expressions instead of stale forge.<platform> references;
  FULLSEND_FORGE description references overlay env section; add
  FULLSEND_SOURCE_TRACKER to Variables table; document Jira-source overlay
- Document intentional omission of existing-PR check in Jira pre-script
  (forge_list_prs_for_issue searches for #N closing keywords which do not
  apply to Jira issue keys)
- Add ISSUE_NUMBER to Jira overlay sandbox env block (explicit dependency
  instead of relying on forge overlay merge)
- Fix empty-issue-context test to actually exercise the empty-file guard
  (previously exited at CLI-failure branch before reaching -s check)
- Correct JIRA_BASE_URL documentation: optional, not required
- Add emoji prefix to Jira notice annotation for consistency
- Clarify SKILL.md ISSUE_NUMBER wording for Jira flow

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

Copy link
Copy Markdown
Contributor Author

🔧 Fix agent — iteration 1 (bot-triggered)

Addressed 9 of 13 review findings: updated stale documentation in docs/code.md (Multi-forge section, FULLSEND_FORGE description, added FULLSEND_SOURCE_TRACKER), documented intentional omission of existing-PR check for Jira flows, added ISSUE_NUMBER to Jira overlay sandbox env, fixed empty-context test coverage, corrected JIRA_BASE_URL documentation accuracy, added emoji prefix for naming consistency, and clarified SKILL.md wording. Disagreed with 4 findings: code-duplication (better as follow-up), code-organization (already documented), error-handling-idiom (style preference), and protected-path (informational).

Fixed (9):

  1. stale-doc: Multi-forge support section describes forge. (docs/code.md): Updated Multi-forge section to describe overlays with CEL when: expressions, mentioned Jira-source overlay, and corrected FULLSEND_FORGE description to reference overlay env section
  2. missing-doc: FULLSEND_SOURCE_TRACKER not documented (docs/code.md): Added FULLSEND_SOURCE_TRACKER to the Variables table with description, default, and valid values
  3. stale-doc: FULLSEND_FORGE description references forge..env (docs/code.md): Updated FULLSEND_FORGE description to reference overlay env section matched via when: expression
  4. edge-case: missing existing-PR check in Jira pre-script (scripts/pre-code-jira.src.sh): Documented intentional omission: forge_list_prs_for_issue searches for closing keywords (#N) which do not apply to Jira issue keys (PROJ-42); a Jira-aware check requires cross-system linking not yet available
  5. logic-error: ISSUE_NUMBER not set by Jira overlay (harness/code.yaml): Added ISSUE_NUMBER to Jira overlay sandbox env block, making the dependency explicit instead of relying on forge overlay merge
  6. logic-error: SKILL.md ISSUE_NUMBER wording imprecise for Jira flow (skills/code-implementation/SKILL.md): Changed 'set by the harness' to 'set in the Jira overlay sandbox env block' for precision
  7. test-inadequate: empty-issue-context test does not exercise empty-file guard (scripts/pre-code-jira-test.sh): Replaced test with a custom mock that succeeds (exit 0) but writes no output, so the script reaches the -s empty-file guard instead of exiting at the CLI-failure branch
  8. documentation-accuracy: JIRA_BASE_URL listed as required but treated as optional (scripts/pre-code-jira.src.sh): Moved JIRA_BASE_URL from Required to Optional section in header comment, noting it is derived from ISSUE_URL when unset
  9. naming-convention: notice annotation missing emoji prefix (scripts/pre-code-jira.src.sh): Added emoji prefix to match sibling pre-code.src.sh convention

Disagreed (4):

  1. code-duplication: pre-commit tool resolution is verbatim copy: Extracting to a shared lib/*.lib.sh is a valid improvement but would require modifying pre-code.src.sh (changing an established file not in this PR's scope) and increasing the change surface. Better addressed in a follow-up PR focused on deduplication.
  2. code-organization: Jira overlay ordering is load-bearing and implicit: The ordering dependency is already documented in the YAML comment. Adding explicit enforcement mechanism would require changes to the fullsend runtime, which is beyond this PR's scope.
  3. error-handling-idiom: uses ${VAR:?} instead of accumulate-and-report pattern: The ${VAR:?} pattern is a valid bash idiom that provides clear error messages. For only 3 required variables, the functional difference from accumulate-and-report is minimal. Changing the pattern is a style preference that would diverge from this file's existing idiom without improving correctness.
  4. protected-path: 5 of 6 changed files are under protected paths: Informational finding noting that human approval is required. No code change needed — this is a process requirement already handled by the review workflow.

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

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 11:06 PM UTC · Completed 11:48 PM UTC

Commit: 418d9fa · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high

@fullsend-ai-review
fullsend-ai-review Bot dismissed their stale review August 27, 2026 23:48

Superseded by updated review

fullsend-ai-review[bot]

This comment was marked as outdated.

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

Copy link
Copy Markdown
Member

/fs-fix why FULLSEND_SOURCE_TRACKER and not FULLSEND_TRACKER (like triage)?

@fullsend-ai-coder

fullsend-ai-coder Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

🤖 Finished Fix · ✅ Success · Started 2:28 AM UTC · Completed 2:34 AM UTC

Commit: 418d9fa · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $2.12

Align the Jira-source overlay env var with the established triage
convention (FULLSEND_TRACKER) instead of inventing a new name.

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

Copy link
Copy Markdown
Contributor Author

🔧 Fix agent — iteration 2 (human-triggered)

Renamed FULLSEND_SOURCE_TRACKER to FULLSEND_TRACKER across all 3 files (harness/code.yaml, skills/code-implementation/SKILL.md, docs/code.md) to align with the established triage harness convention. All pre-code-jira tests (12/12) and pre-code tests (33/33) pass.

Fixed (3):

  1. rename FULLSEND_SOURCE_TRACKER to FULLSEND_TRACKER for consistency with triage (harness/code.yaml): Renamed FULLSEND_SOURCE_TRACKER to FULLSEND_TRACKER in both runner and sandbox env blocks of the Jira-source overlay, matching the established FULLSEND_TRACKER convention used by the triage harness
  2. rename FULLSEND_SOURCE_TRACKER to FULLSEND_TRACKER for consistency with triage (skills/code-implementation/SKILL.md): Updated the code-implementation skill to reference FULLSEND_TRACKER instead of FULLSEND_SOURCE_TRACKER in both the prose description and the bash code block
  3. rename FULLSEND_SOURCE_TRACKER to FULLSEND_TRACKER for consistency with triage (docs/code.md): Updated the environment variable table to use FULLSEND_TRACKER and noted it matches the triage convention

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

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 2:36 AM UTC · Completed 2:56 AM UTC

Commit: ea7b2ab · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $9.07

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review fullsend-ai-review Bot removed the requires-manual-review Review requires human judgment label Aug 28, 2026
@fullsend-ai-coder

fullsend-ai-coder Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

🤖 Finished Fix · ✅ Success · Started 2:58 AM UTC · Completed 3:06 AM UTC

Commit: ea7b2ab · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $2.83

- Reorder Jira overlay after forge overlays in harness/code.yaml to fix
  ISSUE_URL env variable conflict under last-writer-wins merge semantics
- Sanitize ISSUE_URL and JIRA_BASE_URL in GHA ::error:: workflow commands
  to prevent command injection via embedded newlines
- Add warning log in SKILL.md when FULLSEND_TRACKER=jira but context file
  is missing, instead of silently falling through to forge APIs
- Add "Testing code agent with Jira" section to LOCAL.md

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

Copy link
Copy Markdown
Contributor Author

🔧 Fix agent — iteration 3 (bot-triggered)

Addressed 5 of 8 review findings. Fixed the medium-severity ISSUE_URL env variable conflict by reordering the Jira overlay after forge overlays. Hardened two GHA workflow command echo sites against injection. Added missing-file warning in SKILL.md and local testing docs. Disagreed with 3 findings: protected-path (informational), edge-case (documented intentional), code-duplication (out of scope).

Fixed (5):

  1. env-variable-conflict: ISSUE_URL collision between Jira and forge overlays (harness/code.yaml): Reordered Jira overlay after forge overlays so its ISSUE_URL wins under last-writer-wins merge semantics. Updated comment to document the ordering rationale.
  2. GHA-workflow-command-injection: ISSUE_URL unsanitized in ::error:: command (scripts/pre-code-jira.src.sh): Added _sanitize_gha function that strips newlines/carriage returns and escapes :: and % to prevent GHA workflow command injection. Applied to ISSUE_URL in the URL validation error message.
  3. GHA-workflow-command-injection: JIRA_BASE_URL unsanitized in ::error:: command (scripts/pre-code-jira.src.sh): Applied _sanitize_gha to JIRA_BASE_URL in the base-URL cross-check error message.
  4. logic-error: SKILL.md falls through to forge APIs when FULLSEND_TRACKER=jira but context file missing (skills/code-implementation/SKILL.md): Added elif branch that logs a ::warning:: when FULLSEND_TRACKER=jira but .issue-context.json is missing, before falling back to forge API calls.
  5. missing-doc: LOCAL.md lacks section for testing code agent with Jira (LOCAL.md): Added 'Testing code agent with Jira' section documenting env vars (FULLSEND_WORK_ITEM_URL, JIRA_USER_EMAIL, JIRA_TOKEN, JIRA_BASE_URL), the FULLSEND_TRACKER=jira signal, and overlay composition behavior.

Disagreed (3):

  1. protected-path: 5 of 7 changed files are under protected paths: Informational finding requiring human approval — no code change is applicable. The PR modifies governance files as documented in the PR description and issue code agent: read Jira source issues through an event overlay #1074.
  2. edge-case: Jira pre-script omits existing-PR check: The reviewer acknowledges this is documented as intentional (lines 101-114 of pre-code-jira.src.sh). forge_list_prs_for_issue searches for forge issue closing keywords (#N) which produce false negatives for Jira keys (PROJ-42). A Jira-aware check requires cross-system linking out of scope for the initial overlay.
  3. code-duplication: pre-commit tool resolution is a verbatim copy from pre-code.src.sh: Extracting to a shared lib/precommit-tools.lib.sh is a significant refactor that changes the bundling pipeline and touches multiple scripts. This is out of scope for this PR — the duplication is documented in the code comments and can be addressed in a follow-up.

Tests: passed

Strategy change: Iteration 3 (at escalation threshold): changed overlay ordering strategy from Jira-first to Jira-last to resolve the ISSUE_URL conflict under last-writer-wins semantics.

Decision points
  • Chose to reorder Jira overlay after forge overlays instead of removing ISSUE_URL from forge overlays or renaming the Jira key (alternatives: Remove ISSUE_URL from forge overlays, Rename Jira ISSUE_URL key, Document merge semantics only; rationale: Reordering is the simplest fix: Jira's ISSUE_URL wins via last-writer-wins, pre_script still overrides the top-level default, and non-Jira flows are unaffected since the Jira overlay doesn't match.)

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

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 3:08 AM UTC · Completed 3:50 AM UTC

Commit: 383e72e · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high

@fullsend-ai-review
fullsend-ai-review Bot dismissed their stale review August 28, 2026 03:50

Superseded by updated review

fullsend-ai-review[bot]

This comment was marked as outdated.

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

fullsend-ai-review Bot commented Aug 29, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 11:49 AM UTC · Completed 12:27 PM UTC

Commit: cec717f · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $7.56

@fullsend-ai-review

fullsend-ai-review Bot commented Aug 29, 2026

Copy link
Copy Markdown

🤖 Review · ⚠️ Cancelled · Started 6:43 PM UTC · Ended 6:46 PM UTC

Commit: b36d89f · View workflow run →

@fullsend-ai-review

fullsend-ai-review Bot commented Aug 29, 2026

Copy link
Copy Markdown

🤖 Review · ⚠️ Cancelled · Started 6:48 PM UTC · Ended 6:50 PM UTC

Commit: 0b2a0b8 · View workflow run →

@fullsend-ai-review

fullsend-ai-review Bot commented Aug 29, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 6:52 PM UTC · Completed 7:13 PM UTC

Commit: baa8497 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $12.21

fullsend-ai-review[bot]

This comment was marked as outdated.

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review fullsend-ai-review Bot removed the requires-manual-review Review requires human judgment label Aug 29, 2026
@fullsend-ai-coder

fullsend-ai-coder Bot commented Aug 29, 2026

Copy link
Copy Markdown
Contributor Author

🤖 Finished Fix · ❌ Failure (post-script /home/runner/work/agents/agents/.fullsend/.fullsend-cache/resources/sha256/e14f2eff53cc1ed5cb9200bf5471721ee531c61b6314e8977d603eae321214cd/scripts/post-fix.sh failed: exit status 1) · Started 7:15 PM UTC · Completed 7:32 PM UTC

Commit: baa8497 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $5.62

@fullsend-ai-coder

Copy link
Copy Markdown
Contributor Author

⚠️ Post-fix script failed — Signed-off-by rejected (exit code 1)

The fix agent completed, but the post-fix script failed before finishing.

Workflow run: https://github.com/fullsend-ai/agents/actions/runs/33270303619

Details:
Agent commit contains a Signed-off-by trailer. Agents must not use 'git commit -s' or append Signed-off-by trailers.
Please check the workflow logs for full details and retry with /fs-fix if appropriate.

@fullsend-ai-review

fullsend-ai-review Bot commented Aug 29, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 7:43 PM UTC · Completed 8:02 PM UTC

Commit: 296bc17 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $10.10

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-coder

fullsend-ai-coder Bot commented Aug 29, 2026

Copy link
Copy Markdown
Contributor Author

🤖 Finished Fix · ✅ Success · Started 8:04 PM UTC · Completed 8:14 PM UTC

Commit: 296bc17 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $3.65

- Restore CODE_ALLOWED_TARGET_BRANCHES passthrough in top-level env.runner
  (env-var-contract-violation)
- Replace FULLSEND_WORK_ITEM_URL with ISSUE_URL in SKILL.md — the sandbox
  only receives ISSUE_URL via the harness mapping (logic-error)
- Use grep -qwF for cross-work-item ownership check to prevent substring
  false positives e.g. PROJ-1 matching PROJ-12 (injection-check-weakening)
- Rename _sanitize_gha to _gha_sanitize and add ANSI-escape strip to match
  canonical definition (naming-inconsistency)
- Apply _gha_sanitize to ::notice:: interpolation for defense-in-depth
  (GHA-workflow-command-injection)
- Add REPO_FULL_NAME :? guard in pre-code-jira.src.sh (missing-validation)
- Remove ISSUE_REF_KEYWORD intermediary to match test helper pattern
  (style-inconsistency)

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

Copy link
Copy Markdown
Contributor Author

🔧 Fix agent — iteration 4 (bot-triggered)

Addressed 7 of 10 review findings. Fixed: CODE_ALLOWED_TARGET_BRANCHES passthrough restoration, SKILL.md variable reference correction (FULLSEND_WORK_ITEM_URL to ISSUE_URL), cross-work-item injection substring match hardening, _sanitize_gha to _gha_sanitize rename with ANSI strip, GHA workflow command injection defense-in-depth, REPO_FULL_NAME validation guard, and ISSUE_REF_KEYWORD style inconsistency. Disagreed with 3: protected-path (informational), code-duplication (out of PR scope), edge-case (intentionally documented).

Fixed (7):

  1. env-var-contract-violation: CODE_ALLOWED_TARGET_BRANCHES removed from top-level env.runner (harness/code.yaml): Restored CODE_ALLOWED_TARGET_BRANCHES passthrough in top-level env.runner block, matching AGENTS.md section 8 canonical example
  2. logic-error: SKILL.md references FULLSEND_WORK_ITEM_URL which is not in env.sandbox (skills/code-implementation/SKILL.md): Replaced all 3 FULLSEND_WORK_ITEM_URL references with ISSUE_URL, matching the sandbox env mapping in harness/code.yaml
  3. Cross-work-item injection check weakening via grep -qF substring match (scripts/post-code.src.sh): Changed grep -qF to grep -qwF (word-boundary match) so PROJ-1 URL does not match PROJ-12 URL
  4. naming-inconsistency: _sanitize_gha should be _gha_sanitize with ANSI strip (scripts/pre-code-jira.src.sh): Renamed _sanitize_gha to _gha_sanitize and added ANSI-escape strip to match canonical definition in triage-ops.lib.sh
  5. GHA-workflow-command-injection: ::notice:: interpolates values without sanitization (scripts/pre-code-jira.src.sh): Applied _gha_sanitize() to ISSUE_URL, PROJECT_KEY, and ISSUE_KEY in ::notice:: annotation for defense-in-depth
  6. missing-validation: REPO_FULL_NAME not validated with :? guard (scripts/pre-code-jira.src.sh): Added REPO_FULL_NAME :? guard to match documentation and base pre-code.src.sh convention
  7. style-inconsistency: ISSUE_REF_KEYWORD intermediary in production but not in test helper (scripts/post-code.src.sh): Removed ISSUE_REF_KEYWORD intermediary variable, building ISSUE_REFERENCE directly to match the test helper build_pr_body pattern

Disagreed (3):

  1. protected-path: 11 of 14 changed files under protected paths: Informational finding requiring human approval. No code change needed — the PR description explains the rationale for modifying governance files.
  2. code-duplication: pre-commit tool resolution is a verbatim copy from pre-code.src.sh: Extracting to scripts/lib/precommit-resolve.lib.sh requires modifying pre-code.src.sh, which is not part of this PR changed files. The extraction would be scope creep; best addressed in a follow-up PR to avoid unrelated changes.
  3. edge-case: Jira pre-script omits existing-PR check: Already documented as intentional in the script (lines 110-122). forge_list_prs_for_issue searches for closing keywords (#N) which produce false negatives for Jira keys (PROJ-42). A Jira-aware check requires cross-system linking out of scope for the initial overlay.

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-coder fullsend-ai-coder Bot added the needs-human Agent loop needs human intervention label Aug 29, 2026
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 29, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 8:16 PM UTC · Completed 8:36 PM UTC

Commit: 5a18c25 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $8.72

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-coder

fullsend-ai-coder Bot commented Aug 29, 2026

Copy link
Copy Markdown
Contributor Author

🤖 Finished Fix · ✅ Success · Started 8:38 PM UTC · Completed 8:46 PM UTC

Commit: 5a18c25 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $2.32

docs/code.md described the code agent deriving the work-item key from
FULLSEND_WORK_ITEM_URL, but that variable is runner-only — the Jira
overlay maps it to ISSUE_URL in env.sandbox. The prior iteration fixed
SKILL.md; this fixes the remaining reference in the design doc.

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

Copy link
Copy Markdown
Contributor Author

🔧 Fix agent — iteration 5 (bot-triggered)

Fixed the last remaining FULLSEND_WORK_ITEM_URL reference in sandbox-agent documentation (docs/code.md). SKILL.md was already corrected in prior iterations. The docs now consistently use ISSUE_URL, matching the harness overlay's env.sandbox mapping.

Fixed (1):

  1. FULLSEND_WORK_ITEM_URL referenced instead of ISSUE_URL: SKILL.md was already fixed in prior iterations. The remaining instance was in docs/code.md line 192, which described the code agent (sandbox) deriving the key from FULLSEND_WORK_ITEM_URL — a runner-only variable. Changed to ISSUE_URL to match the sandbox env mapping in harness/code.yaml (ISSUE_URL: "${FULLSEND_WORK_ITEM_URL}").

Tests: passed

Strategy change: Prior iterations fixed SKILL.md but docs/code.md still had the same stale variable reference. This iteration targets docs/code.md instead.

Decision points
  • SKILL.md already uses ISSUE_URL (fixed in prior iterations); identified docs/code.md as the remaining file with the stale reference (alternatives: Disagree with finding since SKILL.md is already correct, Fix docs/code.md which has the same conceptual error; rationale: The reviewer's concern is valid — sandbox-facing documentation should reference ISSUE_URL, not the runner-only FULLSEND_WORK_ITEM_URL. docs/code.md had the last remaining instance of this mismatch.)

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

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 8:48 PM UTC · Completed 9:30 PM UTC

Commit: ab6e33c · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high

@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 review comments could not be posted on the diff (GitHub returned 422) and are included here instead:

  • scripts/post-code.src.sh (file-level): Line 233 · [high] logic-error

Jira+GitLab composition will fail at runtime. When FULLSEND_FORGE=gitlab and event.source.system=jira, the Jira overlay sets ISSUE_URL to the Jira browse URL. The GitLab validation block calls forge_validate_issue_url on ISSUE_URL, which checks against the GitLab URL pattern. The Jira URL will never match, causing exit 1. Additionally, GITLAB_HOST derivation from ISSUE_URL would extract the Jira host instead of the GitLab host, breaking all subsequent GitLab API calls.

Suggested fix: When EXTERNAL_WORK_ITEM=true and FULLSEND_FORGE=gitlab, skip forge_validate_issue_url on ISSUE_URL and derive GITLAB_HOST from CI_SERVER_HOST instead.

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

# Auto-detect and install pre-commit tool dependencies
# ---------------------------------------------------------------------------
# This section is shared with the forge pre-script — the target repo is
# the same regardless of whether the source issue is Jira or a forge.

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] code-duplication

Pre-commit tool resolution block (lines 129-183) is a verbatim copy from pre-code.src.sh. The codebase has an established lib/*.lib.sh pattern.

Suggested fix: Extract to scripts/lib/precommit-resolve.lib.sh and source from both scripts.

echo "::error::Jira issue context is not valid JSON"
exit 1
fi

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] edge-case

The Jira pre-script omits the existing-PR check present in the forge pre-script. Documented as intentional: forge_list_prs_for_issue would produce false negatives for Jira keys (PROJ-42 vs #N).

Comment thread scripts/post-code.src.sh
if pr_body_refs_issue "${PR_BODY_TEXT}" "${ISSUE_NUMBER}"; then
if [ "${EXTERNAL_WORK_ITEM}" = "true" ]; then
if printf '%s\n' "${PR_BODY_TEXT}" | grep -qwF -- "${WORK_ITEM_URL}"; then
PR_CLOSES_THIS_ISSUE=true

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] Cross-work-item injection check weakening

The external-work-item PR ownership check uses grep -qwF. The -w flag mitigates substring false-positives. Primary defense (branch namespace) remains unaffected.

@fullsend-ai-coder

fullsend-ai-coder Bot commented Aug 29, 2026

Copy link
Copy Markdown
Contributor Author

🤖 Finished Fix · ❌ Failure (running pre-script: exit status 1) · Started 9:32 PM UTC · Completed 9:32 PM UTC

Commit: ab6e33c · View workflow run →

Effort: high

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

Labels

needs-human Agent loop needs human intervention ready-for-review risk/elevated PR risk: elevated

Projects

None yet

Development

Successfully merging this pull request may close these issues.

code agent: read Jira source issues through an event overlay

1 participant