Skip to content

fix(cli): a valid config was reported invalid because of a tick - #540

Merged
imran-siddique merged 1 commit into
mainfrom
fix/cli-markers-on-legacy-console
Aug 21, 2026
Merged

fix(cli): a valid config was reported invalid because of a tick#540
imran-siddique merged 1 commit into
mainfrom
fix/cli-markers-on-legacy-console

Conversation

@imran-siddique

Copy link
Copy Markdown
Member

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-config fails on its own success path:

✗ Config invalid: 'charmap' codec can't encode character '✓' in position 0: character maps to <undefined>

The config was valid. position 0 is the tick at the front of the success message. The success echo sat inside the same try as the validation, so except Exception caught the UnicodeEncodeError raised 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

  1. _marker() returns the glyph only when the target stream can encode it, and OK / ERROR otherwise. The marker is decoration and should degrade rather than take the command down.
  2. 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, which are the four sites in the package that write non-ASCII to stdout. Everything else, including cmcp 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.callback with the first click.echo raising UnicodeEncodeError, and asserts the command never claims the config is invalid. Verified both ways:

  • against the previous structure: 1 failed, 12 passed
  • against this branch: 13 passed

Full unit suite: 1185 passed, 8 skipped.

🤖 Generated with Claude Code

https://claude.ai/code/session_013EQx4N5BzTQbY8kvXUsdkY

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
imran-siddique requested a review from a team as a code owner August 21, 2026 04:26
@imran-siddique
imran-siddique merged commit 398b066 into main Aug 21, 2026
7 of 13 checks passed
@imran-siddique
imran-siddique deleted the fix/cli-markers-on-legacy-console branch August 21, 2026 04:39
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>
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