fix(#489): replace network-dependent git commands in skill - #591
Conversation
|
🤖 Finished Review · ✅ Success · Started 7:32 AM UTC · Completed 7:47 AM UTC |
ReviewFindingsMedium
Low
Previous runReviewFindingsHigh
Medium
Next steps:
Previous run (2)ReviewFindingsMedium
Labels: PR modifies a skill definition file (skills/code-implementation/SKILL.md) and was authored by the code agent bot |
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
fullsend-ai-review
Bot
added
requires-manual-review
|
/fs-fix rebase and address feedback |
|
🤖 Finished Fix · ❌ Failure · Started 9:01 AM UTC · Completed 9:06 AM UTC |
|
The fix agent completed, but the post-fix script failed before finishing. Workflow run: https://github.com/fullsend-ai/.fullsend/actions/runs/30618287563 Details: |
|
/fs-fix rebase, address feedback do not sign-off-by the commit, otherwise the post-fix script will reject your contribution. |
|
🤖 Finished Fix · ❌ Failure · Started 7:08 AM UTC · Completed 7:13 AM UTC |
|
The fix agent completed, but the post-fix script failed before finishing. Workflow run: https://github.com/fullsend-ai/.fullsend/actions/runs/30792497738 Details: |
|
/fs-fix rebase, address feedback do not sign-off-by the commit, otherwise the post-fix script will reject your contribution. |
|
🤖 Finished Fix · ❌ Failure · Started 9:32 AM UTC · Completed 9:36 AM UTC |
|
The fix agent completed, but the post-fix script failed before finishing. Workflow run: https://github.com/fullsend-ai/.fullsend/actions/runs/30801784467 Details: |
|
/fs-fix rebase, address feedback do not sign-off-by the commit, otherwise the post-fix script will reject your contribution. |
|
🤖 Finished Fix · ✅ Success · Started 7:23 AM UTC · Completed 7:29 AM UTC |
The code-implementation skill instructed agents to run `git fetch origin` and diff against `origin/<target-branch>`, both of which require git protocol access that the sandbox network policy blocks. Branch creation (step 5): removed `git fetch origin` and `origin/<target-branch>` from `git checkout -b`. The sandbox checks out the default branch at its latest commit, so HEAD is already the correct base. Changed-file discovery (step 9c): replaced `git diff --name-only origin/<target-branch>` with `git diff --name-only HEAD`. Origin refs may not be available without a successful fetch. Note: pre-commit could not run in-sandbox (exit 3, network error fetching hook environments). The post-script runs pre-commit authoritatively on the runner. Closes #489
Step 4's existing-branch handling used origin/<target> in git log and git diff commands. These fail in the sandbox because the network policy blocks git protocol access to github.com, so origin refs may not be available. Replace origin/<target> with the local <target-branch> ref discovered in step 3, consistent with the approach already used in steps 5 and 9c. Addresses review feedback on #591
🔧 Fix agent — iteration 1 (human-triggered)Rebased branch on main. Fixed stale origin/ references in step 4 (lines 269-270) that would fail in the sandbox. Added explanatory note matching the pattern already used in steps 5 and 9c. The protected-path finding is informational and requires human reviewer action. Fixed (1):
Disagreed (1):
Tests: passedNext steps:
|
3b84d7c to
bf814f6
Compare
|
🤖 Finished Review · ✅ Success · Started 7:29 AM UTC · Completed 7:44 AM UTC |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
fullsend-ai-review
Bot
removed
the
requires-manual-review
|
/fs-fix |
|
🤖 Finished Fix · ✅ Success · Started 7:46 AM UTC · Completed 7:50 AM UTC |
Step 9c's changed-file discovery used `git diff --name-only HEAD`, which on the existing-branch path (step 4 -> step 9) only shows uncommitted working-tree changes — not the full set of files changed on the feature branch. Replaced with `git diff --name-only <target-branch>` to match the step 4 fix in this PR and correctly list all files differing from the target branch. Updated the explanatory text to accurately describe the command's behavior. Addresses review feedback on #591
🔧 Fix agent — iteration 2 (human-triggered)Addressed 2 of 3 review findings (1 high logic-error, 1 medium incorrect-documentation). The third finding (protected-path) is informational — human approval is required per policy. Changed step 9c to use Fixed (2):
Disagreed (1):
Tests: passedNext steps:
|
|
🤖 Finished Review · ✅ Success · Started 7:51 AM UTC · Completed 8:10 AM UTC |
Superseded by updated review
fullsend-ai-review
Bot
added
the
requires-manual-review
|
🤖 Finished Retro · ✅ Success · Started 8:32 AM UTC · Completed 8:43 AM UTC |
Retro: PR #591 — Replace network-dependent git commands in code-implementation skillTimeline
What went well
What went poorly
Novel findingThe review agent identified that Proposals filed
|
…view skills
fix-review/SKILL.md: Replace origin/${BASE_BRANCH} with the local
${BASE_BRANCH} ref in the strategy-escalation diff range (line 235).
The sandbox checks out the base branch at its latest commit, so the
local ref is already current and origin refs may not be available.
code-review/SKILL.md and docs-review/SKILL.md: Replace the standalone
origin/HEAD branch discovery with gh repo view as the primary method,
keeping origin/HEAD as a fallback with error suppression. Confirmed
that git rev-parse --abbrev-ref origin/HEAD fails in the sandbox
(origin/HEAD is not set), while gh repo view works.
This is the same class of bug that fullsend-ai#489 documented and PR fullsend-ai#591 fixed
in the code-implementation skill.
Closes fullsend-ai#670
Step 4's existing-branch handling used origin/<target> in git log and git diff commands. These fail in the sandbox because the network policy blocks git protocol access to github.com, so origin refs may not be available. Replace origin/<target> with the local <target-branch> ref discovered in step 3, consistent with the approach already used in steps 5 and 9c. Addresses review feedback on #591
Step 9c's changed-file discovery used `git diff --name-only HEAD`, which on the existing-branch path (step 4 -> step 9) only shows uncommitted working-tree changes — not the full set of files changed on the feature branch. Replaced with `git diff --name-only <target-branch>` to match the step 4 fix in this PR and correctly list all files differing from the target branch. Updated the explanatory text to accurately describe the command's behavior. Addresses review feedback on #591
fix-review/SKILL.md: Replace origin/${BASE_BRANCH} with the local
${BASE_BRANCH} ref in the strategy-escalation diff range (line 235).
The sandbox checks out the base branch at its latest commit, so the
local ref is already current and origin refs may not be available.
code-review/SKILL.md and docs-review/SKILL.md: Replace the standalone
origin/HEAD branch discovery with gh repo view as the primary method,
keeping origin/HEAD as a fallback with error suppression. Confirmed
that git rev-parse --abbrev-ref origin/HEAD fails in the sandbox
(origin/HEAD is not set), while gh repo view works.
This is the same class of bug that #489 documented and PR #591 fixed
in the code-implementation skill.
Closes #670
Step 4's existing-branch handling used origin/<target> in git log and git diff commands. These fail in the sandbox because the network policy blocks git protocol access to github.com, so origin refs may not be available. Replace origin/<target> with the local <target-branch> ref discovered in step 3, consistent with the approach already used in steps 5 and 9c. Addresses review feedback on fullsend-ai#591
… ref Step 9c's changed-file discovery used `git diff --name-only HEAD`, which on the existing-branch path (step 4 -> step 9) only shows uncommitted working-tree changes — not the full set of files changed on the feature branch. Replaced with `git diff --name-only <target-branch>` to match the step 4 fix in this PR and correctly list all files differing from the target branch. Updated the explanatory text to accurately describe the command's behavior. Addresses review feedback on fullsend-ai#591
…view skills
fix-review/SKILL.md: Replace origin/${BASE_BRANCH} with the local
${BASE_BRANCH} ref in the strategy-escalation diff range (line 235).
The sandbox checks out the base branch at its latest commit, so the
local ref is already current and origin refs may not be available.
code-review/SKILL.md and docs-review/SKILL.md: Replace the standalone
origin/HEAD branch discovery with gh repo view as the primary method,
keeping origin/HEAD as a fallback with error suppression. Confirmed
that git rev-parse --abbrev-ref origin/HEAD fails in the sandbox
(origin/HEAD is not set), while gh repo view works.
This is the same class of bug that fullsend-ai#489 documented and PR fullsend-ai#591 fixed
in the code-implementation skill.
Closes fullsend-ai#670
1 participant
Summary
Replaces git commands in the code-implementation skill that depend on network access blocked by the sandbox policy:
git fetch originandorigin/<target-branch>fromgit checkout -b. The sandbox already checks out the default branch at its latest commit, soHEADis the correct base — no fetch needed.git diff --name-only origin/<target-branch>withgit diff --name-only HEAD. Origin refs may not exist without a successful fetch.Added explanatory notes at each location documenting the sandbox constraint so future editors understand why these patterns differ from typical git workflows.
Context
In run 30339994159, the agent could not discover the default branch and the post-code script rejected the output. PR #488 addresses the default branch discovery (problem 1 in the issue); this PR addresses the remaining two problems (branch creation and diff commands).
Testing
git fetch origincommands in the modified sections.git checkout -bwithout origin ref,git diff --name-only HEAD) are sandbox-compatible.Closes #489
Post-script verification
agent/489-sandbox-git-commands)c8d9d827d794064bab0c5f30d6ed702c8c1a4df0..HEAD)