Skip to content

fix(cli): reject unknown args in logs tail / backup list|show; flag a corrupt preferences file - #27

Merged
ChanMeng666 merged 1 commit into
masterfrom
claude/cli-known-limits-ee12t7
Oct 20, 2026
Merged

ChanMeng666 merged 1 commit into
masterfrom
claude/cli-known-limits-ee12t7

Conversation

@ChanMeng666

Copy link
Copy Markdown
Owner

Description

Closes two items under "Known limits that are accepted for now" in docs/PROJECT_STATUS.md. Both broke the CLI contract (JSON in, JSON out, stable error codes).

1. logs tail and backup list|show ignored arguments they don't define

  • logs tail now validates with _check_args(valued=("--n", "--level"), max_positionals=0) before it reads any state. --n must be a positive integer. --level must be one of info|warn|error|debug. Anything else returns INVALID_USAGE … Nothing was changed. These all used to run silently: a bare --n, --n five, --level loud, --follow, or any positional.
  • --n 0 and negative values are now rejected too. They used to return the whole over-read window, because events[-0:] returns every event.
  • backup list takes no arguments and backup show takes exactly one id. All four backup subcommands now validate before prefs_factory() runs. restore and prune keep their old grammar; only the order changed.

2. A corrupt user_preferences.json looked like the defaults

  • New read-only helper _check_prefs_file() in bin/audio-hooks.py. It reads the file and flags it as corrupt when it is not valid JSON (this includes an empty file and invalid UTF-8) or when the JSON is not an object.
  • status gains preferences_file: {path, exists, corrupt}. When the file is corrupt it also carries problem and a warning with code PREFS_FILE_CORRUPT. Top-level ok stays true, the same way install.warning works.
  • diagnose reports PREFS_FILE_CORRUPT as an error (ok:false, exit 1), because every setting the user has is being ignored. It also adds prefs_file to its output. The suggested command is audio-hooks backup list.
  • The new code is catalogued in CLI_ERROR_CODES with appears_in: "diagnose", like DUAL_INSTALL_DETECTED, which also shows up in status. The test only allows error / diagnose / error, diagnose.
  • Both commands stay read-only. The file is detected, never repaired, rewritten, backed up or migrated.
  • Fix needed along the way: a file holding a JSON array or string made status, diagnose and get fail with INTERNAL_ERROR. load() returns such a value as-is and only falls back to {} when the file fails to parse. _load_config_raw() now uses the defaults in that case, as it already did for an unparseable file. hooks/user_preferences.py is unchanged.

Docs

  • docs/PROJECT_STATUS.md: removed both items and added one new known limit (see below).
  • docs/INVARIANTS.md: replaced the "Known limit" sentence in Commands that only report must stay read-only with the rule "detected, never repaired" plus the logs tail / backup grammar.
  • CHANGELOG.md: new ## [Unreleased] section.
  • SKILL.md and README don't describe this behaviour, so they are unchanged.
  • The build mirror was regenerated with bash scripts/build-plugin.sh only.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature
  • Breaking change. Strictly, scripts that passed junk args to logs tail / backup list|show now get INVALID_USAGE, which is the intent.
  • Documentation update

How Has This Been Tested?

Tests added to tests/test_read_only_commands.py:

  • REJECTED: 14 bad invocations of logs tail / backup list|show. Each must return INVALID_USAGE and leave home and data dirs byte-identical. This runs against a fresh home and against an older-version prefs file, so a rejected call doesn't migrate it.
  • READ_ONLY gains logs tail --level error and logs tail --level debug --n 1.
  • TestReadOnlyCommandsCorruptPreferences covers five corrupt files: truncated JSON, empty, array, string, non-UTF-8. For each:
    • status and diagnose both flag it.
    • Every READ_ONLY + REJECTED invocation leaves the file and dirs byte-identical (no .bak, no lock).
    • A write still repairs it.
  • TestReadOnlyCommandsRealProcessCorruptPreferences runs the same check in real processes.
  • Negative check: a valid or older-version file is not reported as corrupt.

python -m unittest discover tests

Ran 803 tests in 48.220s

OK (skipped=5)

bash scripts/build-plugin.sh --check

{"ok":true,"in_sync":true,"checked":207}

Real CLI, scratch CLAUDE_PLUGIN_DATA (and HOME), holding {"audio_theme": "custom", "enabled_hooks": {"stop": fal:

before: e586678f323048e4c0a96b54eef96efc8fb4927f9f070778ffead9535da295ff
--- status (rc=0; excerpt of ok, theme, preferences_file)
{
 "ok": true,
 "theme": "default",
 "preferences_file": {
  "path": "<scratch>/data/user_preferences.json",
  "exists": true,
  "corrupt": true,
  "problem": "not valid JSON: Expecting value: line 1 column 53 (char 52)",
  "warning": {
   "code": "PREFS_FILE_CORRUPT",
   "message": "<scratch>/data/user_preferences.json is not valid JSON: Expecting value: line 1 column 53 (char 52). Every setting in it is ignored: hooks and the status line run on the defaults.",
   "hint": "Restore an earlier copy with `audio-hooks backup restore <id>` (the ids are in `audio-hooks backup list`); the restore backs up the corrupt file first. A hook event or a write such as `audio-hooks set` instead replaces an unparseable file with the defaults. Reporting commands never change it.",
   "suggested_command": "audio-hooks backup list"
  }
 }
}
--- diagnose (rc=1; excerpt of ok, errors, warning codes, prefs_file)
{
 "ok": false,
 "errors": [
  {
   "code": "PREFS_FILE_CORRUPT",
   "message": "<scratch>/data/user_preferences.json is not valid JSON: Expecting value: line 1 column 53 (char 52). Every setting in it is ignored: hooks and the status line run on the defaults.",
   "hint": "(same as above)",
   "suggested_command": "audio-hooks backup list"
  }
 ],
 "warnings": [],
 "prefs_file": {
  "path": "<scratch>/data/user_preferences.json",
  "exists": true,
  "corrupt": true,
  "problem": "not valid JSON: Expecting value: line 1 column 53 (char 52)"
 }
}
after:  e586678f323048e4c0a96b54eef96efc8fb4927f9f070778ffead9535da295ff

Afterwards the data dir held only user_preferences.json: no .bak, lock, logs/ or queue/. <scratch> stands for the session scratchpad path.

Sample rejections (real output):

logs tail --n x       → {"ok":false,"error":{"code":"INVALID_USAGE","message":"--n requires a positive integer, got 'x'. Nothing was changed.", ...}}
logs tail --level loud → {"ok":false,"error":{"code":"INVALID_USAGE","message":"Unknown --level value: 'loud'. Nothing was changed.", ...}}
backup show a b       → {"ok":false,"error":{"code":"INVALID_USAGE","message":"Unexpected argument for `audio-hooks backup show`: b. Nothing was changed.", ...},"unknown_args":["b"]}

Left undone / uncertain

  • New known limit, not fixed: the hook runner crashes on a JSON array or string file. A preferences file that is valid JSON but not an object makes every hook event fail in hook_runner.is_hook_enabled (AttributeError, no sound), and the runner never replaces the file. This was reproduced by hand and predates this PR. Fixing it means changing hook_runner.py or the runtime load() path, which is outside this task's scope. It is now listed under Known limits in PROJECT_STATUS.md. diagnose flags it, and audio-hooks set repairs it (verified).
  • Behaviour change on writes. For an array or string file, a write such as set now repairs it to the defaults (the old bytes are snapshotted to the backups first). Before this PR it failed with INTERNAL_ERROR.
  • Unreadable files are not reported. A file that exists but can't be read (permissions, a directory) is not flagged as corrupt. That case wasn't in scope.
  • Diagnose severity is a judgement call. PREFS_FILE_CORRUPT is reported as an error (diagnose exits 1), not a warning. The reasoning is that the user's settings are silently not in effect.
  • Other read-only commands still ignore extra args. status and diagnose themselves still accept extra arguments without complaint (both handlers take _args and ignore it). That wasn't one of the two listed limits, so it is untouched. It is not recorded in PROJECT_STATUS.md either.
  • Branch name. The branch is claude/cli-known-limits-ee12t7 rather than claude/cli-known-limits, because the session is restricted to that branch.

Checklist

  • My commits follow the Conventional Commits style
  • I have tested my changes locally
  • I have updated documentation where needed
  • I agree to follow this project's Code of Conduct (left for the maintainer)

Generated by Claude Code

… corrupt preferences file

Closes two "Known limits" in docs/PROJECT_STATUS.md that broke the CLI
contract (JSON in, JSON out, stable error codes):

- `logs tail` validates through _check_args: `--n N` (positive integer) and
  `--level info|warn|error|debug`; anything else is INVALID_USAGE.
  `backup list` takes nothing and `backup show` one id; every backup
  subcommand validates before the preferences object is built.
- `status` reports `preferences_file` (corrupt, problem, warning) and
  `diagnose` reports the new error finding PREFS_FILE_CORRUPT when
  user_preferences.json is not valid JSON or not a JSON object. Both stay
  read-only: the file is detected, never repaired or backed up.
- A JSON array/string preferences file no longer crashes status, diagnose
  and get with INTERNAL_ERROR; they report the defaults, as for an
  unparseable file. The hook runner still fails on such a file; recorded
  as a new known limit.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018HwdjLSbSkWMCv6DTyoecy
@ChanMeng666
ChanMeng666 merged commit 59a267d into master Oct 20, 2026
12 checks passed
@ChanMeng666
ChanMeng666 deleted the claude/cli-known-limits-ee12t7 branch October 20, 2026 22:22
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.

2 participants