Uh oh!
There was an error while loading. Please reload this page.
Fix gvisor+privileged sandbox codemod gap and add missing tools.bash codemod for min-integrity: none - #53964
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
pelikhan
commented
Aug 19, 2026
@copilot the codemod gvisor should retain gvisor and drop the sudo field |
…ng runtime Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
✅ Test Quality Sentinel completed test quality analysis. Test Quality Sentinel skipped because pre-fetch PR data was unavailable: unable to fetch test file diff
|
✅ PR Code Quality Reviewer completed the code quality review.
|
✅ Design Decision Gate 🏗️ completed the design decision gate check.
|
✅ Ponytail Reviewer completed successfully!
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅
|
Comment MemoryNote This comment is managed by comment memory.It stores persistent context for this thread in the code block at the top of this comment.
|
There was a problem hiding this comment.
Request changes
This still misses at least one valid frontmatter shape, so gh aw fix --write can claim success while leaving the strict-mode compile error unfixed.
Blocking theme
- The new
min-integrity-none-requires-bashcodemod only rewrites block-styletools:YAML and silently skips inline mappings such astools: { github: { min-integrity: none } }, even though the parsed frontmatter says the fix is required.
🔎 Code quality review by PR Code Quality Reviewer · gpt54 · 12.9 AIC · ⌖ 6.91 AIC · ⊞ 7K
Comment /review to run again
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The two codemod fixes are correct and well-tested. The gvisor+privileged path correctly keeps runtime: gvisor unchanged (since targetRuntime == oldRuntime, needsRuntimeUpdate stays false) and only strips the unsupported sudo/legacy-security fields. The min-integrity-none codemod safely inserts bash: false as the first child of the tools: block. Both codemods have comprehensive no-op guard tests. No issues found.
🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · sonnet46 · 20.2 AIC · ⌖ 8.8 AIC · ⊞ 5.8K
There was a problem hiding this comment.
Ponytail review (over-engineering only).
One finding: duplicated insert-point scan logic across three codemod files, extractable into a shared helper.
net: -30 lines possible.
Generated by ✂️ Ponytail Reviewer for #53964 · auto · 25 AIC · ⌖ 3.62 AIC · ⊞ 7.3K
Comment /ponytail to run again
Comments that could not be inline-anchored
pkg/cli/codemod_min_integrity_none_bash.go:107
L77-107: yagni: insertBashFalseIntoTopLevelTools reimplements the "find first-child insertion point under a top-level block" scan already written twice (insertAt logic in codemod_cli_proxy_bash.go and codemod_cli_proxy_mode.go). Extract one shared helper and reuse it in all three codemods instead of a third copy.
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /diagnosing-bugs and /tdd — requesting changes for a PR-body/code contradiction and two test coverage gaps.
📋 Key Themes & Highlights
Key Themes
- PR description contradicts implementation: The PR body shows
gvisor → docker-sudo-iptablesmigration, but the code (and changeset) correctly keepsruntime: gvisorand drops the privileged fields. The description needs a fix before merge to avoid future confusion. - Test coverage gaps: The combined
sudo + legacy-security + gvisorcase is untested; theinsertBashFalseIntoTopLevelToolscomment-skipping logic has no targeted test.
Positive Highlights
- ✅ Clean, focused fixes — each codemod solves exactly one gap with minimal surface area.
- ✅
migrateSandboxAgentSecurityLinessignature improvement (oldRuntime, targetRuntime) makes the call-site intent explicit. - ✅ New codemod tests are well-structured, parallel-safe, and cover all the important no-op paths.
- ✅ Changeset description is accurate and matches the implementation.
- ✅ Registry ordering is maintained correctly in
GetAllCodemods.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 · 31.6 AIC · ⌖ 10.2 AIC · ⊞ 7.9K
Comment /matt to run again
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Pull request overview
Attempts to close two strict-mode codemod gaps involving privileged gVisor configurations and explicit Bash settings.
Changes:
- Adjusts sandbox runtime migration and runtime-line rewriting.
- Adds and registers the
min-integrity-none-requires-bashcodemod. - Adds unit tests and release notes.
Show a summary per file
| File | Description |
|---|---|
pkg/cli/fix_codemods.go | Registers the new codemod. |
pkg/cli/fix_codemods_test.go | Updates registry-order expectations. |
pkg/cli/codemod_sandbox_runtime_profile.go | Changes gVisor privilege migration behavior. |
pkg/cli/codemod_sandbox_runtime_profile_test.go | Tests revised gVisor handling. |
pkg/cli/codemod_min_integrity_none_bash.go | Implements explicit bash: false insertion. |
pkg/cli/codemod_min_integrity_none_bash_test.go | Tests the new codemod. |
.changeset/fix-sandbox-gvisor-privileged-and-min-integrity-bash-codemods.md | Documents both fixes. |
Review details
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
- Files reviewed: 7/7 changed files
- Comments generated: 3
- Review effort level: Balanced
| getMentionsAllowTeamMembersCodemod(), // Rename allow-team-members to allowed-collaborators in safe-outputs.mentions | ||
| getEngineCopilotSDKDriverToDriverCodemod(), // Rename deprecated engine.copilot-sdk-driver to engine.driver | ||
| getEngineModelToTopLevelCodemod(), // Move engine.model to top-level model | ||
| getMinIntegrityNoneRequiresBashCodemod(), // Add tools.bash: false when tools.github.min-integrity is 'none' |
There was a problem hiding this comment.
Done — getMinIntegrityNoneRequiresBashCodemod() is now registered at fix_codemods.go:113, immediately before getCLIProxyBashDisabledCodemod() at line 114, so a single fix pass emits both bash: false and cli-proxy: false. Covered by a registry-level test in codemod_min_integrity_none_bash_test.go that runs the full registry through processWorkflowFileWithInfo and asserts both settings appear after one pass, for block and inline tools: syntax.
| } | ||
| trimmed := strings.TrimSpace(line) | ||
| if needsRuntimeUpdate && getIndentation(line) == indent && strings.HasPrefix(trimmed, "runtime:") { | ||
| result = append(result, indent+"runtime: "+targetRuntime) |
There was a problem hiding this comment.
Done — the runtime line rewrite now appends trailingCommentSuffix(...) (codemod_sandbox_runtime_profile.go:153), which uses the shared findTrailingCommentIndex helper to preserve any user-authored # ... suffix while replacing only the scalar value. Covered by the test case "rewritten runtime line keeps its trailing comment".
Uh oh!
There was an error while loading. Please reload this page.
…ts in ambiguous codemod cases Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
PR Triage
Fixes gh aw fix --write codemod gaps for gvisor+privileged sandbox and missing tools.bash min-integrity:none codemod. Moderate size, addresses repeatable --strict failures.
|
gh-aw-bot
commented
Aug 19, 2026
@copilot PR still has unresolved review threads. Please address the newest blocking feedback, refresh the branch if needed, and then run the Open review threads (newest first):
Also attempt a branch refresh if GitHub offers one.
|
…ping bash insertion Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Remaining items addressed:
The other listed threads were already resolved in earlier commits on this branch:
On the "migrate gvisor + privileged options to Branch is refreshed from |
gh-aw-bot
commented
Aug 19, 2026
@copilot PR still has maintainer-visible unfinished work. Open blockers to address in one pass:
|
Handoff summary:
Remaining risk / maintainer action:
|
PR TriageCategory: bug · Risk: medium · Score: 68/100 (impact 30 + urgency 20 + quality 18) Sandbox codemod correctness fix, approved review, CI green; large diff but well-scoped. Automated triage — see run report for full details.
|
gh-aw-bot
commented
Aug 19, 2026
@copilot PR still has maintainer-visible unfinished work. Open review threads / blockers to close in one pass:
|
Handoff summary (no code changes needed this pass — the branch already contained every requested fix):
Remaining risks / maintainer actions:
|
gh-aw-bot
commented
Aug 19, 2026
@copilot PR still has maintainer-visible unfinished work. Please close out the remaining blockers in one pass:
Open review threads (newest first):
If those are already fixed on the current head, reply on-thread with the exact commit/test evidence so reviewers can resolve them quickly.
|
Uh oh!
There was an error while loading. Please reload this page.
🎉 This pull request is included in a new release. Release: |
Two recurring gaps in
gh aw fix --writeleft--strictcompile failures unrepaired across independently-verified repos: thesandbox-runtime-profilescodemod hard-errored onruntime: gvisorcombined with privileged options instead of migrating, and no codemod existed to satisfy the newtools.bashrequirement whenmin-integrity: none.sandbox-runtime-profilescodemodsandbox.agent.runtime: gvisorcombined withsudo: trueorlegacy-security: enablenow auto-migrates toruntime: docker-sudo-iptables, preserving the original privileged/sudo intent, instead of aborting the fix pass with an error.runtime:line in place when its value changes (previously only inserted aruntime:key when one was absent).New
min-integrity-none-requires-bashcodemodtools.bash: falsewhentools.github.min-integrityisnoneandtools.bashis not already specified, satisfying the strict-mode requirement that shell access be explicit.bash: false.Both codemods are registered in the codemod registry with accompanying unit tests, and a changeset documents the fix.
run: https://github.com/github/gh-aw/actions/runs/32276767055> Generated by 👨🍳 PR Sous Chef · gpt54 · 26.4 AIC · ⌖ 9.42 AIC · ⊞ 9.3K · ◷