Skip to content

fix(#5241): add missing --file flag to ImportProfile openshell command - #5245

Merged
waynesun09 merged 1 commit into
mainfrom
agent/5241-fix-importprofile-file-flag
Jul 18, 2026
Merged

fix(#5241): add missing --file flag to ImportProfile openshell command#5245
waynesun09 merged 1 commit into
mainfrom
agent/5241-fix-importprofile-file-flag

Conversation

@fullsend-ai-coder

Copy link
Copy Markdown
Contributor

Summary

  • Fix: Add missing --file flag to ImportProfile when invoking openshell provider profile import. The profile path was passed as a bare positional argument, but OpenShell requires --file <FILE> or --from <FROM>.
  • Test: Add TestImportProfile_UsesFileFlag that uses a fake openshell script to verify the --file flag is present in the import command args.

Related Issue

Closes #5241

Changes

  • internal/sandbox/sandbox.go: Add "--file" before profilePath in the exec.CommandContext call inside ImportProfile
  • internal/sandbox/sandbox_test.go: Add TestImportProfile_UsesFileFlag test that captures openshell args and asserts --file is included

Testing

  • go test ./internal/sandbox/ — all 60 tests pass
  • go vet ./internal/sandbox/ — clean
  • scan-secrets --staged — no leaks found
  • The sibling ImportProfiles function already uses --from dir correctly and is not affected

Checklist

  • Minimal change — one-line fix plus one test
  • Follows existing test patterns (fake openshell script)
  • No secrets or sensitive data committed

🤖 Generated with Claude Code


Closes #5241

Post-script verification

  • Branch is not main/master (agent/5241-fix-importprofile-file-flag)
  • Secret scan passed (gitleaks — 6a3bfb93a6275f984b2892798edae2dfdb93b56b..HEAD)
  • PR body secret scan passed (gitleaks — no-git)
  • Pre-commit hooks passed (authoritative run on runner)
  • Tests ran inside sandbox

ImportProfile passed the profile path as a bare positional argument to
`openshell provider profile import`, but OpenShell requires `--file <FILE>`.
This caused all URL-referenced profile imports to fail at sandbox startup.
The sibling ImportProfiles already correctly uses `--from dir`.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@fullsend-ai-coder
fullsend-ai-coder Bot requested a review from a team as a code owner July 17, 2026 14:25
@fullsend-ai-coder fullsend-ai-coder Bot added the ready-for-review Triggers review agent dispatch label Jul 17, 2026
@github-actions

Copy link
Copy Markdown

Site preview

Preview: https://d6768d1d-site.fullsend-ai.workers.dev

Commit: 7b4ddaefa04aa14aa1a7443468e0806e8a07442b

@codecov

codecov Bot commented Jul 17, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@maruiz93

Copy link
Copy Markdown
Contributor

/fs-review

@fullsend-ai-review

fullsend-ai-review Bot commented Jul 17, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 2:45 PM UTC · Completed 2:52 PM UTC
Commit: cc7a526 · View workflow run →

@fullsend-ai-review

Copy link
Copy Markdown

Looks good to me


Labels: PR modifies internal/sandbox/ — sandbox component bug fix

@fullsend-ai-review fullsend-ai-review Bot added ready-for-merge All reviewers approved — ready to merge component/sandbox OpenShell sandbox environment labels Jul 17, 2026

@waynesun09 waynesun09 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ran 3 independent reviews (Claude x2, Grok). All clean, no findings that block this.

  • Fix matches the exact error string from #5241 (`<--file |--from >`).
  • No other `openshell` call site in the codebase has the same positional-arg-instead-of-flag bug (grepped all provider/profile invocations — create/update/delete all correctly use flags or documented positional signatures already).
  • New test is a genuine regression guard, not trivially-passing — traced manually: pre-fix, the logged args would be `provider profile import /some/my-profile.yaml` with no `--file`, which would correctly fail the assertion.
  • No injection surface added (argv form throughout, `--file` is a hardcoded literal) — and as a minor bonus, explicitly binding the path to `--file` is actually more robust than the old bare positional arg, which could have been misparsed if a path ever began with `-`.
  • Test isolation confirmed safe: no `t.Parallel()` in this file, `t.Setenv` auto-restores PATH, and the fake script's `$3` check correctly targets only the `import` invocation, not the preceding `delete` call.

One important process note, not a code issue: merging this alone won't turn fullsend-ai/agents#211's CI green. Confirmed agents#211's functional-tests resolves FULLSEND_VERSION via `gh release view --repo fullsend-ai/fullsend` (floating "latest"), not a source build — and the current latest release (v0.31.0, published 2026-07-16) predates this fix. A new release tag needs to be cut after this merges before #211's CI will actually pick it up.

Approving.

@waynesun09
waynesun09 added this pull request to the merge queue Jul 18, 2026
Merged via the queue into main with commit 9791dda Jul 18, 2026
34 of 40 checks passed
@waynesun09
waynesun09 deleted the agent/5241-fix-importprofile-file-flag branch July 18, 2026 15:49
@fullsend-ai-retro

fullsend-ai-retro Bot commented Jul 18, 2026

Copy link
Copy Markdown

🤖 Finished Retro · ✅ Success · Started 3:51 PM UTC · Completed 4:07 PM UTC
Commit: 7b4ddae · View workflow run →

@fullsend-ai-retro

Copy link
Copy Markdown

Retro: PR #5245 — fix(#5241): add missing --file flag to ImportProfile

Timeline

Time (UTC) Event
Jul 17, 14:14 Issue #5241 filed by maruiz93 — well-specified bug with exact line, fix, and reproducer
Jul 17, 14:15 Triage agent starts
Jul 17, 14:18 Triage completes (3m 44s, $0.55) — labels: type/bug, priority/high, component/sandbox, ready-to-code
Jul 17, 14:19 Code agent starts
Jul 17, 14:26 PR #5245 created (6m 59s, $1.76) — 1-line fix + regression test, 41 tool calls
Jul 17, 14:29 CI passes (all 60 tests green, coverage clean)
Jul 17, 14:44 maruiz93 triggers /fs-review
Jul 17, 14:45 Review agent starts
Jul 17, 14:53 Review agent approves (8m 19s, $2.03) — 3 sub-agents, zero findings, posts "Looks good to me"
Jul 18, 15:37 waynesun09 approves with detailed confirmatory analysis
Jul 18, 15:49 PR merged

Total pipeline cost: ~$4.34 (triage + code + review). Rework: zero — correct fix on first try.

What went well

  • Exceptional issue quality → fast resolution. The issue included exact line number, exact fix code, reproducer, and impact analysis. This enabled the code agent to implement the correct fix in under 3 minutes of active time with zero exploration waste.
  • Code agent efficiency. 41 tool calls, no dead-end file reads, clean workflow: read issue → read source → apply fix → add test → verify → commit. The $1.76 cost is proportionate to the work.
  • Correct approval outcome. Both the review agent and human reviewer approved, and the fix was indeed correct.

Improvement opportunities

Two proposals filed — see linked issues below.

  1. Review agent APPROVE comments lack confirmatory reasoning. The review agent posted "Looks good to me" with no substantive commentary. Meanwhile, the human reviewer provided five specific verifications: confirmed the fix matches the error string, grepped all openshell call sites for the same bug pattern, traced pre-fix behavior to verify the test isn't trivially passing, confirmed no injection surface, and verified test isolation safety. The review agent's sub-agents performed similar analysis internally but none of it was surfaced.

  2. Code agent wastes tokens fighting pre-commit hooks in sandbox. The agent spent 3 commit attempts (~$0.15-0.20 and ~30s) diagnosing and working around pre-commit hook failures caused by the L7 proxy blocking git-upload-pack. Since hooks run authoritatively in the post-script outside the sandbox, this in-sandbox friction is pure waste.

Proposals filed

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

Labels

component/sandbox OpenShell sandbox environment ready-for-merge All reviewers approved — ready to merge ready-for-review Triggers review agent dispatch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(sandbox): ImportProfile passes positional arg instead of --file flag

2 participants