refactor(#1108): use OpenShell providers for Jira credentials - #1109
Conversation
|
🤖 Finished Review · ✅ Success · Started 10:04 PM UTC · Completed 10:26 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high |
|
Risk Assessment: elevated (3/5) DetailsElevated from prior score (2 to 3): LINES_CHANGED crossed the 800 threshold (845), large blast radius, 9 protected paths. High Tier 2 churn with fix/revert density. Tier 3 shows strong issue-PR alignment with 8 clear acceptance criteria. Weighted composite 3.00 rounds to 3. Previous runRisk Assessment: moderate (2/5) DetailsAnchored to prior score (2). Tier 1 signals unchanged — same 14 files, 10 protected paths, bot author, no dependency or CI changes. Tier 2 churn and fix/revert density remain high but were already accounted for in prior assessment. Tier 3 shows good issue-PR alignment with clear acceptance criteria. Weighted composite 2.43 rounds to 2. Previous run (2)Risk Assessment: moderate (2/5) DetailsAnchored to prior score (2). Tier 1 signals unchanged — same 11 files, 7 protected paths, bot author, no dependency or CI changes. Tier 2 churn and fix/revert density remain high but were already accounted for in prior assessment. Tier 3 shows good issue-PR alignment with most acceptance criteria covered. Previous run (3)Risk Assessment: moderate (2/5) DetailsAnchored to prior score (2). Tier 1 signals unchanged from prior assessment — same 11 files, 7 protected paths, bot author, no dependencies. High Tier 2 churn and regression history offset by very recent code age and low revert/sentiment signals. Tier 3 shows good issue-PR alignment. Weighted composite 2.25 rounds to 2. Previous run (4)Risk Assessment: moderate (2/5) DetailsAnchored to prior assessment (score 2). Only LOCAL.md documentation changed since last review; Tier 1 signals unchanged — same file count (11), same protected-path count (7), same bot author, same dependency status. No articulable reason to change the score. Previous run (5)Risk Assessment: moderate (2/5) DetailsModerate file count (10) with 7 protected-path changes (harness/, scripts/, skills/), but bot author, same-day triaged issue, clear acceptance criteria coverage, and security-focused credential boundary refactor mitigate risk. |
ReviewFindingsMedium
Low
Next steps:
Previous runReviewFindingsMedium
Low
Previous run (2)ReviewFindingsHigh
Medium
Low
Next steps:
Previous run (3)ReviewFindingsMedium
Low
Next steps:
Previous run (4)ReviewFindingsMedium
Low
Previous run (5)ReviewFindingsMedium
Low
Next steps:
|
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
🤖 Finished Fix · ✅ Success · Started 10:28 PM UTC · Completed 10:34 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $1.61 |
🔧 Fix agent — iteration 1 (bot-triggered)Fixed two stale references in LOCAL.md: updated the env var comment (line 165) to clarify which vars enter the sandbox, and replaced the prefetch description (lines 178–181) with the new provider-backed API access pattern. Disagreed with three informational findings: protected-path process note (no code fix needed), defense-in-depth observation (intentional design tradeoff), and credential-exposure note (mitigated by profile enforcement). Fixed (2):
Disagreed (3):
Tests: passedNext steps:
|
Update two stale references in LOCAL.md that still described the removed runner-side Jira prefetch pattern: - Line 165: Clarify that JIRA_USER_EMAIL and JIRA_BASE_URL now enter the code sandbox as non-secret config, while JIRA_TOKEN stays on the runner for post-script mutations. - Lines 178–181: Replace the fullsend CLI / host_files / .issue-context.json description with the new provider-backed API access pattern. Addresses #1109
|
🤖 Finished Review · ✅ Success · Started 10:35 PM UTC · Completed 10:54 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $5.55 |
Superseded by updated review
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
fullsend-ai-review
Bot
added
the
requires-manual-review
Converge both triage and code agents on provider-backed Jira API access so the real JIRA_TOKEN never enters either sandbox. The jira-ro provider injects an opaque placeholder at the HTTP layer; sandboxed curl calls authenticate through the provider boundary instead of using the real secret. Triage harness: remove JIRA_TOKEN from env.sandbox and env/jira/triage.env. The provider (already attached) supplies the placeholder. Runner-side post-scripts retain the real token via env.runner for trusted mutations. Code harness: attach providers/jira-ro.yaml and the fullsend-jira-ro OpenShell profile. Remove the runner-side .issue-context.json prefetch — the sandbox reads Jira work items directly via the provider boundary. Remove JIRA_ISSUE_CONTEXT_FILE, JIRA_USER_EMAIL, and JIRA_TOKEN from env.runner (no longer needed). Add JIRA_USER_EMAIL and JIRA_BASE_URL to env.sandbox (non-secret config for API calls). Pre-script: strip credential requirements and issue-fetch logic from pre-code-jira.src.sh, keeping URL validation and pre-commit tool installation. Skill: update code-implementation to query Jira directly via curl instead of reading .issue-context.json. Tests: add harness-jira-test.sh verifying provider/profile selection and JIRA_TOKEN absence from sandbox config. Update pre-code-jira-test.sh for the simplified pre-script. Note: shellcheck could not run (not available in sandbox). pre-commit hooks could not fetch remote repos (network policy); ran equivalent checks directly. Pre-existing post-retro-test.sh failures (16) are unrelated to this change. Closes #1108
Update two stale references in LOCAL.md that still described the removed runner-side Jira prefetch pattern: - Line 165: Clarify that JIRA_USER_EMAIL and JIRA_BASE_URL now enter the code sandbox as non-secret config, while JIRA_TOKEN stays on the runner for post-script mutations. - Lines 178–181: Replace the fullsend CLI / host_files / .issue-context.json description with the new provider-backed API access pattern. Addresses #1109
9d9e033 to
9007670
Compare
|
🤖 Finished Review · ✅ Success · Started 9:05 PM UTC · Completed 9:25 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $7.72 |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
fullsend-ai-review
Bot
removed
the
requires-manual-review
|
🤖 Finished Fix · ✅ Success · Started 9:26 PM UTC · Completed 9:33 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $1.81 |
- Update LOCAL.md comment to clarify JIRA_TOKEN is read by the jira-ro provider on the host, not consumed by the code agent's runner or sandbox environment (stale-reference finding) - Add JIRA_USER_EMAIL and JIRA_TOKEN to base_env() in pre-code-jira-test.sh so no-jira-email-still-succeeds and no-jira-token-still-succeeds tests are no longer redundant with valid-jira-github-succeeds (test-inadequate finding) - Refactor five near-identical Python-in-bash helpers in harness-jira-test.sh into a single parameterized jira_overlay_field() helper, reducing ~80 lines of repetition (code-organization finding) Addresses #1109
🔧 Fix agent — iteration 2 (bot-triggered)Fixed 3 findings: corrected stale LOCAL.md comment about JIRA_TOKEN scope, made redundant pre-code-jira tests meaningful by setting vars in base_env(), and refactored 5 near-identical harness test helpers into a single parameterized function. Recorded reasoned disagreements for 5 informational findings the reviewer acknowledged as acceptable. Fixed (3):
Disagreed (5):
Tests: passed Decision points
Next steps:
|
|
🤖 Finished Review · ✅ Success · Started 9:35 PM UTC · Completed 9:56 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $8.65 |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
🤖 Finished Fix · ✅ Success · Started 9:58 PM UTC · Completed 10:06 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $2.12 |
The jira-ro provider handles authentication at the network layer via
enforcement: enforce, so --user "${JIRA_USER_EMAIL}:${JIRA_TOKEN}" is
unnecessary. With JIRA_TOKEN removed from sandbox env in this PR, the
undefined variable created a functional gap in four skill files.
Remove --user flags from all curl examples in:
- skills/jira-forge/SKILL.md (8 occurrences)
- skills/issue-labels/jira/SKILL.md (2 occurrences)
- skills/jira-components/SKILL.md (2 occurrences)
- skills/code-implementation/SKILL.md (1 occurrence)
Update jira-forge/SKILL.md introductory text to document provider-backed
auth instead of referencing JIRA_USER_EMAIL/JIRA_TOKEN for Basic auth.
Addresses #1109
Add JIRA_TOKEN (alongside OPENAI_API_KEY) to a sandbox expansion
denylist enforced by a new test that scans all harness YAML files for
two leak paths:
1. env.sandbox values — catches both direct keys and aliases
(e.g. LEAK: "${JIRA_TOKEN}") that would expand the real
credential into the sandbox under a different name.
2. host_files with expand: true — catches expanded env files
whose source references a denylisted variable.
Also wires harness-jira-test.sh and the new denylist test into the
Makefile script-test target so CI runs them on every push.
Addresses #1109
|
I tried this end-to-end against production Jira from ralphbean/jira-triage-test. With the harness pinned at I then pinned the same test to current head It looks like the provider can substitute the opaque token when curl constructs Basic auth, but does not originate the complete Jira Authorization header for a credential-free request. Should we keep |
🔧 Fix agent — iteration 4 (human-triggered)Added sandbox credential expansion denylist enforcement via a new test script that scans all harness YAML files for JIRA_TOKEN and OPENAI_API_KEY references in two paths: (1) env.sandbox values including aliases, and (2) host_files with expand: true. Both harness-jira-test.sh and the new denylist test are now wired into the Makefile script-test target. All 28 denylist checks pass across 7 harness files. Fixed (3):
Tests: passed
Decision points
Next steps:
|
|
🤖 Review · Commit: |
|
I dug into whether the profile could synthesize the header for us. I do not think The Jira credential is an Atlassian API token used against OpenShell does support So, I think the practical fix here is to restore References: OpenShell Providers v2, OpenShell provider injection locations, and Atlassian Basic auth. |
|
/fs-fix The production experiment shows that a credential-free Jira request is anonymous even when Please restore Please also update the accompanying documentation and tests so they assert this distinction: the real token is never expanded into sandbox config or files, while sandbox requests still carry the opaque provider placeholder through Basic auth. Do not switch the profile to bearer auth, since that would require an OAuth token and the |
|
🤖 Finished Fix · ✅ Success · Started 2:17 AM UTC · Completed 2:24 AM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $2.44 |
Restore `--user "${JIRA_USER_EMAIL}:${JIRA_TOKEN}"` in every sandbox
Jira curl command across jira-forge, issue-labels/jira,
jira-components, and code-implementation skills. Jira Cloud tenant
URLs require Basic auth (email:api_token); the OpenShell static
provider replaces the opaque JIRA_TOKEN placeholder in the Basic
Authorization header at the proxy boundary. Without the --user flag,
sandbox requests are anonymous even when the jira-ro provider is
attached.
Update docs/code.md and docs/triage.md to document this distinction:
the real token is never expanded into sandbox config or env files,
while sandbox curl commands carry the provider placeholder through
Basic auth.
Add test assertions in harness-jira-test.sh (skill-level --user
presence + no-bearer-auth guard) and sandbox-credential-boundary-test.sh
(positive complement verifying skills carry the placeholder).
Addresses #1109
🔧 Fix agent — iteration 5 (human-triggered)Restored --user "${JIRA_USER_EMAIL}:${JIRA_TOKEN}" in all sandbox Jira curl commands across 4 skill files. Updated docs to document the distinction: real token never enters sandbox config, while sandbox requests carry the opaque provider placeholder through Basic auth. Added test assertions for --user presence and no-bearer-auth guard. Fixed (8):
Tests: passed
Decision points
Next steps:
|
|
🤖 Finished Review · ✅ Success · Started 2:26 AM UTC · Completed 2:46 AM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $9.60 |
fullsend-ai-review
Bot
added
risk/elevated
fullsend-ai-review
Bot
removed
the
requires-manual-review
|
🤖 Finished Fix · ❌ Failure (running pre-script: exit status 1) · Started 2:48 AM UTC · Completed 2:48 AM UTC Commit: Effort: high |
Signed-off-by: Ralph Bean <rbean@redhat.com>
|
🤖 Finished Review · ✅ Success · Started 5:04 PM UTC · Completed 5:42 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $8.94 |
|
🤖 Finished Retro · ✅ Success · Started 5:22 PM UTC · Completed 5:36 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $5.73 |
Retro: PR #1109 — use OpenShell providers for Jira credentialsTimelineIssue #1108 requested converging triage and code Jira overlays on OpenShell provider-backed credential delivery. The triage agent ran successfully ($1.05, 6 min). The code agent created PR #1109 with a 14-file, 477+/299− implementation ($5.65, 16 min). Six review and six fix iterations followed over ~44 hours before the human maintainer approved and merged. What went well
What went poorlyReview-directed regression (primary issue). In Review Iteration 4 (commit Repeated informational finding. The Test tooling mismatch. The fix agent created test scripts using Python YAML parsing ( Cost. Estimated total: ~$81+ across all agent runs (6 reviews at $5–10 each, 6 fix iterations, triage, code). The oscillation cycle (remove then restore Existing issues with new evidence
Proposals filed
|
|
Review skipped — this PR is already merged. The Posted by fullsend post-review check |
1 participant
Summary
JIRA_TOKENnever enters either sandbox — only thejira-roprovider's opaque placeholder is available to sandboxed Jira clients..issue-context.jsonprefetch from the code agent; the sandbox now reads Jira work items directly through the provider boundary viacurl.harness-jira-test.shverifying provider/profile selection and token absence from sandbox configuration for both agents.Changes
Harness configuration:
harness/triage.yaml— removeJIRA_TOKENfromenv.sandbox(provider supplies placeholder)harness/code.yaml— attachproviders/jira-ro.yamlandprofiles/fullsend-jira-ro.yaml; removehost_filesprefetch and credential env vars fromenv.runner; addJIRA_USER_EMAILandJIRA_BASE_URLtoenv.sandboxenv/jira/triage.env— removeJIRA_TOKENexport (was expanded into sandbox viaexpand: true)Scripts:
scripts/pre-code-jira.src.sh— remove credential requirements and issue-fetch logic; retain URL validation and pre-commit tool installationscripts/pre-code-jira.sh— rebuilt viamake script-buildSkills and docs:
skills/code-implementation/SKILL.md— update Jira issue fetching to use provider-backed API access instead of.issue-context.jsondocs/code.md— updateFULLSEND_TRACKERdescription and Jira-source overlay documentationdocs/triage.md— document provider-backed credential delivery in Jira setup and migration notesTests:
scripts/harness-jira-test.sh(new) — verifies provider/profile presence,JIRA_TOKENabsence from sandbox config, and related assertions for both triage and code harnessesscripts/pre-code-jira-test.sh— updated for simplified pre-script (removed prefetch/credential tests, added tests confirming credentials are no longer required)Testing
bash scripts/harness-jira-test.sh— 17/17 passbash scripts/pre-code-jira-test.sh— 8/8 pass (source and bundled modes)make check-bundle— passhack/lint-agent-docs— passscan-secrets— passpost-retro-test.shfailures (16) are unrelatedCloses #1108
Post-script verification
agent/1108-jira-provider-credentials)c4879824d5045bfb8c64527a74e620332e18d95a..HEAD)