fix(cli): a valid config was reported invalid because of a tick - #540
Merged
Conversation
On a Windows console, whose default code page cannot encode U+2713,
`cmcp validate-config` failed on its own success path:
✗ Config invalid: 'charmap' codec can't encode character '✓'
in position 0: character maps to <undefined>
The config was valid. The success echo sat inside the same try block as
the validation, so `except Exception` caught the UnicodeEncodeError from
printing the result and relabelled it as a validation failure, then
exited 1. A reader following the published quickstart was told their
config was broken because the tool could not draw a tick.
Two changes:
- _marker() returns the glyph only when the target stream can encode it,
and "OK" or "ERROR" otherwise. The marker is decoration; it should
degrade rather than take the command down.
- The success echo moves out of the try. Only the validation belongs
there. A failure to print is not a failure to validate.
Applied to validate-config and validate-bundle, the four sites that
print non-ASCII. Nothing else in the package writes non-ASCII to stdout.
Regression test asserts the second point directly: with the first echo
raising, the command must never claim the config is invalid. It fails
against the previous structure and passes against this one.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013EQx4N5BzTQbY8kvXUsdkY
imran-siddique
added a commit
that referenced
this pull request
Aug 21, 2026
The regression test added in #540 used percent formatting in an assertion message and the repo lints for UP031, so ruff check fails on main. It got there because I gated the merge on a check-status grep rather than on the lint job itself. Claude-Session: https://claude.ai/code/session_013EQx4N5BzTQbY8kvXUsdkY Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
imran-siddique
added a commit
that referenced
this pull request
Aug 21, 2026
* fix: ruff UP031 in the CLI encoding test The regression test added in #540 used percent formatting in an assertion message and the repo lints for UP031, so ruff check fails on main. It got there because I gated the merge on a check-status grep rather than on the lint job itself. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013EQx4N5BzTQbY8kvXUsdkY * ci: deploy the docs when the theme or the landing page changes Only trace-spec listed overrides/** in its docs deploy filter, so a change to overrides/main.html merged to main and the live site kept serving the old template. The og:title fix did exactly that: merged, green, and not deployed. The same gap covers files the build publishes but the filter never named. index.md is the site landing page now, and the governance documents are copied into the build, so any of them can change without triggering a deploy. Aligns each filter with what the build actually copies. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013EQx4N5BzTQbY8kvXUsdkY --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Found while running the published quickstart end to end to verify it.
On a Windows console, whose default code page cannot encode
U+2713,cmcp validate-configfails on its own success path:The config was valid.
position 0is the tick at the front of the success message. The success echo sat inside the sametryas the validation, soexcept Exceptioncaught theUnicodeEncodeErrorraised by printing the result, relabelled it as a validation failure, and exited 1.So the failure mode is not cosmetic: a correct config is reported as broken, with a codec error as the stated reason. A reader following the quickstart stops at step 2 believing they mis-pasted something.
Two changes
_marker()returns the glyph only when the target stream can encode it, andOK/ERRORotherwise. The marker is decoration and should degrade rather than take the command down.try. Only the validation belongs there. A failure to print is not a failure to validate.Applied to
validate-configandvalidate-bundle, which are the four sites in the package that write non-ASCII to stdout. Everything else, includingcmcp verify, is already ASCII.On the test
CliRunner(charset="cp1252")is not enough on its own: click encodes leniently, so the tick never raises and a test built on it passes against the broken code too. I checked, and threw that version away.The real test drives
validate_config.callbackwith the firstclick.echoraisingUnicodeEncodeError, and asserts the command never claims the config is invalid. Verified both ways:1 failed, 12 passed13 passedFull unit suite:
1185 passed, 8 skipped.🤖 Generated with Claude Code
https://claude.ai/code/session_013EQx4N5BzTQbY8kvXUsdkY