Skip to content

fix(ci): verify managed-settings.json without runner-shell quote stripping - #100

Merged
gatezh merged 1 commit into
masterfrom
fix-ci-verify-managed-settings-quoting
May 13, 2026
Merged

gatezh merged 1 commit into
masterfrom
fix-ci-verify-managed-settings-quoting

Conversation

@gatezh

@gatezh gatezh commented May 13, 2026

Copy link
Copy Markdown
Owner

What

Rewrites the jq assertion that PR #99 added to the verify matrix in build-claude-code.yml so it survives GitHub Actions' template expansion + bash -c wrapping.

Why

PR #99 merged green on PRs but the post-merge Build claude-code run failed all four verify legs (run 25765306805) with:

jq: error: syntax error, unexpected '/' (Unix shell quoting issues?) at <top-level>, line 1:
.hooks.SessionStart[0].hooks[0].command == /usr/local/bin/patch-playwright-mcp

Root cause: the matrix value contained a literal "…" inside a single-quoted jq filter. After GHA expands ${{ matrix.verify-command }} into

docker run … bash -c "${{ matrix.verify-command }}"

the outer "…" is the shell's double-quote context, so the inner " characters closed and reopened the outer string rather than reaching jq as string delimiters. By the time bash -c parsed the filter, the comparison value was an unquoted bare token starting with /, and jq rejected it.

This is the same class of bug PR #93 fixed for AGENT_BROWSER_EXECUTABLE_PATH. The fix is the same: emit the value with jq -r (raw string, no surrounding quotes) and assert the match via grep -qx, so no " ever appears in the matrix value.

Changes

  • .github/workflows/build-claude-code.yml — all four verify-command rows:
    • Before: jq -e '.hooks.SessionStart[0].hooks[0].command == \"/usr/local/bin/patch-playwright-mcp\"' /etc/claude-code/managed-settings.json >/dev/null
    • After: jq -r '.hooks.SessionStart[0].hooks[0].command' /etc/claude-code/managed-settings.json | grep -qx /usr/local/bin/patch-playwright-mcp

Notes

  • The new idiom keeps the same coverage: it still confirms the file exists (jq -r errors if the file is missing), parses as JSON, contains .hooks.SessionStart[0].hooks[0].command, and the value equals the patch binary path exactly.
  • The check now fails closed on any reshape of managed-settings.json (e.g. matcher added, hook moved to position 1) — that's the same fragility the previous version had, intentional for now since the file's shape is the contract.

Test plan

  • actionlint clean.
  • Simulated bash -c "<value>" locally with the YAML-decoded matrix string against a copy of managed-settings.json — predicate exits 0.
  • CI Build claude-code run on merge — all four verify legs (claude-code/claude-code-sandbox × amd64/arm64) should pass.

Follow-up to #99; recovers the master build.

…pping

The verify-command added in #99 embedded literal " around the expected path
inside a single-quoted jq filter. After GHA expansion into bash -c "...",
the inner " terminated and reopened the outer "..." rather than reaching
jq as a string delimiter, so jq received

    .hooks.SessionStart[0].hooks[0].command == /usr/local/bin/patch-playwright-mcp

and failed with "unexpected '/'". Switch to the printenv | grep -qx idiom
used since #93: jq -r prints the bare string value, grep -qx asserts an
exact match. No literal " inside the matrix value, so the value survives
both the YAML decode and the bash -c wrapping unchanged.
@gatezh
gatezh merged commit dcd8d19 into master May 13, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant