Experiment: target repo skills in triage - #10
Conversation
…experiment Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
HTTPS URLs fail without interactive auth in automated runs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Control and treatment runs completed successfully. Target repo skills ARE discovered and used by the triage agent in sandboxes. Results sanitized to redact secrets. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…y labels The agent references skill-defined labels in comment text but post-triage.sh only assigns workflow labels (ready-to-code, etc.). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review: #10
Head SHA: 3eee7fd
Timestamp: 2026-04-29T09:22:00Z
Outcome: comment-only
Summary
This PR adds a self-contained A/B experiment testing whether target repository .claude/skills/ are discovered by the fullsend triage agent inside OpenShell sandboxes. All 17 files are new additions (1751 lines, 0 deletions) under the target-repo-skills/ directory. The experiment is well-designed with clear methodology, controlled variables, and sanitized results. No critical or high findings were identified. A few medium/low observations are noted below for consideration.
Findings
Medium
-
[Correctness]
target-repo-skills/setup-target-repo.sh:48— Thetrapstatement uses unquoted variable expansion:trap "rm -rf ${WORK_DIR}" EXIT. IfWORK_DIRwere to contain spaces or special characters (unlikely withmktemp -dbut possible), this could behave unexpectedly. Safer:trap 'rm -rf "${WORK_DIR}"' EXIT— but note that single-quoting prevents variable expansion at trap-set time, so the pattern used here (double-quoted, expanded at set time) is actually correct for this use case. However,run.shdoes not use a trap for its temp dirs (repo_dir,run_output_dir,SKILL_WORK_DIR), relying only on explicitrm -rf. If the script exits early (e.g.,set -etriggers), temp dirs are leaked.
Remediation: Add trap-based cleanup inrun.shfor temp directories, or use a single top-level temp dir with a trap. -
[Correctness]
target-repo-skills/setup-target-repo.sh:53-55— Thecp -roftarget-repo-files/*followed by a separatemkdir -p handlers && cp -r handlers/*is redundant. The firstcp -ralready copieshandlers/recursively. The extra copy is harmless but unnecessary.
Remediation: Remove lines 53-55 (themkdir -pand conditionalcpof handlers).
Low
-
[Style/conventions] Multiple files are missing a trailing newline (
go.mod,health.go,users.go,main.go,SKILL.md,README.md,HOW_TO.md,setup-target-repo.sh,run.sh). While not functionally impactful, POSIX convention expects text files to end with a newline, and some tools may warn.
Remediation: Add trailing newlines to all text files. -
[Correctness]
target-repo-skills/setup-target-repo.sh:37— The label parsing usesIFS=: read -r prefix name color <<< "${entry}"where entries like"area:api:#0e8a16"are split on:. This producesprefix=area,name=api,color=#0e8a16. Thecolorfield then has its#stripped with${color#\#}. This works correctly for the current label format but would break if a label name itself contained a colon.
Remediation: Current labels don't contain extra colons, so this is fine as-is. Just noting the fragility.
Info
-
[Intent alignment] The PR has no linked issue, which is expected for an experiment. The scope is appropriate — it adds experiment infrastructure and results without modifying any existing code or production paths.
-
[Platform security] The
run.shscript sources.experiment-state(line 17), which is a file generated bysetup-target-repo.sh. This is safe in the experiment context since the user controls both scripts. The.experiment-statefile is correctly gitignored. -
[Content security] The design doc (line 199-203) correctly identifies that target repo skills are user-controlled content and flags prompt injection risk as a follow-up concern. The experiment itself does not introduce this risk — it merely measures existing behavior.
-
[Injection defense] PR body, commit messages, and code comments were inspected. No prompt injection patterns, non-rendering Unicode, or bidirectional override characters detected. The PR description accurately reflects the diff contents.
-
[Platform security] The sanitize function in
run.sh(lines 57-80) redacts GH tokens, GCP project IDs, credential paths, OAuth tokens, and service account emails from result files before they can be committed. The committed result files (results/control/andresults/treatment/) show no residual secrets — the transcripts contain only session IDs, tool calls, and triage output.
Footer
Outcome: comment-only
This review applies to SHA 3eee7fd856db806d247b565d3a461567c22ea377. Any push to the PR head clears this review and requires a new evaluation.
waynesun09
left a comment
waynesun09
left a comment
There was a problem hiding this comment.
Multi-Agent Review — 4 agents (security, quality, cursor, gemini)
Overall: Well-designed A/B experiment with sound methodology and clear results. No secrets found committed. Approve with fixes for the issues below.
Summary of inline comments (9 findings, medium+)
| # | Severity | File | Finding |
|---|---|---|---|
| 1 | High | run.sh:64 |
sed -i not portable to macOS — sanitization silently fails, risking secret leakage |
| 2 | High | run.sh:57-68 |
Incomplete sanitization patterns — missing ghs_, gho_, github_pat_, IP addresses, Vertex IDs |
| 3 | High | run.sh:25 |
source of unvalidated state file is arbitrary code execution vector |
| 4 | Medium | run.sh:95-101 |
No cleanup trap for temp dirs — unsanitized secrets may persist on failure |
| 5 | Medium | run.sh:106-110 |
|| true silently swallows fullsend failures |
| 6 | Medium | run.sh:96 |
SSH clone URLs not portable for HTTPS-only auth users |
| 7 | Medium | run.sh:104 |
export GH_TOKEN leaks token to all child processes |
| 8 | Medium | setup-target-repo.sh:53 |
--force push to main without safety check |
| 9 | Medium | setup-target-repo.sh:41 |
Trap uses double quotes — fragile variable expansion |
Strengths
- Rigorous A/B design with clear hypothesis, controls, and measurable outcomes
- Proactive secret sanitization before committing results
- Excellent documentation (design spec, HOW_TO, README with analysis)
set -euo pipefailin both scripts- Clean
.experiment-statepattern for cross-script state - Design doc proactively addresses prompt injection risks from target repo skills
- Replace `source` of state file with safe key-value parsing - Use portable `sed > tmp && mv` instead of `sed -i` - Add missing sanitization patterns (ghs_, gho_, github_pat_, IPs) - Add cleanup trap in run_triage for temp dirs - Log fullsend exit code instead of silent `|| true` - Scope GH_TOKEN and GITHUB_ISSUE_URL to fullsend command - Fix trap quoting to use single quotes in setup script - Remove redundant handlers copy in setup script - Re-run experiment to validate changes Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Review: #10Head SHA: f602212 SummaryThis PR adds a well-structured A/B experiment testing whether target repository FindingsInfo
FooterOutcome: approve Previous runReview: #10Head SHA: ed298f0 SummaryThis PR adds a well-structured A/B experiment testing whether Claude Code discovers FindingsMedium
Low
Info
FooterOutcome: comment-only |
|
/fix |
Restore the original experiment result files that match the README's behavioral analysis — the re-run produced "duplicate" results because issue fullsend-ai#2 was identical to fullsend-ai#1. Add trailing newlines to all text files. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
3 participants
Summary
.claude/skills/are discovered and used by the fullsend triage agent inside OpenShell sandboxes, despiteCLAUDE_CONFIG_DIRbeing overridden to/tmp/claude-configCLAUDE_CONFIG_DIRKey findings
triage-guidance(0 in control)post-triage.shonly applies workflow labels (ready-to-code), not the taxonomy labels from the skill — noted as a potential pipeline improvementWhat's included
setup-target-repo.shandrun.shautomation scriptsTriage agent output visible at: maruiz93/experiment-target-repo-skills#1
Test plan
setup-target-repo.shcreates synthetic repo, labels, and issuerun.shexecutes control and treatment runs with secret sanitizationtriage-guidancetriage-guidance🤖 Generated with Claude Code