Drop redundant --yolo from Pi engine invocations#35950
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
--yolo from Pi engine invocations
|
📰 BREAKING: Smoke Copilot is now investigating this pull request. Sources say the story is developing... |
|
✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟 |
|
🚀 Smoke Pi MISSION COMPLETE! Pi delivered. 🥧 |
|
🚀 Smoke Gemini MISSION COMPLETE! Gemini has spoken. ✨ |
|
🎬 THE END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨ |
|
🚀 Smoke Antigravity MISSION COMPLETE! Antigravity has spoken. ✨ |
Agent Container Tool Check
Result: 12/12 tools available ✅ Overall Status: PASS
|
Smoke Test: Gemini
Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "localhost"See Network Configuration for more information.
|
|
@copilot investigate why the safeoutputs shell script is not available. Make sure cli-proxy is enabled on smoke-pi so that safeoutputs scripts are generated. see https://github.com/github/gh-aw/actions/runs/26689051273/job/78662209578 |
|
Smoke Test: Codex - 26689051269
Warning Firewall blocked 6 domainsThe following domains were blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "accounts.google.com"
- "android.clients.google.com"
- "clients2.google.com"
- "contentautofill.googleapis.com"
- "safebrowsingohttpgateway.googleapis.com"
- "www.google.com"See Network Configuration for more information.
|
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. Warning Firewall blocked 6 domainsThe following domains were blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "accounts.google.com"
- "android.clients.google.com"
- "clients2.google.com"
- "contentautofill.googleapis.com"
- "safebrowsingohttpgateway.googleapis.com"
- "www.google.com"See Network Configuration for more information.
|
🧪 Smoke Test: Claude — Run 26689051295Core #1–13: ✅ all passed Overall: PARTIAL (2 skipped, rest pass) Warning Firewall blocked 6 domainsThe following domains were blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "accounts.google.com"
- "android.clients.google.com"
- "clients2.google.com"
- "contentautofill.googleapis.com"
- "safebrowsingohttpgateway.googleapis.com"
- "www.google.com"See Network Configuration for more information.
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
💥 Automated smoke test review - all systems nominal!
Warning
Firewall blocked 6 domains
The following domains were blocked by the firewall during workflow execution:
accounts.google.comandroid.clients.google.comclients2.google.comcontentautofill.googleapis.comsafebrowsingohttpgateway.googleapis.comwww.google.com
To allow these domains, add them to the
network.allowedlist in your workflow frontmatter:
network:
allowed:
- defaults
- "accounts.google.com"
- "android.clients.google.com"
- "clients2.google.com"
- "contentautofill.googleapis.com"
- "safebrowsingohttpgateway.googleapis.com"
- "www.google.com"See Network Configuration for more information.
💥 [THE END] — Illustrated by Smoke Claude · opus48 1.2M
| // filterPiArgs removes redundant Pi CLI flags that gh-aw should not pass through. | ||
| // Pi runs in yolo mode by default, so explicit --yolo flags are ignored while all | ||
| // other engine args are preserved in order. | ||
| func filterPiArgs(args []string) []string { |
There was a problem hiding this comment.
Nice — extracting filterPiArgs into a small pure helper keeps the execution path readable and easy to unit test.
| func filterPiArgs(args []string) []string { | ||
| filtered := make([]string, 0, len(args)) | ||
| for _, arg := range args { | ||
| if arg == "--yolo" || strings.HasPrefix(arg, "--yolo=") { |
There was a problem hiding this comment.
Good call handling both --yolo and --yolo=* forms; the prefix check covers value-style flags cleanly.
|
Smoke: Drop redundant Warning Firewall blocked 6 domainsThe following domains were blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "accounts.google.com"
- "android.clients.google.com"
- "clients2.google.com"
- "contentautofill.googleapis.com"
- "safebrowsingohttpgateway.googleapis.com"
- "www.google.com"See Network Configuration for more information.
|
There was a problem hiding this comment.
Looks good overall. The change is focused, and the added regression coverage makes the Pi default-yolo behavior much clearer.
Warning
Firewall blocked 6 domains
The following domains were blocked by the firewall during workflow execution:
accounts.google.comandroid.clients.google.comclients2.google.comcontentautofill.googleapis.comsafebrowsingohttpgateway.googleapis.comwww.google.com
To allow these domains, add them to the
network.allowedlist in your workflow frontmatter:
network:
allowed:
- defaults
- "accounts.google.com"
- "android.clients.google.com"
- "clients2.google.com"
- "contentautofill.googleapis.com"
- "safebrowsingohttpgateway.googleapis.com"
- "www.google.com"See Network Configuration for more information.
📰 BREAKING: Report filed by Smoke Copilot · gpt54 15.1M
| // Append any user-supplied extra args from engine.args | ||
| if workflowData.EngineConfig != nil { | ||
| piArgs = append(piArgs, workflowData.EngineConfig.Args...) | ||
| piArgs = append(piArgs, filterPiArgs(workflowData.EngineConfig.Args)...) |
There was a problem hiding this comment.
Nice touch routing engine.args through filterPiArgs here; it keeps the command assembly readable and makes the redundant-flag policy explicit at the call site.
| lockBytes, err := os.ReadFile(lockPath) | ||
| require.NoError(t, err, "should read smoke-pi lock file") | ||
|
|
||
| assert.NotContains(t, string(lockBytes), "--yolo", |
There was a problem hiding this comment.
This lockfile assertion is a solid regression check for the compiled smoke workflow, not just the helper behavior.
| // Append any user-supplied extra args from engine.args | ||
| if workflowData.EngineConfig != nil { | ||
| piArgs = append(piArgs, workflowData.EngineConfig.Args...) | ||
| piArgs = append(piArgs, filterPiArgs(workflowData.EngineConfig.Args)...) |
There was a problem hiding this comment.
Nice touch routing engine.args through filterPiArgs here; it keeps the command assembly readable and makes the redundant-flag policy explicit at the call site.
| lockBytes, err := os.ReadFile(lockPath) | ||
| require.NoError(t, err, "should read smoke-pi lock file") | ||
|
|
||
| assert.NotContains(t, string(lockBytes), "--yolo", |
There was a problem hiding this comment.
This lockfile assertion is a solid regression check for the compiled smoke workflow, not just the helper behavior.
|
📰 DEVELOPING STORY: Smoke Copilot reports failed to deliver outputs. Our correspondents are investigating the incident... |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
... Addressed in f0349fe. I verified smoke-pi already compiles with the expected cli-proxy/safeoutputs wiring ( |
|
@copillot update prompt to try to call the script directly at ${RUNNER_TEMP}/gh-aw/mcp-cli/bin |
There was a problem hiding this comment.
Pull request overview
Removes redundant --yolo / --yolo=* flags from Pi engine invocations now that Pi defaults to yolo mode, and adds regression coverage to prevent reintroducing the flag in generated Smoke Pi workflow output.
Changes:
- Filter
--yoloand--yolo=*out ofengine.argswhen composing the Pi CLI command while preserving all other args and their order. - Add unit tests covering Pi arg filtering behavior and an execution-step regression test ensuring
--yolois not emitted. - Add a lockfile regression test asserting
smoke-pi.lock.ymldoes not contain--yolo, plus a changeset entry.
Show a summary per file
| File | Description |
|---|---|
pkg/workflow/pi_engine.go |
Filters redundant yolo flags from Pi CLI args during execution step composition. |
pkg/workflow/pi_engine_test.go |
Adds unit + execution-step tests to ensure yolo flags are dropped and other args preserved. |
pkg/workflow/compiled_lock_files_test.go |
Adds a regression test asserting the compiled Smoke Pi lockfile does not contain --yolo. |
.changeset/patch-remove-pi-yolo-arg.md |
Documents the patch-level change for release notes. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 4/4 changed files
- Comments generated: 0
Pi now runs in yolo mode by default, so passing
--yolothrough Smoke Pi and the Pi harness is redundant. This change removes that coupling from the Pi execution path and adds regression coverage so the flag does not creep back into generated workflow output.Execution path
--yoloand--yolo=*fromengine.argsbefore composing the Pi CLI command.Smoke Pi guardrail
smoke-pi.lock.ymldoes not contain--yolo.Coverage
Example of the new behavior:
Pi is invoked as if the workflow had declared:
✨ PR Review Safe Output Test - Run 26689051295
Warning
Firewall blocked 6 domains
The following domains were blocked by the firewall during workflow execution:
accounts.google.comandroid.clients.google.comclients2.google.comcontentautofill.googleapis.comsafebrowsingohttpgateway.googleapis.comwww.google.comSee Network Configuration for more information.