Conversation
Contributor
|
Hey A few things to address before this is ready for review:
If you'd like a hand finishing up, you can use this prompt with your coding agent:
|
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/8019f8cc-b685-4b57-a559-453d8ffd34c6 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
…orkflow (#30065) The Copilot CLI 1.0.40 crashes immediately (exit code 1, no output) when passed --allow-tool arguments containing single-quoted jq filter expressions like shell(jq '.data[] | {id, billing}' /path). Comparing the successful run (25289424690) vs failing runs (25291737132+), the failing runs had 8 new jq tools with single-quoted filter expressions: - jq '.data[] | {id, billing}' ... - jq '[.data[] | keys] | add | unique' ... etc. Fix: Replace all 8 specific jq bash tools with a single 'jq' entry, which allows the Copilot agent to run any jq command without single-quote quoting issues. Recompile lock file with --allow-tool shell(jq) instead of per-filter entries. Agent-Logs-Url: https://github.com/github/gh-aw/sessions/8019f8cc-b685-4b57-a559-453d8ffd34c6 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Fix daily model inventory checker engine failure
fix: replace single-quoted jq filter tools in daily-model-inventory to prevent Copilot CLI crash
May 4, 2026
Collaborator
|
@copilot the compiler should detect and properly handle single quotes |
…that crash Copilot CLI Add sanitizeCopilotShellCommand() in copilot_engine_tools.go that: - Detects single quotes in bash tool commands using strings.Cut - Truncates the command to a safe prefix (e.g. jq '.filter' → jq) - Emits a compiler warning pointing users to the simpler form - Deduplicates sanitized allow-tool values after sort Copilot CLI 1.0.40 crashes at startup (exit 1, no output) when --allow-tool arguments contain single-quoted filter expressions due to multi-level shell escaping conflicts in the AWF entrypoint. Restore daily-model-inventory.md to its original form with specific jq filter tools - the compiler now handles them safely at compile time. Also deduplicates sorted --allow-tool values to avoid 7 identical shell(jq) entries when multiple filter tools truncate to the same prefix. Tests added: - TestSanitizeCopilotShellCommand: unit tests for the new helper - New cases in TestCopilotEngineComputeToolArguments: truncation, deduplication, and passthrough of commands without single quotes Agent-Logs-Url: https://github.com/github/gh-aw/sessions/fd996d8a-30e1-43fe-bb00-346c34d9668d Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Contributor
Author
Done in 94d6702. Added
The Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Copilot
AI
changed the title
fix: replace single-quoted jq filter tools in daily-model-inventory to prevent Copilot CLI crash
fix: compiler detects and sanitizes single-quoted bash tool commands that crash Copilot CLI
May 4, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a Copilot CLI startup crash when --allow-tool shell(...) arguments include embedded single quotes (e.g., jq filters), by sanitizing bash tool commands into safe prefix-matching forms and deduplicating resulting allow-tool entries.
Changes:
- Add single-quote detection/sanitization for bash tool commands when generating Copilot CLI
--allow-tool shell(...)args, plus value deduplication after sorting. - Add unit tests covering sanitization behavior and deduplication in computed tool arguments.
- Recompile workflow lock files (notably
daily-model-inventory.lock.yml) to remove single-quoted jq filters and emit a single--allow-tool shell(jq)entry.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/copilot_engine_tools.go | Adds sanitizeCopilotShellCommand() and deduplicates sorted --allow-tool values to avoid single-quote crashes and repeated entries. |
| pkg/workflow/copilot_engine_test.go | Adds focused tests for sanitization and new cases validating truncation + deduplication in computed tool args. |
| .github/workflows/smoke-codex.lock.yml | Lockfile update; also changes Codex invocation plumbing (now via codex_harness.cjs and --prompt-file). |
| .github/workflows/smoke-call-workflow.lock.yml | Lockfile update reflecting regenerated runner command/harness usage. |
| .github/workflows/daily-observability-report.lock.yml | Lockfile update reflecting regenerated runner command/harness usage. |
| .github/workflows/daily-model-inventory.lock.yml | Removes single-quoted jq allow-tool entries and replaces with a deduplicated shell(jq) allow-tool entry. |
| .github/workflows/daily-fact.lock.yml | Lockfile update reflecting regenerated runner command/harness usage. |
| .github/workflows/daily-cache-strategy-analyzer.lock.yml | Lockfile update reflecting regenerated runner command/harness usage. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 8/8 changed files
- Comments generated: 2
Comment on lines
+99
to
+103
| sanitized, wasSanitized := sanitizeCopilotShellCommand(cmdStr) | ||
| if wasSanitized { | ||
| fmt.Fprintln(os.Stderr, console.FormatWarningMessage( | ||
| fmt.Sprintf("bash tool %q contains single quotes that crash Copilot CLI; "+ | ||
| "truncated to safe prefix %q for shell() prefix-matching. "+ |
| # shellcheck disable=SC1003 | ||
| sudo -E awf --config "${RUNNER_TEMP}/gh-aw/awf-config.json" --container-workdir "${GITHUB_WORKSPACE}" --mount "${RUNNER_TEMP}/gh-aw:${RUNNER_TEMP}/gh-aw:ro" --mount "${RUNNER_TEMP}/gh-aw:/host${RUNNER_TEMP}/gh-aw:ro" --env-all --exclude-env CODEX_API_KEY --exclude-env GH_AW_GH_TOKEN --exclude-env GITHUB_MCP_SERVER_TOKEN --exclude-env MCP_GATEWAY_API_KEY --exclude-env OPENAI_API_KEY --log-level info --proxy-logs-dir /tmp/gh-aw/sandbox/firewall/logs --audit-dir /tmp/gh-aw/sandbox/firewall/audit --enable-host-access --allow-host-ports 80,443,8080 --skip-pull \ | ||
| -- /bin/bash -c 'export PATH="${RUNNER_TEMP}/gh-aw/mcp-cli/bin:$PATH" && export PATH="$(find /opt/hostedtoolcache /home/runner/work/_tool -maxdepth 4 -type d -name bin 2>/dev/null | tr '\''\n'\'' '\'':'\'')$PATH"; [ -n "$GOROOT" ] && export PATH="$GOROOT/bin:$PATH" || true && INSTRUCTION="$(cat /tmp/gh-aw/aw-prompts/prompt.txt)" && codex ${GH_AW_MODEL_AGENT_CODEX:+-c model="$GH_AW_MODEL_AGENT_CODEX" }exec -c web_search="disabled" --dangerously-bypass-approvals-and-sandbox --skip-git-repo-check "$INSTRUCTION"' 2>&1 | tee -a /tmp/gh-aw/agent-stdio.log | ||
| -- /bin/bash -c 'export PATH="${RUNNER_TEMP}/gh-aw/mcp-cli/bin:$PATH" && export PATH="$(find /opt/hostedtoolcache /home/runner/work/_tool -maxdepth 4 -type d -name bin 2>/dev/null | tr '\''\n'\'' '\'':'\'')$PATH"; [ -n "$GOROOT" ] && export PATH="$GOROOT/bin:$PATH" || true && GH_AW_NODE_EXEC="${GH_AW_NODE_BIN:-}"; if [ -z "$GH_AW_NODE_EXEC" ] || [ ! -x "$GH_AW_NODE_EXEC" ]; then GH_AW_NODE_EXEC="$(command -v node 2>/dev/null || echo node)"; fi; "$GH_AW_NODE_EXEC" ${RUNNER_TEMP}/gh-aw/actions/codex_harness.cjs codex ${GH_AW_MODEL_AGENT_CODEX:+-c model="$GH_AW_MODEL_AGENT_CODEX" }exec -c web_search="disabled" --dangerously-bypass-approvals-and-sandbox --skip-git-repo-check --prompt-file /tmp/gh-aw/aw-prompts/prompt.txt' 2>&1 | tee -a /tmp/gh-aw/agent-stdio.log |
Contributor
🧪 Test Quality Sentinel ReportTest Quality Score: 90/100✅ Excellent test quality
Test Classification DetailsThe PR adds the View all 5 test cases
Flagged Tests — Requires ReviewNo individual tests require remediation. The only flag is test inflation (115 additions in test file vs. 40 additions in production file ≈ 2.9:1). This is a minor concern: much of the additional test-file volume covers regression-proofing of pre-existing behaviour in the same Language SupportTests analyzed:
Verdict
📖 Understanding Test ClassificationsDesign Tests (High Value) verify what the system does:
Implementation Tests (Low Value) verify how the system does it:
Goal: Shift toward tests that describe the system's behavioral contract — the promises it makes to its users and collaborators. References: §25297769960
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Copilot CLI 1.0.40 exits immediately (code 1, no output) when passed
--allow-toolarguments containing single-quoted jq filter expressions. The workflow broke when 8 specific jq tools were added with filters like:These compile to
--allow-tool 'shell(jq '\''.data[] | {id, billing}'\'' /path)'— the embedded single-quote escaping trips the CLI parser and causes a silent startup crash.Changes
pkg/workflow/copilot_engine_tools.go: AddedsanitizeCopilotShellCommand()that detects single quotes in bash tool commands usingstrings.Cut, truncates to a safe prefix (e.g.jq '.filter' /path→jq), emits a compiler warning pointing users to the simpler form, and deduplicates identical sanitized values so multiple filter tools produce exactly one--allow-toolentry. Copilot CLI uses prefix matching forshell()soshell(jq)still allows all jq invocations.pkg/workflow/copilot_engine_test.go: AddedTestSanitizeCopilotShellCommandunit tests and new cases inTestCopilotEngineComputeToolArgumentscovering truncation, deduplication, and passthrough.daily-model-inventory.md: Restored to its original form with specific jq filter tools — the compiler now handles them safely at compile time.daily-model-inventory.lock.yml: Recompiled — now emits a single deduplicated--allow-tool shell(jq)with no embedded single quotes.