Skip to content

refactor(skills): migrate pr-risk-assessment from CLAUDE_CONFIG_DIR to FULLSEND_SKILLS_DIR - #1677

Open
jflowers wants to merge 1 commit into
fullsend-ai:mainfrom
jflowers:opsx/runtime-neutral-skill-path
Open

jflowers wants to merge 1 commit into
fullsend-ai:mainfrom
jflowers:opsx/runtime-neutral-skill-path

Conversation

@jflowers

@jflowers jflowers commented Oct 14, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Replace runtime-specific CLAUDE_CONFIG_DIR path references with the
runtime-neutral FULLSEND_SKILLS_DIR env var so pr-risk-assessment
scripts work on any supported runtime, not just Claude.

Closes #1672

How to Test

# Verify no CLAUDE_CONFIG_DIR references remain
grep -r CLAUDE_CONFIG_DIR skills/ scripts/

# Run the skill env reachability test
bash scripts/skill-env-reachability-test.sh

# Run the risk-tier1 test suite
bash scripts/risk-tier1-test.sh

How to Demo

Trivial refactor — the invocation path in SKILL.md and
risk-assessment.md now uses ${FULLSEND_SKILLS_DIR}/pr-risk-assessment/scripts/risk-tier1.sh
instead of ${CLAUDE_CONFIG_DIR}/skills/pr-risk-assessment/scripts/risk-tier1.sh.
The reachability test validates FULLSEND_SKILLS_DIR is in the sandbox
runtime vars and the previously known gap is resolved.

Key Files Changed

File Change
skills/pr-risk-assessment/SKILL.md Script invocation path updated
skills/pr-review/sub-agents/risk-assessment.md Script invocation path updated
skills/pr-risk-assessment/scripts/risk-tier1.sh Header comment updated
scripts/skill-env-reachability-test.sh Added FULLSEND_SKILLS_DIR to SANDBOX_RUNTIME_VARS; removed resolved KNOWN_GAPS entry

I wrote this contribution with AI assistance and cannot explain all changes in it.

This PR was generated by /uf.finale (AI-assisted).

@jflowers
jflowers requested a review from a team as a code owner October 14, 2026 17:11
@jflowers jflowers self-assigned this Oct 14, 2026
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Use runtime-neutral paths for PR risk assessment

✨ Enhancement 🧪 Tests 🕐 10-20 Minutes

Grey Divider

AI Description

• Point risk-assessment instructions to the Tier 1 script through FULLSEND_SKILLS_DIR instead of
 Claude-specific configuration.
• Recognize the shared skills directory in reachability checks and remove the resolved exception.
Diagram

graph TD
  A["Risk skill"] --> B["Risk sub-agent"] --> C["Shared skills path"] --> D["Tier 1 script"]
  E["Reachability test"] --> C
Loading
High-Level Assessment

Use the existing FULLSEND_SKILLS_DIR path in both invocation instructions. It removes the Claude-specific assumption without adding runtime detection or changing Tier 1 behavior.

Files changed (4) +4 / -7

Tests (1) +1 / -4
skill-env-reachability-test.shRecognize the shared skills directory +1/-4

Recognize the shared skills directory

• Adds 'FULLSEND_SKILLS_DIR' to sandbox runtime variables and removes the tracked 'CLAUDE_CONFIG_DIR' reachability exception for PR risk assessment.

scripts/skill-env-reachability-test.sh

Documentation (3) +3 / -3
risk-assessment.mdUse the shared path in sub-agent instructions +1/-1

Use the shared path in sub-agent instructions

• Changes the Tier 1 command to resolve the script through 'FULLSEND_SKILLS_DIR' rather than Claude's configuration directory.

skills/pr-review/sub-agents/risk-assessment.md

SKILL.mdUse the shared path in the risk skill +1/-1

Use the shared path in the risk skill

• Changes the documented Tier 1 invocation to use the runtime-neutral skills directory.

skills/pr-risk-assessment/SKILL.md

risk-tier1.shAlign the script header with the new invocation +1/-1

Align the script header with the new invocation

• Updates the header's example command to use 'FULLSEND_SKILLS_DIR'; script behavior is unchanged.

skills/pr-risk-assessment/scripts/risk-tier1.sh

@fullsend-ai-review

fullsend-ai-review Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 5:13 PM UTC · Completed 5:20 PM UTC

Commit: 66d79ae · View workflow run →

Runtime: pi · Model: sonnet → claude-sonnet-5-5 · Effort: high · Cost: $1.30

@qodo-code-review

qodo-code-review Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (1)

Grey Divider


Action required

1. Protected files require human approval 📜 Skill insight § Compliance
Description
The PR modifies files under scripts/ and skills/, both listed in the repository's active
REVIEW_PROTECTED_PATHS value. Any changed file under those prefixes requires a protected-path
review finding, so this PR cannot be auto-approved.
Code

scripts/skill-env-reachability-test.sh[43]

+  FULLSEND_SKILLS_DIR
Relevance

●●● Strong

Protected-path governance is explicit, and recent history accepts REVIEW_PROTECTED_PATHS enforcement
concerns.

PR-#569

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The changed files are under the protected scripts/ and skills/ prefixes, and the repository
config lists both prefixes in REVIEW_PROTECTED_PATHS; the checklist requires a protected-path
finding for every matching changed file.

scripts/post-review-test.sh[432-432]
scripts/skill-env-reachability-test.sh[43-43]
skills/pr-risk-assessment/SKILL.md[207-207]
Skill: pr-review

Dismiss ↗ | View ↗


2. Risk script path is unset ⊘ Outdated
Description
The skill instructions now invoke the Tier 1 script through FULLSEND_SKILLS_DIR, but the pinned
fullsend runner does not export or reserve that variable. On every supported runtime the expansion
is empty unless fullsend is updated, so the risk-assessment sub-agent attempts to run
/pr-risk-assessment/scripts/risk-tier1.sh instead of the uploaded script.
Code

skills/pr-review/sub-agents/risk-assessment.md[27]

+   bash "${FULLSEND_SKILLS_DIR}/pr-risk-assessment/scripts/risk-tier1.sh"
Relevance

●●● Strong

The runtime-neutral path depends on an unverified cross-repository environment contract, creating a
concrete execution failure.

PR-#1181

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The PR changes both skill instructions to rely on FULLSEND_SKILLS_DIR, and adds it to a local
reachability-test allowlist, but the pinned fullsend runner's infrastructure environment list
contains no such variable and its runtime export for Claude only defines CLAUDE_CONFIG_DIR.
Fullsend uploads the same skills to separate runtime-specific directories for Claude, Codex, and pi,
so the new path cannot resolve without a coordinated fullsend contract change.

agents -> fullsend
skills/pr-review/sub-agents/risk-assessment.md[25-28]
scripts/skill-env-reachability-test.sh[34-45]
External repo: fullsend-ai/fullsend, internal/runtime/claude.go [66-69]
External repo: fullsend-ai/fullsend, internal/cli/run.go [3231-3247]
External repo: fullsend-ai/fullsend, internal/runtime/claude.go [120-136]
External repo: fullsend-ai/fullsend, internal/runtime/codex_bootstrap.go [198-206]
External repo: fullsend-ai/fullsend, internal/runtime/pi_bootstrap.go [259-269]

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 agents skill now depends on `FULLSEND_SKILLS_DIR`, but the pinned fullsend runner does not inject that environment variable and uploads skills into runtime-specific directories. This makes the Tier 1 script invocation fail in deployed runs.

### Fix Focus Areas
- skills/pr-review/sub-agents/risk-assessment.md[27-27]
- skills/pr-risk-assessment/SKILL.md[207-207]
- internal/cli/run.go[3231-3247]
- internal/cli/run.go[3640-3648]
- internal/runtime/claude.go[66-69]
- internal/runtime/codex_bootstrap.go[198-206]
- internal/runtime/pi_bootstrap.go[259-269]

### Recommended Fix
Coordinate the change with fullsend: define the runtime-neutral `FULLSEND_SKILLS_DIR` contract, export it from the runner during sandbox bootstrap, reserve it from harness overrides, and make it point to the directory where the selected runtime uploads the skills. Update the fullsend runtime tests and release/dependency pin before relying on this variable in the agents skill; otherwise revert the skill invocations to paths guaranteed by the currently deployed runner.

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



Remediation recommended

3. A header comment changes without code ✓ Resolved
Description
The header comment in risk-tier1.sh changes the documented invocation path while the surrounding
executable script remains unchanged. Because this hunk contains only a comment edit, it adds
maintenance noise without a functional change for the comment to explain.
Code

skills/pr-risk-assessment/scripts/risk-tier1.sh[5]

+#   bash "${FULLSEND_SKILLS_DIR}/pr-risk-assessment/scripts/risk-tier1.sh"
Relevance

●●● Strong

The repository enforces comment-only change rules; this isolated documentation hunk is directly
reviewable under that policy.

PR-#1520

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The diff changes only the invocation comment on line 5 of risk-tier1.sh; no executable line in
that hunk changes. This directly matches the checklist's prohibition on comment changes without
associated functional changes.

Rule 1540690: Do not change comments on lines without functional code changes in a diff
skills/pr-risk-assessment/scripts/risk-tier1.sh[5-5]

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 `risk-tier1.sh` header comment is modified without any functional code change in the same block, violating the requirement that comment-only changes document an associated implementation change.

## Fix Focus Areas
- skills/pr-risk-assessment/scripts/risk-tier1.sh[5-5]

## Recommended Fix
Revert the standalone header-comment edit, or include a corresponding executable change in the same block if the invocation behavior itself is being changed there.

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


Grey Divider

Context sources
✅ Compliance rules (platform): 57 rules
✅ Skills: 4 invoked
  code-review
  code-implementation
  pr-review
  docs-review
✅ Cross-repo context — repo relationships
  Explored: repo: fullsend-ai/fullsend (sha: f5934f59) — View relationship
Review mode: Auto: 🚀 Fast: Localized path refactor with minimal, self-contained runtime impact.

Grey Divider

Tip of the day
💡 Did you know, you can enable the Remediation agent and Qodo fixes findings in a dedicated fix PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

FULLSEND_OUTPUT_DIR
FULLSEND_OUTPUT_SCHEMA
FULLSEND_ROLE
FULLSEND_SKILLS_DIR

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

1. Protected files require human approval 📜 Skill insight § Compliance

The PR modifies files under scripts/ and skills/, both listed in the repository's active
REVIEW_PROTECTED_PATHS value. Any changed file under those prefixes requires a protected-path
review finding, so this PR cannot be auto-approved.

Dismiss ↗ | View ↗

Comment thread skills/pr-risk-assessment/scripts/risk-tier1.sh
Comment thread skills/pr-review/sub-agents/risk-assessment.md
@fullsend-ai-review fullsend-ai-review Bot added the risk/moderate PR risk: moderate label Oct 14, 2026
@fullsend-ai-review

fullsend-ai-review Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

<!-- fullsend:risk-assessment -->
Risk Assessment: moderate (2/5)

Details

Preserving prior score of 2 (moderate) as Tier 1 metadata signals are unchanged for this 4-file, 11-line mechanical env-var rename touching 4 protected paths with no CI, dependency, or security-sensitive changes.

Previous run

<!-- fullsend:risk-assessment -->
Risk Assessment: moderate (2/5)

Details

Preserving prior score of 2 (moderate) as Tier 1 metadata signals are unchanged for this 4-file, 11-line mechanical env-var rename touching 4 protected paths with no CI, dependency, or security-sensitive changes.

Previous run (2)

<!-- fullsend:risk-assessment -->
Risk Assessment: moderate (2/5)

Details

Tier 1 signals are unchanged from the prior run: a tiny 4-file, 11-line mechanical env-var rename with no CI, dependency, or security changes across 4 protected paths, yielding a composite of 2.1 that anchors the score at moderate (2).

Previous run (3)

<!-- fullsend:risk-assessment -->
Risk Assessment: moderate (2/5)

Details

Tier 1 signals are unchanged from the prior run: a tiny 4-file, 11-line mechanical env-var rename with no CI, dependency or security-sensitive changes, and all 4 files are protected paths. Fix-heavy file history and the dependency on upstream runtimes exporting FULLSEND_SKILLS_DIR give a composite of about 2.1, so the prior score of 2 stands.

Previous run (4)

<!-- fullsend:risk-assessment -->
Risk Assessment: moderate (2/5)

Details

Tiny 11-line, 4-file mechanical env-var rename with no CI, dependency or security-sensitive changes and a well-scoped issue, but all 4 files are protected paths and have heavy recent fix churn, giving a composite of about 2.0. Residual risk is that runtimes that do not export FULLSEND_SKILLS_DIR will break the script path.

Previous run (5)

<!-- fullsend:risk-assessment -->
Risk Assessment: moderate (2/5)

Details

Tiny 11-line, 4-file mechanical env-var rename with no CI, dependency or security-sensitive changes and a scoped issue, but all 4 files are protected paths and the files have had frequent fix commits recently, so the composite is about 2.1, which rounds to 2.

Previous run (6)

<!-- fullsend:risk-assessment -->
Risk Assessment: moderate (2/5)

Details

Tier 1 signals are unchanged from the prior run (4 files, 11 lines, 4 protected paths, no dependency changes, human author), so the prior score of 2 stands. Tier 2 shows moderate churn and heavy fix history. Tier 3 shows a small, well-scoped change, with some rollback risk because runtime support for FULLSEND_SKILLS_DIR is unconfirmed.

Previous run (7)

<!-- fullsend:risk-assessment -->
Risk Assessment: moderate (2/5)

Details

Tier 1 signals are unchanged from the prior run (4 files, 11 lines, 4 protected paths, no dependency changes, human author), so the prior score of 2 stands. Tier 2 shows moderate churn and heavy fix history. Tier 3 shows a small, well-scoped change with rollback risk because upstream FULLSEND_SKILLS_DIR support is unconfirmed.

Previous run (8)

<!-- fullsend:risk-assessment -->
Risk Assessment: moderate (2/5)

Details

Tier 1 signals match the prior run (4 files, 11 lines, 4 protected paths, no dependency changes, human author), so the prior score of 2 is kept. The change is a small, focused variable rename. The residual risk is high fix/revert history on the touched files and unconfirmed runtime export of FULLSEND_SKILLS_DIR.

Previous run (9)

<!-- fullsend:risk-assessment -->
Risk Assessment: moderate (2/5)

Details

Tiny 4-file, 11-line refactor that swaps one env var path for another and matches the issue scope. Risk is lifted slightly above the minimum by 4 protected-path files (agent skill and script files), high fix/revert history on the touched files, and no feature flag.

Previous run (10)

<!-- fullsend:risk-assessment -->
Risk Assessment: moderate (2/5)

Details

Tier 1 is unchanged from the prior run (4 files, 11 lines, 4 protected paths, human author, no CI or dependency changes). Tier 2 still shows moderate churn and many fix/revert commits, and Tier 3 shows a small, well-scoped change, so the prior score of 2 stands. The residual risk is that the change merges before runtimes export FULLSEND_SKILLS_DIR.

Previous run (11)

<!-- fullsend:risk-assessment -->
Risk Assessment: moderate (2/5)

Details

Tier 1 is unchanged from the prior run: 4 files, 11 lines, 4 protected paths, human author, no CI or dependency changes. Tier 2 shows moderate churn and a high count of fix/revert commits, and Tier 3 shows a small, well-scoped change. The weighted composite is about 2.2, so the prior score of 2 stands. The main residual risk is that the migration may land before the runtime exports FULLSEND_SKILLS_DIR, which would break the skill path.

Previous run (12)

<!-- fullsend:risk-assessment -->
Risk Assessment: moderate (2/5)

Details

Tier 1 is unchanged from the prior run: 4 files, 11 lines, 4 protected paths, human author, no CI or dependency changes. Nothing in Tier 2 or Tier 3 justifies changing the prior score of 2. The main residual risk is that the runtime may not export FULLSEND_SKILLS_DIR.

Previous run (13)

<!-- fullsend:risk-assessment -->
Risk Assessment: moderate (2/5)

Details

Tier 1 is unchanged from the prior run (4 files, 11 lines, 4 protected paths, human author), Tier 2 shows moderate churn and heavy fix history, and Tier 3 carries rollback risk because the runtime-side env var is not confirmed, so the prior score of 2 stands.

Previous run (14)

<!-- fullsend:risk-assessment -->
Risk Assessment: moderate (2/5)

Details

Tier 1 is unchanged from the prior run (4 files, 11 lines, 4 protected paths, human author). Tier 2 shows moderate churn and a high fix history. Tier 3 carries rollback risk because upstream FULLSEND_SKILLS_DIR support is unconfirmed. The composite is about 2.05, so the prior score of 2 stands.

Previous run (15)

<!-- fullsend:risk-assessment -->
Risk Assessment: moderate (2/5)

Details

Tier 1 is unchanged from the prior run (4 files, 11 lines, 4 protected paths, human author). Tier 2 shows moderate churn and fix history, and Tier 3 carries a small rollback risk because upstream FULLSEND_SKILLS_DIR support is unconfirmed. The composite is about 2.0, so the prior score of 2 stands.

Previous run (16)

<!-- fullsend:risk-assessment -->
Risk Assessment: moderate (2/5)

Details

Tier 1 is unchanged from the prior run (4 files, 11 lines, 4 protected paths, human author), and Tier 2 churn and fix history plus Tier 3 rollback concerns give a composite of about 2.0 (0.5×1.88 + 0.3×2.3 + 0.2×2.0), so the prior score of 2 stands.

Previous run (17)

<!-- fullsend:risk-assessment -->
Risk Assessment: moderate (2/5)

Details

Tier 1 is unchanged from the prior run: 4 files and 11 lines, all 4 files in protected paths, no CI, dependency or security-sensitive files, and a human author. The files have moderate recent churn and fix history, and the change alters the skill invocation path with no flag, so the composite is about 2.0 and the prior score of 2 stands.

Previous run (18)

<!-- fullsend:risk-assessment -->
Risk Assessment: moderate (2/5)

Details

Small 4-file, 11-line change with no CI, dependency or security-sensitive files, but all 4 files are in protected paths. It changes the skill invocation path with no flag while upstream FULLSEND_SKILLS_DIR support is unconfirmed. Tier 1 signals are unchanged from the prior run, so the score stays at 2.

Previous run (19)

Risk Assessment: moderate (2/5)

Details

Small 4-file, 11-line change with no CI, dependency or security-sensitive files, but all 4 files are in protected paths and it changes the skill invocation path with no flag while upstream FULLSEND_SKILLS_DIR support is unconfirmed; Tier 1 signals are unchanged from the prior run, so the score stays at 2.

Previous run (20)

Risk Assessment: moderate (2/5)

Details

Small 4-file, 11-line change with no CI, dependency or security-sensitive files, but all 4 files are in protected paths and it alters the skill invocation path with no flag while upstream support for FULLSEND_SKILLS_DIR is unconfirmed.

Previous run (21)

Risk Assessment: moderate (2/5)

Details

Tiny 4-file, 11-line refactor with no CI, dependency or security-sensitive changes, but all four files are protected paths and the FULLSEND_SKILLS_DIR migration may be blocked until upstream runtimes export the variable (unverified).

@fullsend-ai-review

fullsend-ai-review Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

Review

Findings

High

  • [breaking-config] skills/pr-review/sub-agents/risk-assessment.md:27 — The Tier 1 invocation now reads ${FULLSEND_SKILLS_DIR}, but nothing in this repo sets or exports that variable, and the pi review sandbox does not set it either. With it unset the path expands to /pr-risk-assessment/scripts/risk-tier1.sh, which does not exist, so the Tier 1 script fails (exit 127) and no metadata signals are produced. The identical invocation in skills/pr-risk-assessment/SKILL.md:207 has the same problem. Issue pr-risk-assessment skill path is not runtime-neutral #1672's triage comment says this migration is blocked until the upstream fullsend runtimes export the variable, and nothing here shows that has happened; CLAUDE_CONFIG_DIR currently works on the claude runtime, so this change breaks a working setup. The PR also adds FULLSEND_SKILLS_DIR to SANDBOX_RUNTIME_VARS and drops the KNOWN_GAPS entry, so the reachability test now treats the variable as runtime-provided instead of verifying it.
    Remediation: Do not merge until upstream fullsend exports FULLSEND_SKILLS_DIR from all runtimes and that is confirmed, or use a fallback that works today, e.g. ${FULLSEND_SKILLS_DIR:-${CLAUDE_CONFIG_DIR:-${PI_CODING_AGENT_DIR:-$CODEX_HOME}}/skills}. Keep the KNOWN_GAPS entry until the upstream change lands.

Medium

  • [protected-path] skills/pr-risk-assessment/SKILL.md — This PR modifies protected governance/infrastructure paths: scripts/skill-env-reachability-test.sh, skills/pr-review/sub-agents/risk-assessment.md, skills/pr-risk-assessment/SKILL.md, and skills/pr-risk-assessment/scripts/risk-tier1.sh. The PR links issue pr-risk-assessment skill path is not runtime-neutral #1672 and explains the rationale, but human approval is always required for protected-path changes regardless of context.

Low

  • [test-weakened] scripts/skill-env-reachability-test.sh:43 — Adding FULLSEND_SKILLS_DIR to SANDBOX_RUNTIME_VARS exempts it from reachability validation even though it is absent from the current sandbox and has no producer in this repository. Together with deleting the known-gap entry, this suppresses the unresolved skill-path dependency instead of verifying that runtime injection exists.
    Remediation: Keep the unresolved FULLSEND_SKILLS_DIR dependency tracked in KNOWN_GAPS rather than the universal runtime allowlist until injection across supported runtimes is verified.

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

<!-- fullsend:review-findings-v2:eyJ2ZXJzaW9uIjoyLCJhY3Rpb24iOiJyZXF1ZXN0LWNoYW5nZXMiLCJmaW5kaW5ncyI6W3sic2V2ZXJpdHkiOiJoaWdoIiwiY2F0ZWdvcnkiOiJicmVha2luZy1jb25maWciLCJmaWxlIjoic2tpbGxzL3ByLXJldmlldy9zdWItYWdlbnRzL3Jpc2stYXNzZXNzbWVudC5tZCIsImlkIjoiZl9lMWIxZDc3M2I5NjZiYjAwIiwibGluZSI6MjcsImFjdGlvbmFibGUiOnRydWV9LHsic2V2ZXJpdHkiOiJsb3ciLCJjYXRlZ29yeSI6InRlc3Qtd2Vha2VuZWQiLCJmaWxlIjoic2NyaXB0cy9za2lsbC1lbnYtcmVhY2hhYmlsaXR5LXRlc3Quc2giLCJpZCI6ImZfMzdhYzJiNjg5NWQ4NzMyYiIsImxpbmUiOjQzLCJhY3Rpb25hYmxlIjpmYWxzZX1dLCJkaXNwb3NpdGlvbnMiOlt7ImlkIjoiZl9lMWIxZDc3M2I5NjZiYjAwIiwic3RhdHVzIjoib3BlbiJ9LHsiaWQiOiJmXzM3YWMyYjY4OTVkODczMmIiLCJzdGF0dXMiOiJvcGVuIn1dfQ== -->

Review

Findings

High

  • [breaking-config] skills/pr-review/sub-agents/risk-assessment.md:27 — The skill invocation now depends on FULLSEND_SKILLS_DIR, but nothing in this repo sets it, and the variable is unset in the current pi review sandbox (skills live at /sandbox/pi-config/skills). With it unset the path expands to /pr-risk-assessment/scripts/risk-tier1.sh and the Tier 1 script fails. The change also drops the CLAUDE_CONFIG_DIR path that worked on claude, with no fallback. The same dependency appears at skills/pr-risk-assessment/SKILL.md:207. Adding the variable to SANDBOX_RUNTIME_VARS only makes the reachability test tolerate it; it does not make any runtime provide it. The fullsend runtime repo was not available to check whether injection has landed.
    Remediation: Land and release runtime injection of FULLSEND_SKILLS_DIR in fullsend-ai/fullsend for every runtime first, then merge this migration. Alternatively keep a fallback in both invocations until then, e.g. ${FULLSEND_SKILLS_DIR:-${CLAUDE_CONFIG_DIR:-${PI_CODING_AGENT_DIR:-$CODEX_HOME}}/skills}.

Medium

  • [protected-path] skills/pr-review/sub-agents/risk-assessment.md — This PR modifies protected paths: scripts/skill-env-reachability-test.sh, skills/pr-review/sub-agents/risk-assessment.md, skills/pr-risk-assessment/SKILL.md, skills/pr-risk-assessment/scripts/risk-tier1.sh. The PR links issue pr-risk-assessment skill path is not runtime-neutral #1672 and explains the rationale, but human approval is always required for protected-path changes regardless of context.

Low

  • [test-weakened] scripts/skill-env-reachability-test.sh:43 — Adding FULLSEND_SKILLS_DIR to SANDBOX_RUNTIME_VARS and deleting the pr-risk-assessment:CLAUDE_CONFIG_DIR KNOWN_GAPS entry turns an unresolved dependency into a passing check without establishing runtime delivery; the file header says this list is only for vars injected on every runtime.
    Remediation: Keep the dependency tracked in KNOWN_GAPS until universal runtime injection is verified.

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)

<!-- fullsend:review-findings-v2:eyJ2ZXJzaW9uIjoyLCJhY3Rpb24iOiJyZXF1ZXN0LWNoYW5nZXMiLCJmaW5kaW5ncyI6W3sic2V2ZXJpdHkiOiJoaWdoIiwiY2F0ZWdvcnkiOiJicmVha2luZy1jb25maWciLCJmaWxlIjoic2tpbGxzL3ByLXJldmlldy9zdWItYWdlbnRzL3Jpc2stYXNzZXNzbWVudC5tZCIsImlkIjoiZl9lMWIxZDc3M2I5NjZiYjAwIiwibGluZSI6MjcsImFjdGlvbmFibGUiOnRydWV9LHsic2V2ZXJpdHkiOiJsb3ciLCJjYXRlZ29yeSI6InRlc3Qtd2Vha2VuZWQiLCJmaWxlIjoic2NyaXB0cy9za2lsbC1lbnYtcmVhY2hhYmlsaXR5LXRlc3Quc2giLCJpZCI6ImZfMzdhYzJiNjg5NWQ4NzMyYiIsImxpbmUiOjQzLCJhY3Rpb25hYmxlIjpmYWxzZX1dLCJkaXNwb3NpdGlvbnMiOlt7ImlkIjoiZl9lMWIxZDc3M2I5NjZiYjAwIiwic3RhdHVzIjoib3BlbiJ9LHsiaWQiOiJmXzM3YWMyYjY4OTVkODczMmIiLCJzdGF0dXMiOiJvcGVuIn1dfQ== -->

Review

Findings

High

  • [breaking-config] skills/pr-review/sub-agents/risk-assessment.md:27 — The Tier 1 command now depends on ${FULLSEND_SKILLS_DIR}, and nothing in this repo defines or exports that variable. The review sandbox for this run (pi runtime) has it unset, with PI_CODING_AGENT_DIR=/sandbox/pi-config. When it is unset the path expands to /pr-risk-assessment/scripts/risk-tier1.sh, the script is not found (exit 127), and no Tier 1 signals are produced. The same dependency is in skills/pr-risk-assessment/SKILL.md line 207. Issue pr-risk-assessment skill path is not runtime-neutral #1672's triage comment says the upstream fullsend runtime export is a prerequisite, and nothing shows it has landed. Migrating now also regresses the claude runtime, which works today through CLAUDE_CONFIG_DIR.
    Remediation: Merge only after the upstream fullsend runtime exports FULLSEND_SKILLS_DIR (pointing at the skills directory) on claude, codex and pi. Until then, add a fallback in both invocations, such as ${FULLSEND_SKILLS_DIR:-${CLAUDE_CONFIG_DIR}/skills}, or fail fast with a clear message when the variable is unset.

Medium

  • [protected-path] skills/pr-risk-assessment/SKILL.md — This PR modifies protected paths: scripts/skill-env-reachability-test.sh, skills/pr-review/sub-agents/risk-assessment.md, skills/pr-risk-assessment/SKILL.md, skills/pr-risk-assessment/scripts/risk-tier1.sh. The PR links issue pr-risk-assessment skill path is not runtime-neutral #1672 and explains the rationale, but human approval is always required for protected-path changes.

Low

  • [test-weakened] scripts/skill-env-reachability-test.sh:43 — Adding FULLSEND_SKILLS_DIR to SANDBOX_RUNTIME_VARS, and removing the pr-risk-assessment:CLAUDE_CONFIG_DIR KNOWN_GAPS entry, makes the reachability test report a pass for a variable that is absent from the current pi sandbox. The header of this script limits the allowlist to variables injected on every runtime, and that has not been confirmed for this one.
    Remediation: Keep FULLSEND_SKILLS_DIR out of SANDBOX_RUNTIME_VARS, and keep the pr-risk-assessment skill path is not runtime-neutral #1672 KNOWN_GAPS entry, until universal runtime injection is confirmed.

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)

<!-- fullsend:review-findings-v2:eyJ2ZXJzaW9uIjoyLCJhY3Rpb24iOiJyZXF1ZXN0LWNoYW5nZXMiLCJmaW5kaW5ncyI6W3sic2V2ZXJpdHkiOiJoaWdoIiwiY2F0ZWdvcnkiOiJicmVha2luZy1jb25maWciLCJmaWxlIjoic2tpbGxzL3ByLXJldmlldy9zdWItYWdlbnRzL3Jpc2stYXNzZXNzbWVudC5tZCIsImlkIjoiZl9lMWIxZDc3M2I5NjZiYjAwIiwibGluZSI6MjcsImFjdGlvbmFibGUiOnRydWV9LHsic2V2ZXJpdHkiOiJsb3ciLCJjYXRlZ29yeSI6InRlc3Qtd2Vha2VuZWQiLCJmaWxlIjoic2NyaXB0cy9za2lsbC1lbnYtcmVhY2hhYmlsaXR5LXRlc3Quc2giLCJpZCI6ImZfMzdhYzJiNjg5NWQ4NzMyYiIsImxpbmUiOjQzLCJhY3Rpb25hYmxlIjpmYWxzZX1dLCJkaXNwb3NpdGlvbnMiOlt7ImlkIjoiZl9lMWIxZDc3M2I5NjZiYjAwIiwic3RhdHVzIjoib3BlbiJ9LHsiaWQiOiJmXzM3YWMyYjY4OTVkODczMmIiLCJzdGF0dXMiOiJvcGVuIn1dfQ== -->

Review

Findings

High

  • [breaking-config] skills/pr-review/sub-agents/risk-assessment.md:27 — The sub-agent now runs bash "${FULLSEND_SKILLS_DIR}/pr-risk-assessment/scripts/risk-tier1.sh". Nothing in this repo defines or exports FULLSEND_SKILLS_DIR: a grep of the base checkout finds no match, the PR changes no harness, and issue pr-risk-assessment skill path is not runtime-neutral #1672's triage thread says the upstream runtimes must export it first. In this review's pi sandbox the variable is unset, so the path expands to /pr-risk-assessment/scripts/risk-tier1.sh and the Tier 1 script cannot be found. The claude runtime works today through CLAUDE_CONFIG_DIR and would break the same way. The same unresolved path appears at skills/pr-risk-assessment/SKILL.md:207 and in the comment at skills/pr-risk-assessment/scripts/risk-tier1.sh:5. The PR does not show that the upstream prerequisite has been met.
    Remediation: Do not merge until the upstream fullsend runtimes (claude, codex, pi) export FULLSEND_SKILLS_DIR, and link the upstream release or PR that does so. Alternatively keep a temporary fallback such as ${FULLSEND_SKILLS_DIR:-${CLAUDE_CONFIG_DIR}/skills}.

Medium

  • [protected-path] skills/pr-risk-assessment/SKILL.md — This PR modifies protected governance/infrastructure paths: scripts/skill-env-reachability-test.sh, skills/pr-review/sub-agents/risk-assessment.md, skills/pr-risk-assessment/SKILL.md, and skills/pr-risk-assessment/scripts/risk-tier1.sh. It links issue pr-risk-assessment skill path is not runtime-neutral #1672 and explains the rationale, but human approval is always required for protected-path changes.

Low

  • [test-weakened] scripts/skill-env-reachability-test.sh:43 — Adding FULLSEND_SKILLS_DIR to SANDBOX_RUNTIME_VARS skips its reachability check for every skill, and removing the pr-risk-assessment:CLAUDE_CONFIG_DIR known-gap entry (pr-risk-assessment skill path is not runtime-neutral #1672) removes the warning that tracked this gap. The test now treats the variable as delivered by every runtime, and nothing in this repo confirms that.
    Remediation: Keep the variable tracked as a known reachability gap until runtime injection is confirmed upstream, rather than exempting it globally.

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)

<!-- fullsend:review-findings-v2:eyJ2ZXJzaW9uIjoyLCJhY3Rpb24iOiJyZXF1ZXN0LWNoYW5nZXMiLCJmaW5kaW5ncyI6W3sic2V2ZXJpdHkiOiJoaWdoIiwiY2F0ZWdvcnkiOiJicmVha2luZy1jb25maWciLCJmaWxlIjoic2tpbGxzL3ByLXJldmlldy9zdWItYWdlbnRzL3Jpc2stYXNzZXNzbWVudC5tZCIsImlkIjoiZl9lMWIxZDc3M2I5NjZiYjAwIiwibGluZSI6MjcsImFjdGlvbmFibGUiOnRydWV9LHsic2V2ZXJpdHkiOiJsb3ciLCJjYXRlZ29yeSI6InRlc3Qtd2Vha2VuZWQiLCJmaWxlIjoic2NyaXB0cy9za2lsbC1lbnYtcmVhY2hhYmlsaXR5LXRlc3Quc2giLCJpZCI6ImZfMzdhYzJiNjg5NWQ4NzMyYiIsImxpbmUiOjQzLCJhY3Rpb25hYmxlIjpmYWxzZX1dLCJkaXNwb3NpdGlvbnMiOlt7ImlkIjoiZl9lMWIxZDc3M2I5NjZiYjAwIiwic3RhdHVzIjoib3BlbiJ9LHsiaWQiOiJmXzM3YWMyYjY4OTVkODczMmIiLCJzdGF0dXMiOiJvcGVuIn1dfQ== -->

Review

Findings

High

  • [breaking-config] skills/pr-review/sub-agents/risk-assessment.md:27 — The Tier 1 invocation now depends on FULLSEND_SKILLS_DIR, but nothing in this repo or PR defines or injects it: no harness/*.yaml env.sandbox block sets it, and it is unset in the live pi review sandbox (skills live at /sandbox/pi-config/skills). The command expands to bash "/pr-risk-assessment/scripts/risk-tier1.sh" and Tier 1 collection fails. This does not fix pi/codex and regresses claude, where CLAUDE_CONFIG_DIR previously worked. The same invocation is at skills/pr-risk-assessment/SKILL.md:207. The PR does not state that it depends on a fullsend-side runtime change.
    Remediation: Land the fullsend runtime change that exports FULLSEND_SKILLS_DIR=<ConfigDir>/skills on every runtime (and reference it) before switching these invocations; or define the var in the harness env.sandbox blocks delivering the skill; or keep a fallback in both invocations, e.g. ${FULLSEND_SKILLS_DIR:-${CLAUDE_CONFIG_DIR}/skills}. Keep the KNOWN_GAPS entry until the variable is verifiably set.

Medium

  • [protected-path] skills/pr-risk-assessment/SKILL.md — This PR modifies protected paths: scripts/skill-env-reachability-test.sh, skills/pr-review/sub-agents/risk-assessment.md, skills/pr-risk-assessment/SKILL.md, skills/pr-risk-assessment/scripts/risk-tier1.sh. The PR links pr-risk-assessment skill path is not runtime-neutral #1672 and explains the rationale, but human approval is always required for protected-path changes, regardless of context.

Low

  • [test-weakened] scripts/skill-env-reachability-test.sh:43 — Adding FULLSEND_SKILLS_DIR to SANDBOX_RUNTIME_VARS and removing the pr-risk-assessment:CLAUDE_CONFIG_DIR KNOWN_GAPS entry makes the reachability check pass unconditionally for a variable that is not delivered by any harness or verified in the runtime. The gap is hidden rather than validated.
    Remediation: Keep FULLSEND_SKILLS_DIR out of SANDBOX_RUNTIME_VARS and track pr-risk-assessment:FULLSEND_SKILLS_DIR in KNOWN_GAPS until runtime injection is verified.

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

<!-- fullsend:review-findings-v2:eyJ2ZXJzaW9uIjoyLCJhY3Rpb24iOiJyZXF1ZXN0LWNoYW5nZXMiLCJmaW5kaW5ncyI6W3sic2V2ZXJpdHkiOiJoaWdoIiwiY2F0ZWdvcnkiOiJicmVha2luZy1jb25maWciLCJmaWxlIjoic2tpbGxzL3ByLXJldmlldy9zdWItYWdlbnRzL3Jpc2stYXNzZXNzbWVudC5tZCIsImlkIjoiZl9lMWIxZDc3M2I5NjZiYjAwIiwibGluZSI6MjcsImFjdGlvbmFibGUiOnRydWV9LHsic2V2ZXJpdHkiOiJsb3ciLCJjYXRlZ29yeSI6InRlc3Qtd2Vha2VuZWQiLCJmaWxlIjoic2NyaXB0cy9za2lsbC1lbnYtcmVhY2hhYmlsaXR5LXRlc3Quc2giLCJpZCI6ImZfMzdhYzJiNjg5NWQ4NzMyYiIsImxpbmUiOjQzLCJhY3Rpb25hYmxlIjpmYWxzZX1dLCJkaXNwb3NpdGlvbnMiOlt7ImlkIjoiZl9lMWIxZDc3M2I5NjZiYjAwIiwic3RhdHVzIjoib3BlbiJ9LHsiaWQiOiJmXzM3YWMyYjY4OTVkODczMmIiLCJzdGF0dXMiOiJvcGVuIn1dfQ== -->

Review

Findings

High

  • [breaking-config] skills/pr-review/sub-agents/risk-assessment.md:27 — The PR replaces ${CLAUDE_CONFIG_DIR}/skills/... with ${FULLSEND_SKILLS_DIR}/... at risk-assessment.md:27 and skills/pr-risk-assessment/SKILL.md:207, with no fallback. Nothing in this repo sets FULLSEND_SKILLS_DIR: a grep over the base checkout finds nothing, and in the PR head the only hits are these consumers and the test exemption. The variable is also empty in the pi review sandbox. Where it is unset, the command becomes bash "/pr-risk-assessment/scripts/risk-tier1.sh", which fails with 'No such file' (exit 127), and the Tier 1 signals are lost. Claude-runtime deployments worked before because CLAUDE_CONFIG_DIR was set there, so this regresses them unless the fullsend runtime injection described in pr-risk-assessment skill path is not runtime-neutral #1672 has shipped. This PR neither verifies nor gates on that dependency. (Codex and pi were already broken on the old path, so the regression is specific to Claude.)
    Remediation: Do not merge until the fullsend runtime injection of FULLSEND_SKILLS_DIR is released on every runtime (claude, codex and pi), and record the minimum fullsend version needed. Alternatively, keep a working fallback such as ${FULLSEND_SKILLS_DIR:-${CLAUDE_CONFIG_DIR:+$CLAUDE_CONFIG_DIR/skills}}, or fail fast with a clear error when no skills dir resolves.

Medium

  • [protected-path] skills/pr-review/sub-agents/risk-assessment.md — This PR modifies protected paths: scripts/skill-env-reachability-test.sh, skills/pr-review/sub-agents/risk-assessment.md, skills/pr-risk-assessment/SKILL.md, skills/pr-risk-assessment/scripts/risk-tier1.sh. The PR links issue pr-risk-assessment skill path is not runtime-neutral #1672 and explains the rationale, but human approval is always required for protected-path changes, regardless of context.

Low

  • [test-weakened] scripts/skill-env-reachability-test.sh:43 — The PR adds FULLSEND_SKILLS_DIR to SANDBOX_RUNTIME_VARS, which exempts it from reachability validation. The file header limits that list to vars 'injected into the sandbox on every runtime' and says other vars belong in KNOWN_GAPS. The variable is absent from the current sandbox and this PR introduces nothing that sets it. Deleting the pr-risk-assessment:CLAUDE_CONFIG_DIR KNOWN_GAPS entry at the same time turns an unresolved dependency the test explicitly reported into a passing check.
    Remediation: Keep the dependency tracked in KNOWN_GAPS (as pr-risk-assessment:FULLSEND_SKILLS_DIR, referencing pr-risk-assessment skill path is not runtime-neutral #1672) until universal runtime injection is verified, then move it to SANDBOX_RUNTIME_VARS.

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

<!-- fullsend:review-findings-v2:eyJ2ZXJzaW9uIjoyLCJhY3Rpb24iOiJyZXF1ZXN0LWNoYW5nZXMiLCJmaW5kaW5ncyI6W3sic2V2ZXJpdHkiOiJoaWdoIiwiY2F0ZWdvcnkiOiJicmVha2luZy1jb25maWciLCJmaWxlIjoic2tpbGxzL3ByLXJldmlldy9zdWItYWdlbnRzL3Jpc2stYXNzZXNzbWVudC5tZCIsImlkIjoiZl9lMWIxZDc3M2I5NjZiYjAwIiwibGluZSI6MjcsImFjdGlvbmFibGUiOnRydWV9LHsic2V2ZXJpdHkiOiJsb3ciLCJjYXRlZ29yeSI6InRlc3Qtd2Vha2VuZWQiLCJmaWxlIjoic2NyaXB0cy9za2lsbC1lbnYtcmVhY2hhYmlsaXR5LXRlc3Quc2giLCJpZCI6ImZfMzdhYzJiNjg5NWQ4NzMyYiIsImxpbmUiOjQzLCJhY3Rpb25hYmxlIjpmYWxzZX1dLCJkaXNwb3NpdGlvbnMiOlt7ImlkIjoiZl9lMWIxZDc3M2I5NjZiYjAwIiwic3RhdHVzIjoib3BlbiJ9LHsiaWQiOiJmXzM3YWMyYjY4OTVkODczMmIiLCJzdGF0dXMiOiJvcGVuIn1dfQ== -->

Review

Findings

High

  • [breaking-config] skills/pr-review/sub-agents/risk-assessment.md:27 — The Tier 1 invocation here and in skills/pr-risk-assessment/SKILL.md:207 now depends on FULLSEND_SKILLS_DIR, with no fallback. Nothing in this repo defines or exports that variable, and the only references in the PR head are the ones this diff adds. It is not set in the current pi review sandbox, although the skills tree exists at /sandbox/pi-config/skills. Issue pr-risk-assessment skill path is not runtime-neutral #1672 leaves the injection to the fullsend repo, and nothing here shows that change has shipped. On a runtime that does not inject the variable, the command expands to bash "/pr-risk-assessment/scripts/risk-tier1.sh" and cannot find the script, so Tier 1 metadata signals are lost. The old ${CLAUDE_CONFIG_DIR}/skills/... path worked on claude, so this regresses the runtime that works today unless fullsend injects the variable first.
    Remediation: Confirm the fullsend runtime change that injects FULLSEND_SKILLS_DIR on every runtime is merged and released in the lockstep version before merging. Otherwise keep a fallback such as ${FULLSEND_SKILLS_DIR:-${CLAUDE_CONFIG_DIR}/skills}, or add a fail-fast check that the variable is set, in both SKILL.md and risk-assessment.md.

Medium

  • [protected-path] skills/pr-review/sub-agents/risk-assessment.md — This PR modifies protected paths: scripts/skill-env-reachability-test.sh, skills/pr-review/sub-agents/risk-assessment.md, skills/pr-risk-assessment/SKILL.md and skills/pr-risk-assessment/scripts/risk-tier1.sh. The PR links issue pr-risk-assessment skill path is not runtime-neutral #1672 and explains the rationale, but human approval is always required for protected-path changes, regardless of context.

Low

  • [test-weakened] scripts/skill-env-reachability-test.sh:43 — Adding FULLSEND_SKILLS_DIR to SANDBOX_RUNTIME_VARS and removing the pr-risk-assessment:CLAUDE_CONFIG_DIR KNOWN_GAPS entry makes the reachability check treat the variable as injected on every runtime. Nothing in this repo shows that, and the variable is unset in the pi sandbox. The check now passes on an assumption instead of reporting the open dependency. The file header says the allowlist covers only variables injected on every runtime, and that per-skill gaps belong in KNOWN_GAPS. Issue pr-risk-assessment skill path is not runtime-neutral #1672 says to remove the entry "once resolved".
    Remediation: Keep the KNOWN_GAPS entry, and keep FULLSEND_SKILLS_DIR out of the universal allowlist, until a released fullsend version is confirmed to inject the variable. Alternatively, add a comment naming the fullsend PR or version that provides 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 (7)

<!-- fullsend:review-findings-v2:eyJ2ZXJzaW9uIjoyLCJhY3Rpb24iOiJyZXF1ZXN0LWNoYW5nZXMiLCJmaW5kaW5ncyI6W3sic2V2ZXJpdHkiOiJoaWdoIiwiY2F0ZWdvcnkiOiJicmVha2luZy1jb25maWciLCJmaWxlIjoic2tpbGxzL3ByLXJldmlldy9zdWItYWdlbnRzL3Jpc2stYXNzZXNzbWVudC5tZCIsImlkIjoiZl9lMWIxZDc3M2I5NjZiYjAwIiwibGluZSI6MjcsImFjdGlvbmFibGUiOnRydWV9LHsic2V2ZXJpdHkiOiJsb3ciLCJjYXRlZ29yeSI6InRlc3Qtd2Vha2VuZWQiLCJmaWxlIjoic2NyaXB0cy9za2lsbC1lbnYtcmVhY2hhYmlsaXR5LXRlc3Quc2giLCJpZCI6ImZfMzdhYzJiNjg5NWQ4NzMyYiIsImxpbmUiOjQzLCJhY3Rpb25hYmxlIjpmYWxzZX1dLCJkaXNwb3NpdGlvbnMiOlt7ImlkIjoiZl9lMWIxZDc3M2I5NjZiYjAwIiwic3RhdHVzIjoicmVjbGFzc2lmaWVkIn0seyJpZCI6ImZfMzdhYzJiNjg5NWQ4NzMyYiIsInN0YXR1cyI6Im9wZW4ifV19 -->

Review

Findings

High

  • [breaking-config] skills/pr-review/sub-agents/risk-assessment.md:27 — The PR replaces ${CLAUDE_CONFIG_DIR}/skills/... with ${FULLSEND_SKILLS_DIR}/... in the risk-assessment sub-agent (line 27), pr-risk-assessment/SKILL.md (line 207) and the risk-tier1.sh usage comment. Nothing in this repo sets FULLSEND_SKILLS_DIR. A grep of the base checkout finds no match, harness/review.yaml does not supply it, and the current review sandbox does not export it. The runtimes live in fullsend-ai/fullsend, and Machine-checkable sandbox env-var contract (manifest of exported/reserved sandbox variables) fullsend#8465 states that no released runner exports the variable. When it is unset, the Tier 1 command becomes bash "/pr-risk-assessment/scripts/risk-tier1.sh" and fails. Tier 1 risk signals would then be lost on the claude runtime, where the old path works today. Issue pr-risk-assessment skill path is not runtime-neutral #1672 says to remove the KNOWN_GAPS entry only "once resolved", and the triage comment marks the work as blocked on upstream export. The PR also adds the variable to SANDBOX_RUNTIME_VARS, so the reachability test stays green while the runtime is broken. This finding consolidates the prior api-contract finding on the same line with the scope and cross-repo contract concerns.
    Remediation: Hold the merge until a released fullsend runner exports FULLSEND_SKILLS_DIR on claude, codex and pi, and state the minimum fullsend version in the PR. Until then, keep a working fallback such as ${FULLSEND_SKILLS_DIR:-${CLAUDE_CONFIG_DIR}/skills}.

Medium

  • [protected-path] skills/pr-review/sub-agents/risk-assessment.md — This PR modifies protected paths: scripts/skill-env-reachability-test.sh, skills/pr-review/sub-agents/risk-assessment.md, skills/pr-risk-assessment/SKILL.md and skills/pr-risk-assessment/scripts/risk-tier1.sh. The PR links issue pr-risk-assessment skill path is not runtime-neutral #1672 and explains the rationale. Human approval is always required for protected-path changes, regardless of context.

Low

  • [test-weakened] scripts/skill-env-reachability-test.sh:43 — Adding FULLSEND_SKILLS_DIR to the universal SANDBOX_RUNTIME_VARS allowlist means every skill reference to it passes the reachability check, although this PR adds nothing that sets it. Combined with removing the tracked pr-risk-assessment:CLAUDE_CONFIG_DIR KNOWN_GAPS entry, an unresolved dependency becomes an unconditional pass. The script header limits the allowlist to variables injected on every runtime, which is not yet true for this one.
    Remediation: Keep FULLSEND_SKILLS_DIR out of the allowlist and track it in KNOWN_GAPS until the runtimes deliver it.

Earlier findings


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

<!-- fullsend:review-findings-v2:eyJ2ZXJzaW9uIjoyLCJhY3Rpb24iOiJyZXF1ZXN0LWNoYW5nZXMiLCJmaW5kaW5ncyI6W3sic2V2ZXJpdHkiOiJtZWRpdW0iLCJjYXRlZ29yeSI6ImFwaS1jb250cmFjdCIsImZpbGUiOiJza2lsbHMvcHItcmV2aWV3L3N1Yi1hZ2VudHMvcmlzay1hc3Nlc3NtZW50Lm1kIiwiaWQiOiJmX2UxYjFkNzczYjk2NmJiMDAiLCJsaW5lIjoyNywiYWN0aW9uYWJsZSI6ZmFsc2V9LHsic2V2ZXJpdHkiOiJsb3ciLCJjYXRlZ29yeSI6InRlc3Qtd2Vha2VuZWQiLCJmaWxlIjoic2NyaXB0cy9za2lsbC1lbnYtcmVhY2hhYmlsaXR5LXRlc3Quc2giLCJpZCI6ImZfMzdhYzJiNjg5NWQ4NzMyYiIsImxpbmUiOjQzLCJhY3Rpb25hYmxlIjpmYWxzZX1dLCJkaXNwb3NpdGlvbnMiOlt7ImlkIjoiZl9lMWIxZDc3M2I5NjZiYjAwIiwic3RhdHVzIjoib3BlbiJ9XX0= -->

Review

Findings

Medium

  • [api-contract] skills/pr-review/sub-agents/risk-assessment.md:27 — The Tier 1 invocation here and in skills/pr-risk-assessment/SKILL.md:207 now depends on FULLSEND_SKILLS_DIR. Nothing in this repo sets that variable (it appears only in the four changed lines), and it is not set in the current pi review sandbox. If the fullsend runtime in use does not export it yet, the path becomes /pr-risk-assessment/scripts/risk-tier1.sh and Tier 1 metadata collection fails. On the claude runtime that would regress the previous CLAUDE_CONFIG_DIR path. Whether the upstream injection has shipped cannot be checked from this repo. The issue (pr-risk-assessment skill path is not runtime-neutral #1672) asks for this env var, and the other FULLSEND_* variables also come from the external runtime.
    Remediation: Confirm the fullsend release these harnesses target exports FULLSEND_SKILLS_DIR on the claude, codex and pi runtimes, and link the upstream change in the PR description. Otherwise hold the merge until that injection ships.
  • [protected-path] skills/pr-review/sub-agents/risk-assessment.md — This PR modifies protected paths: scripts/skill-env-reachability-test.sh, skills/pr-review/sub-agents/risk-assessment.md, skills/pr-risk-assessment/SKILL.md, skills/pr-risk-assessment/scripts/risk-tier1.sh. The PR links issue pr-risk-assessment skill path is not runtime-neutral #1672 and explains the rationale, but human approval is always required for protected-path changes, regardless of context.

Low

  • [test-weakened] scripts/skill-env-reachability-test.sh:43 — Adding FULLSEND_SKILLS_DIR to SANDBOX_RUNTIME_VARS and removing the pr-risk-assessment:CLAUDE_CONFIG_DIR KNOWN_GAPS entry makes the static reachability check treat the new dependency as reachable without proof. Nothing in this repo can show that the runtime actually injects it, and that is the same unverified dependency as the finding above.
    Remediation: Before merging, confirm the upstream fullsend injection has shipped. If it has not, keep a KNOWN_GAPS entry until it does.

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

<!-- fullsend:review-findings-v2:eyJ2ZXJzaW9uIjoyLCJhY3Rpb24iOiJyZXF1ZXN0LWNoYW5nZXMiLCJmaW5kaW5ncyI6W3sic2V2ZXJpdHkiOiJtZWRpdW0iLCJjYXRlZ29yeSI6ImFwaS1jb250cmFjdCIsImZpbGUiOiJza2lsbHMvcHItcmV2aWV3L3N1Yi1hZ2VudHMvcmlzay1hc3Nlc3NtZW50Lm1kIiwiaWQiOiJmX2UxYjFkNzczYjk2NmJiMDAiLCJsaW5lIjoyNywiYWN0aW9uYWJsZSI6ZmFsc2V9XX0= -->

Review

Findings

Medium

  • [api-contract] skills/pr-review/sub-agents/risk-assessment.md:27 — The consumers are migrated to FULLSEND_SKILLS_DIR before anything supplies it. In the running pi review sandbox, env has PI_CODING_AGENT_DIR=/sandbox/pi-config but no FULLSEND_SKILLS_DIR. Searches of the harnesses, scripts and sandbox configuration found no injection. The exact command in the diff therefore resolves to /pr-risk-assessment/scripts/risk-tier1.sh and fails with exit 127, although the script exists under /sandbox/pi-config/skills. The same applies to skills/pr-risk-assessment/SKILL.md:207. Adding the variable to SANDBOX_RUNTIME_VARS (scripts/skill-env-reachability-test.sh:43) and removing the #1672 KNOWN_GAPS entry only stops the reachability test from flagging it; nothing exports the variable. Issue pr-risk-assessment skill path is not runtime-neutral #1672 proposes FULLSEND_SKILLS_DIR only as a direction ("e.g."). The runtime change would be in fullsend-ai/fullsend, which I could not check. If it is not released, this change also breaks the Claude path that worked through CLAUDE_CONFIG_DIR.
    Remediation: Land and release the fullsend runtime change that exports FULLSEND_SKILLS_DIR=<ConfigDir>/skills on every supported runtime before merging, or keep a working path-resolution mechanism until then. Link that fullsend PR or release in this PR description, and run the invocation in a real sandbox before treating the gap as resolved.

  • [protected-path] skills/pr-risk-assessment/SKILL.md — This PR modifies protected paths: scripts/skill-env-reachability-test.sh, skills/pr-review/sub-agents/risk-assessment.md, skills/pr-risk-assessment/SKILL.md and skills/pr-risk-assessment/scripts/risk-tier1.sh. The PR links pr-risk-assessment skill path is not runtime-neutral #1672 and explains the rationale. Human approval is always required for protected-path changes, regardless of context.


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

Review

Findings

High

  • [breaking-config] skills/pr-review/sub-agents/risk-assessment.md:27 — The Tier 1 invocation here and in skills/pr-risk-assessment/SKILL.md:207 now expands ${FULLSEND_SKILLS_DIR}. Nothing in this repo or diff sets it, and it is unset in the pi review sandbox. Unset, the path becomes /pr-risk-assessment/scripts/risk-tier1.sh, which does not exist, so Tier 1 fails. On claude this replaces a ${CLAUDE_CONFIG_DIR} path that works today. The pr-risk-assessment skill path is not runtime-neutral #1672 triage comment says the migration is blocked until the runtimes export the variable.
    Remediation: Do not merge until the claude, codex and pi runtimes export FULLSEND_SKILLS_DIR, or add a fallback that works on every runtime and fails clearly when empty (e.g. ${FULLSEND_SKILLS_DIR:-${CLAUDE_CONFIG_DIR}/skills} plus a check) in both files.

Medium

  • [test-weakened] scripts/skill-env-reachability-test.sh:43 — Adding FULLSEND_SKILLS_DIR to SANDBOX_RUNTIME_VARS and removing the pr-risk-assessment:CLAUDE_CONFIG_DIR KNOWN_GAPS entry turns a tracked gap into an unconditional pass. That array is for vars injected on every runtime, and nothing shows this one is.
    Remediation: Add the variable only after the runtimes export it; until then keep the KNOWN_GAPS entry for pr-risk-assessment skill path is not runtime-neutral #1672.
  • [protected-path] skills/pr-review/sub-agents/risk-assessment.md:27 — The PR modifies protected paths: scripts/skill-env-reachability-test.sh, skills/pr-review/sub-agents/risk-assessment.md, skills/pr-risk-assessment/SKILL.md, skills/pr-risk-assessment/scripts/risk-tier1.sh. It links pr-risk-assessment skill path is not runtime-neutral #1672 and explains its rationale, but human approval is always required for protected-path changes.

Low

  • [misleading-label] skills/pr-risk-assessment/SKILL.md:207 — The PR body calls this a trivial refactor and says the known gap is resolved, but the test passes only because the variable was added to the always-provided list.
  • [misleading-label] scripts/skill-env-reachability-test.sh:43 — The SANDBOX_RUNTIME_VARS addition is not named in pr-risk-assessment skill path is not runtime-neutral #1672, though it pairs with the authorized KNOWN_GAPS removal. The remaining concern is the unestablished delivery claim, covered by the test-weakened finding.

Earlier findings

  • scripts/skill-env-reachability-test.sh:43, unauthorized-change — reclassified to low (misleading-label): the addition pairs with the KNOWN_GAPS removal that pr-risk-assessment skill path is not runtime-neutral #1672 authorizes.
  • skills/pr-risk-assessment/SKILL.md:207, fail-open — reclassified, remains info and is subsumed by the breaking-config finding.

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

<!-- fullsend:review-findings-v2:eyJ2ZXJzaW9uIjoyLCJhY3Rpb24iOiJyZXF1ZXN0LWNoYW5nZXMiLCJmaW5kaW5ncyI6W3sic2V2ZXJpdHkiOiJoaWdoIiwiY2F0ZWdvcnkiOiJicmVha2luZy1jb25maWciLCJmaWxlIjoic2tpbGxzL3ByLXJldmlldy9zdWItYWdlbnRzL3Jpc2stYXNzZXNzbWVudC5tZCIsImlkIjoiZl82ZWM4NjA5MjMyM2MwMzU0IiwibGluZSI6MjcsImFjdGlvbmFibGUiOnRydWV9LHsic2V2ZXJpdHkiOiJtZWRpdW0iLCJjYXRlZ29yeSI6InRlc3Qtd2Vha2VuZWQiLCJmaWxlIjoic2NyaXB0cy9za2lsbC1lbnYtcmVhY2hhYmlsaXR5LXRlc3Quc2giLCJpZCI6ImZfMDkxYTAzMGRiMWE2MmJjMiIsImxpbmUiOjQzLCJhY3Rpb25hYmxlIjp0cnVlfSx7InNldmVyaXR5IjoibWVkaXVtIiwiY2F0ZWdvcnkiOiJ1bmF1dGhvcml6ZWQtY2hhbmdlIiwiZmlsZSI6InNjcmlwdHMvc2tpbGwtZW52LXJlYWNoYWJpbGl0eS10ZXN0LnNoIiwiaWQiOiJmXzUxYWVjM2MyZWZlZTk0YzAiLCJsaW5lIjo0MywiYWN0aW9uYWJsZSI6ZmFsc2V9LHsic2V2ZXJpdHkiOiJsb3ciLCJjYXRlZ29yeSI6Im1pc2xlYWRpbmctbGFiZWwiLCJmaWxlIjoic2tpbGxzL3ByLXJpc2stYXNzZXNzbWVudC9TS0lMTC5tZCIsImlkIjoiZl8yYzgxZTcyNjU1ZWQ0MDY3IiwibGluZSI6MjA3LCJhY3Rpb25hYmxlIjpmYWxzZX0seyJzZXZlcml0eSI6ImluZm8iLCJjYXRlZ29yeSI6ImZhaWwtb3BlbiIsImZpbGUiOiJza2lsbHMvcHItcmlzay1hc3Nlc3NtZW50L1NLSUxMLm1kIiwiaWQiOiJmXzk2YjczM2ZmODZmYmYxNGEiLCJsaW5lIjoyMDcsImFjdGlvbmFibGUiOmZhbHNlfV0sImRpc3Bvc2l0aW9ucyI6W3siaWQiOiJmXzZlYzg2MDkyMzIzYzAzNTQiLCJzdGF0dXMiOiJvcGVuIn0seyJpZCI6ImZfMDkxYTAzMGRiMWE2MmJjMiIsInN0YXR1cyI6Im9wZW4ifSx7ImlkIjoiZl85NmI3MzNmZjg2ZmJmMTRhIiwic3RhdHVzIjoicmVjbGFzc2lmaWVkIn0seyJpZCI6ImZfMmM4MWU3MjY1NWVkNDA2NyIsInN0YXR1cyI6Im9wZW4ifSx7ImlkIjoiZl81MWFlYzNjMmVmZWU5NGMwIiwic3RhdHVzIjoicmVjbGFzc2lmaWVkIn1dfQ== -->

Review

Findings

High

  • [breaking-config] skills/pr-review/sub-agents/risk-assessment.md:27 — The Tier 1 invocation now depends on FULLSEND_SKILLS_DIR, a contract the fullsend runtimes (claude, codex, pi) must fulfil by exporting it. Nothing in this repo defines or documents it, and the harness image pin cannot supply it. The pi review sandbox for this run has no FULLSEND_SKILLS_DIR (only PI_CODING_AGENT_DIR), and the triage comment on pr-risk-assessment skill path is not runtime-neutral #1672 says the upstream export has not landed. Unset, the command expands to /pr-risk-assessment/scripts/risk-tier1.sh and fails, so no Tier 1 signals are produced. This also breaks the claude runtime, which works today through CLAUDE_CONFIG_DIR. The same dependency appears in skills/pr-risk-assessment/SKILL.md:207 and the risk-tier1.sh header comment. The upstream runtime code was not available to inspect, so universal injection could not be confirmed.
    Remediation: Land and release the FULLSEND_SKILLS_DIR export in all fullsend runtimes first and confirm the harness image includes it. Until then, hold the PR or merge only with a fallback such as SKILLS_DIR="${FULLSEND_SKILLS_DIR:-${CLAUDE_CONFIG_DIR:+$CLAUDE_CONFIG_DIR/skills}}" that fails fast when empty.

Medium

  • [test-weakened] scripts/skill-env-reachability-test.sh:43 — Adding FULLSEND_SKILLS_DIR to SANDBOX_RUNTIME_VARS exempts the variable from the reachability check instead of validating delivery. The array is documented as vars "injected into the sandbox on every runtime", and is_sandbox_runtime_var only checks membership. Base reported the CLAUDE_CONFIG_DIR dependency as a tracked KNOWN_GAPS entry; this PR removes it with no evidence that any runtime injects the new variable.
    Remediation: Do not add FULLSEND_SKILLS_DIR as a universal exemption until all runtimes are shown to inject it. Until then, keep an explicit tracked reachability gap.
  • [unauthorized-change] scripts/skill-env-reachability-test.sh:43 — Issue pr-risk-assessment skill path is not runtime-neutral #1672 says to remove the KNOWN_GAPS entry only "once resolved". The triage comment names the prerequisite as an upstream change adding the variable to all runtimes, and that change is unconfirmed. The PR still edits the allowlist and deletes the gap entry, so the test passes without checking that any runtime exports the variable. The PR body's claim that the gap is "resolved" is unsupported.
    Remediation: Keep the KNOWN_GAPS entry, pointed at the upstream prerequisite, until fullsend exports FULLSEND_SKILLS_DIR on all runtimes, or land this PR only after the upstream change ships.
  • [protected-path] skills/pr-review/sub-agents/risk-assessment.md — This PR modifies protected governance/infrastructure paths: scripts/skill-env-reachability-test.sh, skills/pr-review/sub-agents/risk-assessment.md, skills/pr-risk-assessment/SKILL.md, and skills/pr-risk-assessment/scripts/risk-tier1.sh. It links pr-risk-assessment skill path is not runtime-neutral #1672 and explains the rationale, but human approval is always required for protected-path changes.

Low

  • [misleading-label] skills/pr-risk-assessment/SKILL.md:207 — The title says refactor and the body says "Trivial refactor", but the change is not behavior-preserving. It replaces ${CLAUDE_CONFIG_DIR}/skills/..., which works on claude today, with ${FULLSEND_SKILLS_DIR}/..., which nothing in this repo defines.
    Remediation: Reframe the PR as a change that depends on upstream fullsend exporting FULLSEND_SKILLS_DIR, and link or confirm that change.

Earlier findings

  • skills/pr-risk-assessment/SKILL.md:207, fail-open — reclassified to info: with the variable unset the path is the absolute /pr-risk-assessment/..., not repo-relative, so no repository code is executed. The risk gate in scripts/post-review.sh also downgrades approvals when the assessment is missing or degraded. The availability problem is tracked under the breaking-config finding.
  • scripts/skill-env-reachability-test.sh, scope-exceeded — reclassified to a medium unauthorized-change finding above: the test edits are within the issue's direction but made before its "once resolved" condition is met.

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

<!-- fullsend:review-findings-v2:eyJ2ZXJzaW9uIjoyLCJhY3Rpb24iOiJyZXF1ZXN0LWNoYW5nZXMiLCJmaW5kaW5ncyI6W3sic2V2ZXJpdHkiOiJoaWdoIiwiY2F0ZWdvcnkiOiJicmVha2luZy1jb25maWciLCJmaWxlIjoic2tpbGxzL3ByLXJldmlldy9zdWItYWdlbnRzL3Jpc2stYXNzZXNzbWVudC5tZCIsImlkIjoiZl82ZWM4NjA5MjMyM2MwMzU0IiwibGluZSI6MjcsImFjdGlvbmFibGUiOnRydWV9LHsic2V2ZXJpdHkiOiJtZWRpdW0iLCJjYXRlZ29yeSI6InRlc3Qtd2Vha2VuZWQiLCJmaWxlIjoic2NyaXB0cy9za2lsbC1lbnYtcmVhY2hhYmlsaXR5LXRlc3Quc2giLCJpZCI6ImZfMDkxYTAzMGRiMWE2MmJjMiIsImxpbmUiOjQzLCJhY3Rpb25hYmxlIjp0cnVlfSx7InNldmVyaXR5IjoibG93IiwiY2F0ZWdvcnkiOiJmYWlsLW9wZW4iLCJmaWxlIjoic2tpbGxzL3ByLXJpc2stYXNzZXNzbWVudC9TS0lMTC5tZCIsImlkIjoiZl85NmI3MzNmZjg2ZmJmMTRhIiwibGluZSI6MjA3LCJhY3Rpb25hYmxlIjpmYWxzZX0seyJzZXZlcml0eSI6ImxvdyIsImNhdGVnb3J5IjoibWlzbGVhZGluZy1sYWJlbCIsImZpbGUiOiJza2lsbHMvcHItcmlzay1hc3Nlc3NtZW50L1NLSUxMLm1kIiwiaWQiOiJmXzJjODFlNzI2NTVlZDQwNjciLCJhY3Rpb25hYmxlIjpmYWxzZX0seyJzZXZlcml0eSI6ImluZm8iLCJjYXRlZ29yeSI6InNjb3BlLWV4Y2VlZGVkIiwiZmlsZSI6InNjcmlwdHMvc2tpbGwtZW52LXJlYWNoYWJpbGl0eS10ZXN0LnNoIiwiaWQiOiJmXzUxYWVjM2MyZWZlZTk0YzAiLCJhY3Rpb25hYmxlIjpmYWxzZX1dLCJkaXNwb3NpdGlvbnMiOlt7ImlkIjoiZl82ZWM4NjA5MjMyM2MwMzU0Iiwic3RhdHVzIjoib3BlbiJ9LHsiaWQiOiJmXzA5MWEwMzBkYjFhNjJiYzIiLCJzdGF0dXMiOiJvcGVuIn0seyJpZCI6ImZfOTZiNzMzZmY4NmZiZjE0YSIsInN0YXR1cyI6Im9wZW4ifSx7ImlkIjoiZl81MWFlYzNjMmVmZWU5NGMwIiwic3RhdHVzIjoicmVjbGFzc2lmaWVkIn0seyJpZCI6ImZfMmM4MWU3MjY1NWVkNDA2NyIsInN0YXR1cyI6Im9wZW4ifV19 -->

Review

Findings

High

  • [breaking-config] skills/pr-review/sub-agents/risk-assessment.md:27 — The sub-agent and skills/pr-risk-assessment/SKILL.md:207 now run bash "${FULLSEND_SKILLS_DIR}/pr-risk-assessment/scripts/risk-tier1.sh". Nothing in this repository sets or declares FULLSEND_SKILLS_DIR: no harness env.sandbox block defines it, and the PR links no fullsend runtime release that exports it. The only addition is an entry in the reachability test's SANDBOX_RUNTIME_VARS allowlist, which exempts the variable from checking. It is unset in the live pi review sandbox, where skills are at /sandbox/pi-config/skills/. If it is unset, the path expands to /pr-risk-assessment/scripts/risk-tier1.sh and Tier 1 extraction fails. The risk assessment is then missing or degraded, which forces a comment verdict on every review. On the claude runtime this also regresses a path that CLAUDE_CONFIG_DIR previously resolved. Severity stays at the prior high: a Tier 1 failure degrades the score and trips the risk-verdict gate rather than failing gracefully.
    Remediation: Confirm that the fullsend runtime injects FULLSEND_SKILLS_DIR on every runtime (claude, codex, pi) and link the fullsend PR or release. Alternatively, define the variable in the harness env.sandbox blocks, or fall back to CLAUDE_CONFIG_DIR, CODEX_HOME or PI_CODING_AGENT_DIR when it is unset. Keep the CLAUDE_CONFIG_DIR reference and its KNOWN_GAPS entry until the contract ships.

Medium

  • [test-weakened] scripts/skill-env-reachability-test.sh:43 — Adding FULLSEND_SKILLS_DIR to SANDBOX_RUNTIME_VARS exempts it from reachability validation everywhere. The PR also removes the tracked pr-risk-assessment:CLAUDE_CONFIG_DIR KNOWN_GAPS entry (pr-risk-assessment skill path is not runtime-neutral #1672). Together these make the test pass although nothing in this repo supplies the variable. The comment above the allowlist limits it to vars "injected into the sandbox on every runtime" and says unresolved runtime paths belong in KNOWN_GAPS; the PR gives no evidence that this variable meets that bar.
    Remediation: Keep FULLSEND_SKILLS_DIR out of the universal allowlist and keep a KNOWN_GAPS entry tracking pr-risk-assessment skill path is not runtime-neutral #1672 until runtime injection is confirmed across supported runtimes.
  • [protected-path] skills/pr-review/sub-agents/risk-assessment.md — The PR modifies protected paths: scripts/skill-env-reachability-test.sh, skills/pr-review/sub-agents/risk-assessment.md, skills/pr-risk-assessment/SKILL.md and skills/pr-risk-assessment/scripts/risk-tier1.sh. It links pr-risk-assessment skill path is not runtime-neutral #1672 and explains the rationale, but human approval is always required for protected-path changes, regardless of context.

Low

  • [fail-open] skills/pr-risk-assessment/SKILL.md:207 — The invocation has no guard for an unset FULLSEND_SKILLS_DIR. The path silently resolves to the filesystem root, and the skill's tier-redistribution rules then degrade the score instead of stopping.
  • [misleading-label] skills/pr-risk-assessment/SKILL.md — The PR title and body present this as a "trivial refactor" that resolves the known gap, but the producer of FULLSEND_SKILLS_DIR is outside this repo and unconfirmed. The change is not a self-contained runtime-neutral migration.

Earlier findings


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

<!-- fullsend:review-findings-v2:eyJ2ZXJzaW9uIjoyLCJhY3Rpb24iOiJyZXF1ZXN0LWNoYW5nZXMiLCJmaW5kaW5ncyI6W3sic2V2ZXJpdHkiOiJoaWdoIiwiY2F0ZWdvcnkiOiJicmVha2luZy1jb25maWciLCJmaWxlIjoic2tpbGxzL3ByLXJldmlldy9zdWItYWdlbnRzL3Jpc2stYXNzZXNzbWVudC5tZCIsImlkIjoiZl82ZWM4NjA5MjMyM2MwMzU0IiwibGluZSI6MjcsImFjdGlvbmFibGUiOnRydWV9LHsic2V2ZXJpdHkiOiJtZWRpdW0iLCJjYXRlZ29yeSI6InRlc3Qtd2Vha2VuZWQiLCJmaWxlIjoic2NyaXB0cy9za2lsbC1lbnYtcmVhY2hhYmlsaXR5LXRlc3Quc2giLCJpZCI6ImZfMDkxYTAzMGRiMWE2MmJjMiIsImxpbmUiOjQzLCJhY3Rpb25hYmxlIjp0cnVlfSx7InNldmVyaXR5IjoibG93IiwiY2F0ZWdvcnkiOiJmYWlsLW9wZW4iLCJmaWxlIjoic2tpbGxzL3ByLXJpc2stYXNzZXNzbWVudC9TS0lMTC5tZCIsImlkIjoiZl85NmI3MzNmZjg2ZmJmMTRhIiwibGluZSI6MjA3LCJhY3Rpb25hYmxlIjpmYWxzZX0seyJzZXZlcml0eSI6ImhpZ2giLCJjYXRlZ29yeSI6InNjb3BlLWV4Y2VlZGVkIiwiZmlsZSI6InNjcmlwdHMvc2tpbGwtZW52LXJlYWNoYWJpbGl0eS10ZXN0LnNoIiwiaWQiOiJmXzUxYWVjM2MyZWZlZTk0YzAiLCJhY3Rpb25hYmxlIjpmYWxzZX0seyJzZXZlcml0eSI6ImxvdyIsImNhdGVnb3J5IjoibWlzbGVhZGluZy1sYWJlbCIsImZpbGUiOiJza2lsbHMvcHItcmlzay1hc3Nlc3NtZW50L1NLSUxMLm1kIiwiaWQiOiJmXzJjODFlNzI2NTVlZDQwNjciLCJhY3Rpb25hYmxlIjpmYWxzZX1dLCJkaXNwb3NpdGlvbnMiOlt7ImlkIjoiZl82ZWM4NjA5MjMyM2MwMzU0Iiwic3RhdHVzIjoib3BlbiJ9LHsiaWQiOiJmXzA5MWEwMzBkYjFhNjJiYzIiLCJzdGF0dXMiOiJvcGVuIn0seyJpZCI6ImZfOTZiNzMzZmY4NmZiZjE0YSIsInN0YXR1cyI6Im9wZW4ifV19 -->

Review

Findings

High

  • [breaking-config] skills/pr-review/sub-agents/risk-assessment.md:27 — ${CLAUDE_CONFIG_DIR}/skills/... is replaced with ${FULLSEND_SKILLS_DIR}/..., but nothing in this repo defines or exports FULLSEND_SKILLS_DIR. A code search of fullsend-ai/fullsend finds no match, and this review sandbox's environment does not contain it. Issue pr-risk-assessment skill path is not runtime-neutral #1672 lists injecting the variable from every runtime as a proposed first step, not as something that already exists. Where it is unset, the command expands to bash "/pr-risk-assessment/scripts/risk-tier1.sh" and fails with "No such file", so Tier 1 risk metadata is lost or the risk-assessment sub-agent fails. This also removes the previously working claude-runtime path with no fallback. The identical invocation at skills/pr-risk-assessment/SKILL.md:207 has the same problem.
    Remediation: Land and release the fullsend runtime change that injects FULLSEND_SKILLS_DIR on claude, codex and pi first, and gate this PR on that version. Until then, add a fallback in both invocations (risk-assessment.md:27 and SKILL.md:207), for example SK="${FULLSEND_SKILLS_DIR:-${CLAUDE_CONFIG_DIR:+$CLAUDE_CONFIG_DIR/skills}}", with an explicit error if it is empty. Keep the KNOWN_GAPS entry until the runtime provides the variable.

  • [scope-exceeded] scripts/skill-env-reachability-test.sh — Issue pr-risk-assessment skill path is not runtime-neutral #1672 authorizes the migration and the KNOWN_GAPS removal only after step 1, in which all runtimes inject FULLSEND_SKILLS_DIR. That step lives outside this repo and has no producer anywhere found. The PR declares the variable "injected on every runtime" and marks the gap resolved, which it is not. In practice the scripts resolve to an empty path on every runtime, including claude, where the old variable worked. See also the breaking-config and test-weakened findings.
    Remediation: Hold this PR until fullsend injects FULLSEND_SKILLS_DIR on all runtimes, or keep the KNOWN_GAPS entry and use a ${FULLSEND_SKILLS_DIR:-${CLAUDE_CONFIG_DIR}/skills} style fallback in the meantime.

Medium

  • [test-weakened] scripts/skill-env-reachability-test.sh:43 — Adding FULLSEND_SKILLS_DIR to SANDBOX_RUNTIME_VARS exempts every reference to it from reachability validation, although the variable is not injected by any runtime verified here. Together with removing the pr-risk-assessment:CLAUDE_CONFIG_DIR KNOWN_GAPS entry, this turns an acknowledged unresolved dependency into a passing check without making the variable reachable.
    Remediation: Keep the unresolved dependency tracked in KNOWN_GAPS (retargeted to FULLSEND_SKILLS_DIR) and do not list it in SANDBOX_RUNTIME_VARS until runtime injection is implemented and verified.

  • [protected-path] scripts/skill-env-reachability-test.sh — The PR modifies protected paths: scripts/skill-env-reachability-test.sh, skills/pr-review/sub-agents/risk-assessment.md, skills/pr-risk-assessment/SKILL.md and skills/pr-risk-assessment/scripts/risk-tier1.sh. The PR links pr-risk-assessment skill path is not runtime-neutral #1672 and explains its rationale, but human approval is always required for protected-path changes regardless of context.

Low

  • [fail-open] skills/pr-risk-assessment/SKILL.md:207 — The Tier 1 invocation depends on an environment variable that may be unset. When it is, the path resolves to a root-level /pr-risk-assessment/scripts/risk-tier1.sh. The expected result is a missing-file failure, so risk scoring degrades. No access gate is involved, and no way for a PR author to plant a script at that path was found.

  • [misleading-label] skills/pr-risk-assessment/SKILL.md — The title ("refactor") and "Closes pr-risk-assessment skill path is not runtime-neutral #1672" overstate the change. The diff changes a runtime path contract that depends on external support that does not exist yet. It is not a behavior-preserving rename, and the issue's first step is unmet.
    Remediation: Retitle as a dependent change and reference pr-risk-assessment skill path is not runtime-neutral #1672 without closing it until the fullsend-side injection lands.


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

<!-- fullsend:review-findings-v2:eyJ2ZXJzaW9uIjoyLCJhY3Rpb24iOiJyZXF1ZXN0LWNoYW5nZXMiLCJmaW5kaW5ncyI6W3sic2V2ZXJpdHkiOiJoaWdoIiwiY2F0ZWdvcnkiOiJicmVha2luZy1jb25maWciLCJmaWxlIjoic2tpbGxzL3ByLXJldmlldy9zdWItYWdlbnRzL3Jpc2stYXNzZXNzbWVudC5tZCIsImlkIjoiZl82ZWM4NjA5MjMyM2MwMzU0IiwibGluZSI6MjcsImFjdGlvbmFibGUiOnRydWV9LHsic2V2ZXJpdHkiOiJtZWRpdW0iLCJjYXRlZ29yeSI6InRlc3Qtd2Vha2VuZWQiLCJmaWxlIjoic2NyaXB0cy9za2lsbC1lbnYtcmVhY2hhYmlsaXR5LXRlc3Quc2giLCJpZCI6ImZfMDkxYTAzMGRiMWE2MmJjMiIsImxpbmUiOjQzLCJhY3Rpb25hYmxlIjp0cnVlfSx7InNldmVyaXR5IjoibG93IiwiY2F0ZWdvcnkiOiJmYWlsLW9wZW4iLCJmaWxlIjoic2tpbGxzL3ByLXJpc2stYXNzZXNzbWVudC9TS0lMTC5tZCIsImlkIjoiZl85NmI3MzNmZjg2ZmJmMTRhIiwibGluZSI6MjA3LCJhY3Rpb25hYmxlIjpmYWxzZX1dLCJkaXNwb3NpdGlvbnMiOlt7ImlkIjoiZl82ZWM4NjA5MjMyM2MwMzU0Iiwic3RhdHVzIjoib3BlbiJ9LHsiaWQiOiJmXzA5MWEwMzBkYjFhNjJiYzIiLCJzdGF0dXMiOiJvcGVuIn1dfQ== -->

Review

Findings

High

  • [breaking-config] skills/pr-review/sub-agents/risk-assessment.md:27 — The PR replaces ${CLAUDE_CONFIG_DIR}/skills/... with ${FULLSEND_SKILLS_DIR}/... in risk-assessment.md, pr-risk-assessment/SKILL.md and the risk-tier1.sh header, with no fallback. Nothing in this repo sets or documents FULLSEND_SKILLS_DIR, and the PR links no fullsend change that exports it; issue pr-risk-assessment skill path is not runtime-neutral #1672 treats runtime injection as a separate step this PR does not do. The variable is also unset in the pi runtime sandbox this review ran in. On a runtime that does not export it, the command expands to /pr-risk-assessment/scripts/risk-tier1.sh and Tier 1 fails. That includes claude, where the old path worked. The PR also adds the variable to SANDBOX_RUNTIME_VARS and removes the KNOWN_GAPS entry, so the reachability test now treats it as guaranteed. No minimum fullsend version is recorded for @v0 consumers, and Closes #1672 closes the issue although its runtime-injection requirement is not met. The rebase onto main did not change the patch.
    Remediation: Gate the merge on a released fullsend change that exports FULLSEND_SKILLS_DIR on every runtime, link it, and note the minimum fullsend version. Until then keep a fallback such as ${FULLSEND_SKILLS_DIR:-${CLAUDE_CONFIG_DIR}/skills} and use Refs #1672 instead of Closes #1672.

Medium

  • [test-weakened] scripts/skill-env-reachability-test.sh:43 — Adding FULLSEND_SKILLS_DIR to SANDBOX_RUNTIME_VARS exempts every reference to it from reachability validation without establishing that any runtime supplies it. Together with removing the pr-risk-assessment:CLAUDE_CONFIG_DIR KNOWN_GAPS entry, the test now silently accepts the replacement dependency even though the command can resolve to /pr-risk-assessment/scripts/risk-tier1.sh. The file's header says runtime-specific variables must not be listed there and should be tracked in KNOWN_GAPS instead.
    Remediation: Keep FULLSEND_SKILLS_DIR out of SANDBOX_RUNTIME_VARS until runtime injection is implemented and verified, and track pr-risk-assessment:FULLSEND_SKILLS_DIR in KNOWN_GAPS (referencing pr-risk-assessment skill path is not runtime-neutral #1672) in the meantime.
  • [protected-path] skills/pr-review/sub-agents/risk-assessment.md — This PR modifies protected governance/infrastructure paths: scripts/skill-env-reachability-test.sh, skills/pr-review/sub-agents/risk-assessment.md, skills/pr-risk-assessment/SKILL.md, skills/pr-risk-assessment/scripts/risk-tier1.sh. The PR links pr-risk-assessment skill path is not runtime-neutral #1672 and explains the rationale, but human approval is always required for protected-path changes, regardless of context.

Low

  • [fail-open] skills/pr-risk-assessment/SKILL.md:207 — When FULLSEND_SKILLS_DIR is unset or empty, the invocation resolves to the root-relative /pr-risk-assessment/scripts/risk-tier1.sh instead of failing fast with a clear error. The same unguarded invocation is in skills/pr-review/sub-agents/risk-assessment.md. Exploiting it would need write access to the sandbox filesystem root, which is unverified and unlikely, and the removed CLAUDE_CONFIG_DIR form had the same empty-expansion behaviour, so the realistic impact is an unclear failure.
    Remediation: Use ${FULLSEND_SKILLS_DIR:?FULLSEND_SKILLS_DIR is required} in both invocations.

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

<!-- fullsend:review-findings-v2:eyJ2ZXJzaW9uIjoyLCJhY3Rpb24iOiJyZXF1ZXN0LWNoYW5nZXMiLCJmaW5kaW5ncyI6W3sic2V2ZXJpdHkiOiJoaWdoIiwiY2F0ZWdvcnkiOiJicmVha2luZy1jb25maWciLCJmaWxlIjoic2tpbGxzL3ByLXJldmlldy9zdWItYWdlbnRzL3Jpc2stYXNzZXNzbWVudC5tZCIsImlkIjoiZl82ZWM4NjA5MjMyM2MwMzU0IiwibGluZSI6MjcsImFjdGlvbmFibGUiOnRydWV9LHsic2V2ZXJpdHkiOiJtZWRpdW0iLCJjYXRlZ29yeSI6InRlc3Qtd2Vha2VuZWQiLCJmaWxlIjoic2NyaXB0cy9za2lsbC1lbnYtcmVhY2hhYmlsaXR5LXRlc3Quc2giLCJpZCI6ImZfMDkxYTAzMGRiMWE2MmJjMiIsImxpbmUiOjQzLCJhY3Rpb25hYmxlIjp0cnVlfV0sImRpc3Bvc2l0aW9ucyI6W3siaWQiOiJmXzZlYzg2MDkyMzIzYzAzNTQiLCJzdGF0dXMiOiJvcGVuIn0seyJpZCI6ImZfMDkxYTAzMGRiMWE2MmJjMiIsInN0YXR1cyI6Im9wZW4ifV19 -->

Review

Findings

High

  • [breaking-config] skills/pr-review/sub-agents/risk-assessment.md:27 — Both Tier 1 commands (here and skills/pr-risk-assessment/SKILL.md:207) now depend on the fullsend runtime exporting FULLSEND_SKILLS_DIR into the sandbox. Nothing in this repo (harness env blocks, skills) or in the review sandbox's pi-config sets the variable, and the PR does not show a producer; issue pr-risk-assessment skill path is not runtime-neutral #1672 describes runtime injection as a proposal that lives in the fullsend repo. If the variable is unset, the path expands to /pr-risk-assessment/scripts/risk-tier1.sh and Tier 1 extraction fails with no fallback. That regresses the claude runtime, where ${CLAUDE_CONFIG_DIR}/skills/... worked; pi/codex were already broken. The PR also says "Closes pr-risk-assessment skill path is not runtime-neutral #1672" while delivering only the consumer side, so the tracking issue would close before the dependency exists.
    Remediation: Before merging, link a released fullsend version that exports FULLSEND_SKILLS_DIR for the claude, codex and pi runtimes and document it as the minimum version; otherwise sequence this PR after the fullsend change, or fail fast with a clear error when the variable is unset. Change "Closes pr-risk-assessment skill path is not runtime-neutral #1672" to a partial reference until runtime injection lands.

Medium

  • [test-weakened] scripts/skill-env-reachability-test.sh:43 — SANDBOX_RUNTIME_VARS is an unconditional exemption: any skill reference to a listed variable skips the harness reachability check; it does not verify that the runtime injects the variable. This PR adds FULLSEND_SKILLS_DIR to the list and removes the KNOWN_GAPS entry pr-risk-assessment:CLAUDE_CONFIG_DIR (pr-risk-assessment skill path is not runtime-neutral #1672), so the result changes from a tracked gap to PASS without evidence that the variable is provided. If it is not injected, CI stays green while the risk step fails at runtime.
    Remediation: Keep the dependency tracked in KNOWN_GAPS (re-keyed to FULLSEND_SKILLS_DIR) until a fullsend release that exports it is confirmed, then move it to SANDBOX_RUNTIME_VARS.
  • [protected-path] scripts/skill-env-reachability-test.sh — The PR modifies protected paths: scripts/skill-env-reachability-test.sh, skills/pr-review/sub-agents/risk-assessment.md, skills/pr-risk-assessment/SKILL.md, skills/pr-risk-assessment/scripts/risk-tier1.sh. The PR links issue pr-risk-assessment skill path is not runtime-neutral #1672 and explains the rationale, but human approval is always required for protected-path changes regardless of context.

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-review

fullsend-ai-review Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 9:51 PM UTC · Completed 9:59 PM UTC

Commit: d3876a1 · View workflow run →

Runtime: pi · Model: sonnet → claude-sonnet-5-5 · Effort: high · Cost: $1.59

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review

fullsend-ai-review Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 9:15 PM UTC · Completed 9:23 PM UTC

Commit: 6e369e5 · View workflow run →

Runtime: pi · Model: sonnet → claude-sonnet-5-5 · Effort: high · Cost: $1.33

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review

fullsend-ai-review Bot commented Oct 17, 2026 •

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 5:16 PM UTC · Completed 5:25 PM UTC

Commit: 4a52d13 · View workflow run →

Runtime: pi · Model: sonnet → claude-sonnet-5-5 · Effort: high · Cost: $1.40

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review

fullsend-ai-review Bot commented Oct 19, 2026 •

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 8:00 PM UTC · Completed 8:08 PM UTC

Commit: e4bc3ba · View workflow run →

Runtime: pi · Model: sonnet → claude-sonnet-5-5 · Effort: high · Cost: $0.94

@fullsend-ai-review
fullsend-ai-review Bot dismissed stale reviews from themself October 19, 2026 20:08

Superseded by updated review

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review

fullsend-ai-review Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 1:50 AM UTC · Completed 1:58 AM UTC

Commit: e67fd16 · View workflow run →

Runtime: pi · Model: sonnet → claude-sonnet-5-5 · Effort: high · Cost: $1.51

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review

fullsend-ai-review Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 5:51 PM UTC · Completed 6:00 PM UTC

Commit: 4f0f5a8 · View workflow run →

Runtime: pi · Model: sonnet → claude-sonnet-5-5 · Effort: high · Cost: $1.94

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review

fullsend-ai-review Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 9:18 PM UTC · Completed 9:38 PM UTC

Commit: c17ba75 · View workflow run →

Runtime: pi · Model: sonnet → claude-sonnet-5-5 · Effort: high · Cost: $1.61

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review

fullsend-ai-review Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 11:59 PM UTC · Completed 12:06 AM UTC

Commit: c27507d · View workflow run →

Runtime: pi · Model: sonnet → claude-sonnet-5-5 · Effort: high · Cost: $1.45

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review

fullsend-ai-review Bot commented Oct 24, 2026 •

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 4:26 AM UTC · Completed 4:37 AM UTC

Commit: 374faa6 · View workflow run →

Runtime: pi · Model: sonnet → claude-sonnet-5-5 · Effort: high · Cost: $2.07

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review

fullsend-ai-review Bot commented Oct 24, 2026 •

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 4:53 PM UTC · Completed 5:01 PM UTC

Commit: e232f6b · View workflow run →

Runtime: pi · Model: sonnet → claude-sonnet-5-5 · Effort: high · Cost: $1.40

fullsend-ai-review[bot]

This comment was marked as outdated.

…o FULLSEND_SKILLS_DIR

Replace runtime-specific CLAUDE_CONFIG_DIR path references with the
runtime-neutral FULLSEND_SKILLS_DIR env var so pr-risk-assessment
scripts work on any supported runtime, not just Claude.

- Update script invocation path in SKILL.md and risk-assessment.md
- Update header comment in risk-tier1.sh
- Add FULLSEND_SKILLS_DIR to SANDBOX_RUNTIME_VARS in reachability test
- Remove resolved KNOWN_GAPS entry for pr-risk-assessment:CLAUDE_CONFIG_DIR

AI-Assisted: true
Assisted-by: claude-opus-4-6
Generated with AI assistance (claude-opus-4-6)

Signed-off-by: Jay Flowers <jay.flowers@gmail.com>
@jflowers
jflowers force-pushed the opsx/runtime-neutral-skill-path branch from e232f6b to 51b4a2e Compare October 25, 2026 00:09
@fullsend-ai-review

fullsend-ai-review Bot commented Oct 25, 2026 •

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 12:11 AM UTC · Completed 12:18 AM UTC

Commit: 51b4a2e · View workflow run →

Runtime: pi · Model: sonnet → claude-sonnet-5-5 · Effort: high · Cost: $1.41

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

1. Run the Tier 1 metadata script:
```bash
bash "${CLAUDE_CONFIG_DIR}/skills/pr-risk-assessment/scripts/risk-tier1.sh"
bash "${FULLSEND_SKILLS_DIR}/pr-risk-assessment/scripts/risk-tier1.sh"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[high] breaking-config

The Tier 1 invocation now reads ${FULLSEND_SKILLS_DIR}, but nothing in this repo sets or exports it and the pi review sandbox does not set it either. With it unset the path expands to /pr-risk-assessment/scripts/risk-tier1.sh, which does not exist, so the Tier 1 script fails (exit 127). The identical invocation in skills/pr-risk-assessment/SKILL.md has the same problem. Issue #1672 triage says the migration is blocked until upstream fullsend runtimes export the variable; CLAUDE_CONFIG_DIR works today on the claude runtime. The PR also adds FULLSEND_SKILLS_DIR to SANDBOX_RUNTIME_VARS and removes the KNOWN_GAPS entry, so the test treats the variable as runtime-provided instead of verifying it.

Suggested fix: Do not merge until upstream fullsend exports FULLSEND_SKILLS_DIR from all runtimes and that is confirmed, or use a working fallback such as ${FULLSEND_SKILLS_DIR:-${CLAUDE_CONFIG_DIR:-${PI_CODING_AGENT_DIR:-$CODEX_HOME}}/skills}. Keep the KNOWN_GAPS entry until the upstream change lands.

FULLSEND_OUTPUT_DIR
FULLSEND_OUTPUT_SCHEMA
FULLSEND_ROLE
FULLSEND_SKILLS_DIR

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] test-weakened

Adding FULLSEND_SKILLS_DIR to SANDBOX_RUNTIME_VARS exempts it from reachability validation even though it is absent from the current sandbox and has no producer in this repository. Together with deleting the known-gap entry, this suppresses the unresolved skill-path dependency instead of verifying that runtime injection exists.

Suggested fix: Keep the unresolved FULLSEND_SKILLS_DIR dependency tracked in KNOWN_GAPS rather than the universal runtime allowlist until injection across supported runtimes is verified.

This branch was successfully deployed

1 active deployment
dev — 51b4a2e5 Deployed Oct 25, 2026 by jflowers via behaviour #1281
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk/moderate PR risk: moderate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

pr-risk-assessment skill path is not runtime-neutral

1 participant