Skip to content

fix: smoke-test 3.6.0 bugs + quality gates#46

Merged
chenliuyun merged 6 commits into
mainfrom
fix/smoke-test-3.6.0-bugs
May 15, 2026
Merged

fix: smoke-test 3.6.0 bugs + quality gates#46
chenliuyun merged 6 commits into
mainfrom
fix/smoke-test-3.6.0-bugs

Conversation

@chenliuyun
Copy link
Copy Markdown
Collaborator

Summary

  • Fix 9 bugs found in 3.6.0 smoke test (strict int parsing, dry-run stdout discipline, excess arguments, format support, catalog search validation)
  • Add parseStrictInt() as single source of truth for integer validation (rejects leading zeros, whitespace; accepts +N)
  • Migrate all dry-run/diagnostic output from stdout to stderr across 10 commands
  • Add scripts/lint-stdout.sh quality gate (dry-run channel enforcement, bare Number() detection, command test coverage)
  • Add scripts/verify-release.mjs pre-release invariant checker (MCP tool count, audit version, doc consistency)
  • Add tests/contract/format-matrix.test.ts (health + catalog × 5 formats)
  • Add catalog-coverage doctor check

Test plan

  • npm test — 2385 tests pass
  • bash scripts/lint-stdout.sh — all checks pass
  • node scripts/verify-release.mjs — no errors
  • npm run typecheck — clean

🤖 Generated with Claude Code

chenliuyun added 3 commits May 15, 2026 19:33
- parseStrictInt: reject leading zeros (032) and enforce consistent
  integer parsing across setBrightness, setColorTemperature, intRange
- allowExcessArguments(false): extra positional args now error globally
- devices status: accept variadic positional IDs (batch path reuse)
- dry-run output: move human-readable messages to stderr (devices,
  expand, batch, plan) so --format pipelines stay clean
- health --format: wire up resolveFormat/renderRows for all formats;
  handle --format id with a clean UsageError instead of a stack trace
- health audit breakdown: classify statusCode-less errors as 'client'
  (expected) instead of 'unknown' (unexpected)
- catalog search: reject empty keyword
- doctor: add catalog-coverage check (warn on device types missing
  from the static catalog)
- format matrix contract tests (15 cases)
…t errors unexpected

- Remove .trim() from parseStrictInt so ' 50' and '50 ' are rejected
  (whitespace in the value is user error, not a CLI artifact)
- Allow + prefix in regex so +50, +1, +100 are accepted as valid ints
- Revert audit breakdown: only EXPECTED_ERROR_CODES (161/171/190) are
  expected; statusCode-less errors (timeouts, DNS) remain unexpected
  so health degrades correctly during real outages
- Add lint-stdout.sh and verify-release.mjs scripts
- Convert bare Number() in entry-point validators to parseStrictInt()
- Move dry-run output to stderr in install/uninstall/scenes/policy/rules
- Fix corresponding test assertions (stdout → stderr)
@chenliuyun
Copy link
Copy Markdown
Collaborator Author

Code review

Found 3 issues:

  1. rules explain data field incorrectly moved to stderr. The dry_run: true line at line 896 is a structured output field in a key-value report (alongside hysteresis:, maxFiringsPerHour:, suppressIfAlreadyDesired:, last fired: — all on stdout). Moving only this field to stderr breaks the report format. The lint script (lint-stdout.sh) false-positive matched console.log.*dry.run but this is a rule property, not a dry-run diagnostic.

if (detail.hysteresis) console.log(`hysteresis: ${detail.hysteresis}`);
if (detail.maxFiringsPerHour !== null) console.log(`maxFiringsPerHour: ${detail.maxFiringsPerHour}`);
if (detail.suppressIfAlreadyDesired) console.log(`suppressIfAlreadyDesired: true`);
if (detail.dryRun) console.error(`dry_run: true`);
console.log(`last fired: ${detail.lastFired ?? '(never)'}`);

  1. health.ts renames breakdown key from 'unknown' to 'client' (line 113) — a JSON output schema change visible to consumers of health check --json. The test fixture at tests/commands/health-check.test.ts:153 still references breakdown: { 'unknown': 4 } and was not updated to match.

for (const e of errorEntries) {
const code = e.statusCode !== undefined ? String(e.statusCode) : 'client';
breakdown[code] = (breakdown[code] ?? 0) + 1;

  1. New catalog-coverage doctor check added in doctor.ts but not listed in README.md (line 635). CLAUDE.md pre-release checklist item fix: derive CLI version from package.json (v1.3.2) #4 says: "Doctor checks — compare [...] against the list in README.md. All checks must be listed."

{ name: 'catalog', description: 'catalog loads', run: () => checkCatalog() },
{ name: 'catalog-coverage', description: 'all cached device types have catalog entries', run: () => checkCatalogCoverage() },
{ name: 'catalog-schema', description: 'catalog vs agent-bootstrap version aligned', run: () => checkCatalogSchema() },

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

@chenliuyun chenliuyun merged commit e8488cd into main May 15, 2026
11 checks passed
@chenliuyun chenliuyun deleted the fix/smoke-test-3.6.0-bugs branch May 15, 2026 15:51
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