Skip to content

feat(tools): add P2 typed CLI-text read tools (TP-3) - #135

Merged
millsmillsymills merged 2 commits into
phase1from
feat/p2-cli-typed-reads
Jun 23, 2026
Merged

feat(tools): add P2 typed CLI-text read tools (TP-3)#135
millsmillsymills merged 2 commits into
phase1from
feat/p2-cli-typed-reads

Conversation

@millsmillsymills

Copy link
Copy Markdown
Contributor

Implements the P2 band and its TP-3 prerequisite from docs/superpowers/plans/2026-06-23-tool-coverage-to-90pct.md. P0/P1 landed in #130; this is the next phase.

What's here

  • TP-3 — cli_typed helper (tools/_common.py): benign-only wrapper over client.cli_exec, reusing the existing _io_lock-held exchange. It refuses any gated command, so a transmit/destructive subcommand can never hide behind a typed tool.
  • Four typed CLI-text read tools (tools/cli_typed.py, gate none, USB-only):
    • flipperzero_core_statusfree + uptime → heap fields, heap_free_bytes, uptime.
    • flipperzero_fs_treestorage tree <path> → depth-tagged file/dir entries, complete/truncated.
    • flipperzero_loader_listloader list → app names.
    • flipperzero_i2c_scani2c → responding 7-bit addresses.
  • Each returns the raw output alongside parsed fields (firmware CLI text is free-form/unversioned).
  • All raise the typed CLI text mode unavailable error over the WiFi bridge (TP-2).

Grounded in real device output

Parsers were built and validated against output captured live over USB (Momentum firmware), and the four tools were exercised end-to-end through the MCP server against the device. Notably the i2c output is a positional grid, not Found: 0x.. lines — the parser maps each non-- cell's (row, column) to its 7-bit address.

Deferred (intentional)

  • crypto_status (5th P2 tool in the plan): the device crypto command is a usage dump (encrypt/decrypt/has_key/store_key) with no benign read-only status subcommand. Shipping it would return raw help text, which the plan's §1 says does not earn coverage. Left out pending a real read command.
  • Golden fixtures (§6.4): the byte-replay fixtures are per-firmware and need both Momentum and Official hardware; only Momentum is on hand. Parser shapes are locked by unit tests against captured output in the meantime. Follow-up once Official firmware is available.

Coverage

P2 lands +4 (not +5) typed counting tools → matrix now 28 registered tools, new cli_typed group.

Tests

  • tests/unit/test_tool_cli_typed.py — parser tests against real captured output (incl. the i2c grid and the app-open uptime case) + WiFi refusal.
  • tests/integration/test_cli_typed_reads.pyusb-marked live reads.
  • Schema-matrix drift test + GATE map updated. Full non-integration suite: 276 passed.

Note

While capturing reference output, a storage tree /ext (whole volume) flooded USB and briefly wedged the CLI link mid-stream — it recovered on its own. fs_tree's docstring warns to pass a specific subdirectory; bounding large-tree captures cleanly is the TP-4a streaming-primitive work in a later phase.

🤖 Generated with Claude Code

millsmillsymills and others added 2 commits June 23, 2026 13:38
Add the benign-only cli_typed helper (TP-3) and four USB-only typed
CLI-text read tools that parse device output into structure:
core_status (free+uptime), fs_tree (storage tree), loader_list, and
i2c_scan. Each returns raw output alongside the parsed fields; all raise
the typed CLI-unavailable error over the WiFi bridge.

Parsers are grounded in real device output captured over USB (Momentum):
the i2c parser maps the scan grid's (row, column) position to the 7-bit
address rather than matching literal 0x.. tokens. crypto_status from the
plan is deferred: the device crypto command is a usage dump with no
benign read-only status subcommand.

Extends the GATE map and regenerates the tool-schema matrix (28 tools).
Unit tests assert the parsers against captured output and the WiFi
refusal; integration tests (usb-marked) exercise the live reads.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Constrain the i2c grid row regex to 0-7 and bound columns to 16 so
malformed output cannot surface addresses outside the 7-bit space. Add
tests for multi-device i2c, out-of-range rows, the fs_tree truncated
path, and the gated-command refusal in cli_typed.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@millsmillsymills

Copy link
Copy Markdown
Contributor Author

Review summary (/review-pr)

Parallel agents: code-quality, silent-failure, test-coverage.

Must-fix: none. The feared cli_typed None-fall-off does not fire — _classify_client_error is NoReturn and ty validates the -> CliExecResult annotation; the pattern matches ~25 existing call sites.

Fixed in 4a385aa:

  • _parse_i2c hardened: row regex [0-9a-fA-F][0-7], column bound < 16 — malformed grid output can no longer surface addresses outside the 7-bit space.
  • New tests: multi-device i2c, out-of-range row, fs_tree truncated (completed=False) path, and the gated-command refusal in cli_typed (sev-9 defense-in-depth control, previously zero coverage).

Gates: ruff + ty clean, 281 passed / 13 deselected.

Deferred to followup (not blocking): device-unreachable path test for the shared cli_typed helper, ANSI-embedded fixtures for _parse_i2c/_parse_tree, and _parse_tree sizeless-file / ANSI-before-indent depth edges.

@millsmillsymills
millsmillsymills merged commit df27c2a into phase1 Jun 23, 2026
5 checks passed
@millsmillsymills
millsmillsymills deleted the feat/p2-cli-typed-reads branch June 23, 2026 20:56
millsmillsymills added a commit that referenced this pull request Jun 23, 2026
Add device-unreachable and catch-all error-path tests for the shared
cli_typed helper, asserting it surfaces a ToolError rather than an
implicit None on any failure. Add ANSI-bearing fixtures exercising the
_strip_ansi path in the free/i2c/tree parsers and fix the inaccurate
_FREE comment. Cover _parse_tree edges: a file row with no trailing
size, and the leading-escape depth limitation.

Closes #136

Co-authored-by: Claude Opus 4.8 (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

Development

Successfully merging this pull request may close these issues.

1 participant