feat(mcp): add get_log and get_state tools, redact credentials from get_apps

[springfall2008]

This is an automated draft PR generated from issue #4768 — a maintainer should review it before merging.

Fixes #4768

Summary

The MCP server already covered part of #4768 via get_apps/get_config, but had no way to reach predbat.log or Predbat's internal state — the artefacts a bug report normally has to carry. This adds both, and closes the credential leak that made pointing a cloud AI at get_apps unwise.

get_log

Serves predbat.log (plus the rotated predbat.1.log), with optional filter (all/info/warnings/errors, default warnings), search, hours and max_lines. Lines come back oldest-first with severity tagged; max_lines defaults to 500 and is capped at 5000 so a request can't pull a 10MB log through the protocol. Continuation lines with no timestamp of their own (tracebacks) inherit the timestamp of the entry they belong to, so hours keeps or drops a multi-line entry as a whole.

get_state

Deliberately not a debug-yaml download. Sizing a real dump (coverage/cases/predbat_debug_agile1.yaml, 313 top-level keys, 5.3MB) shows 272 of those keys are under 1KB and total under 10KB between them, while 11 keys are 85% of the file and every one is a per-minute series. So the split is:

• Called with no arguments, get_state returns every variable within the per-variable budget — 371 keys / ~2.4KB (~590 tokens) on a fresh instance — and describes the rest.
• Large values go into an omitted section carrying type, length, sample keys and min/max/mean, so the caller can ask for one by name rather than guessing. A size guard, not a size limit.
• keys, filter (regex) and max_bytes narrow or widen the request. Collections over 200 entries are refused on entry count alone, so a 2,880-entry per-minute dict is never serialised just to discover it doesn't fit.
• Values json.dumps can't encode (datetimes, inverter objects) are coerced rather than failing the call.

Credential handling

• get_apps now redacts by default, matching what the web UI's own apps.yaml download already did. Pass masked: false to opt out. Previously _execute_get_apps returned self.base.args verbatim.
• mask_secret_args widened to match secret and token alongside _key/password. sigenergy_app_secret, solis_api_secret, deye_app_secret, alphaess_app_secret, solis_access_token, gateway_mqtt_token and mcp_secret were all being served in the clear. *_expires_at is exempt — a token expiry is exactly what you want visible when debugging a dead cloud integration.
• The debug yaml's exclusion list moved to utils.is_debug_excluded_key and is now shared with create_debug_yaml, so get_state can never return anything a debug dump would not (ha_interface, components, secrets, the URL caches, db*, credentials). Verified against a live instance: the widened rule newly excludes zero attributes on a baseline config, so it costs debug dumps nothing and only bites on cloud-integration credentials.

Shared filtering

Log level rules (classify_log_line/log_line_included) are now shared between get_log and /api/log so the two views of the same log can't drift. The web handler's behaviour is unchanged — its HTML escaping and search highlighting are pinned by a regression test.

Docs

docs/components.md lists every MCP tool in a table, documents get_log's and get_state's arguments and the redaction behaviour, and adds a short "ask an AI assistant to review your setup" section describing the workflow the issue asked for.

Testing

• New apps/predbat/tests/test_web_mcp.py (registered as web_mcp), 10 groups: mask_secret_args key matching, is_debug_excluded_key, read_predbat_log rotation, the shared filter helpers and timestamp parsing, the get_state value helpers, get_apps redaction, get_log, get_state, tools/list registration, and a regression test that /api/log still filters, escapes and highlights as before.
• get_state was probed against a real PredBat instance, not just a fake. That surfaced a genuine bug the fake missed: measure_state_value used None as its "too large" sentinel, so the 14 state variables that are legitimately None were being reported as omitted. Fixed by returning an explicit fits flag, with a regression test.
• tools/triage_test.sh web_mcp — passed. tools/triage_test.sh debug_cases — passed (run explicitly since create_debug_yaml's filter changed; it's marked slow and skipped by --quick).
• cd coverage && ./run_pre_commit — all hooks passed, and the run_all --quick suite it runs passed in full.
• interrogate fails at 89% on main today and is not a pre-commit hook; changed files each moved up or stayed level, and everything added here is documented.

Notes

No debug-yaml download tool. An MCP server cannot write to the client's filesystem and everything it returns lands in the client's context, so a 5MB dump has no viable path through the protocol; get_state reaches the same data incrementally. The "button in the web UI" form of the request needs a billing/key-custody decision and is left for @springfall2008.

🤖 Generated with Claude Code

[Copilot]

🟡 Changes recommended

There are a few concrete API/docs correctness and robustness issues (ordering description mismatch, missing argument validation, and over-broad secret-key exclusion of non-secret expiry metadata) that should be fixed before merging.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds MCP tooling to help AI-assisted debugging/config review by exposing Predbat logs and a size-guarded view of internal state, while reducing the chance of credential leakage via MCP.

Changes:

• Add MCP tools get_log (bounded, filterable predbat.log) and get_state (budgeted internal state with omitted summaries).
• Redact credential-like values from get_apps by default and share the debug-yaml exclusion logic via utils.is_debug_excluded_key.
• Refactor the web log endpoint to reuse shared log classification/filter helpers; add docs and a new test_web_mcp suite.

File summaries

File	Description
docs/components.md	Documents MCP tools, including new get_log/get_state and redaction behavior.
apps/predbat/web.py	Reuses shared log reader + log-level classifier/include rules for /api/log.
apps/predbat/web_mcp.py	Implements MCP get_log/get_state, adds default-redacted get_apps, and registers new tools.
apps/predbat/utils.py	Adds shared secret-key detection, debug exclusion helper, log reader, and log line classification/timestamp parsing.
apps/predbat/userinterface.py	Switches debug-yaml generation to shared is_debug_excluded_key.
apps/predbat/unit_test.py	Registers the new web_mcp tests.
apps/predbat/tests/test_web_mcp.py	Adds coverage for masking, debug exclusion, log rotation/filtering, MCP tools, and /api/log regression behavior.

Review details

Suppressed comments (2)

apps/predbat/web_mcp.py:1293

• get_log casts max_lines and hours directly to int/float; invalid client values currently surface only as a generic exception string. Returning explicit argument errors for bad max_lines / hours makes the tool easier to consume and avoids exposing Python exception details.

            search_term = str(arguments.get("search", "") or "").lower().strip()
            max_lines = int(arguments.get("max_lines", MCP_LOG_DEFAULT_LINES))
            max_lines = max(1, min(max_lines, MCP_LOG_MAX_LINES))
            hours = arguments.get("hours", None)
            hours = float(hours) if hours is not None else None

apps/predbat/utils.py:172

• read_predbat_log() reads log files with the platform default encoding. If the log contains any non-UTF-8 bytes (or mixed encodings), this can raise UnicodeDecodeError and break both /api/log and get_log. Using an explicit UTF-8 decode with errors='replace' is more robust for log ingestion.

def read_predbat_log(logfile=PREDBAT_LOG_FILE, logfile_prev=PREDBAT_LOG_FILE_PREV):
    """
    Return the contents of predbat.log, prefixed with the rotated previous log when one exists.
    """
    logdata = ""
    if os.path.exists(logfile):
        with open(logfile, "r") as f:
            logdata = f.read()
    if os.path.exists(logfile_prev):
        with open(logfile_prev, "r") as f:
            logdata = f.read() + "\n" + logdata
    return logdata

• Files reviewed: 7/7 changed files
• Comments generated: 4
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[springfall2008]

Thanks — five of the six findings were good and are fixed in 15d4f66. One I've not actioned, with reasoning below.

Fixed

• max_lines ordering description. Correct catch, and my wording was the bug: the tool keeps the most recent matching lines but returns them oldest-first. Fixed in the tool schema, the tool description and docs/components.md.
• Argument validation. Added parse_number_argument and compile_filter_argument, which raise a named MCPArgumentError reported as {"success": false, "error": "'max_lines' must be a whole number, got 'lots'"} rather than a raw exception string. This matters more than usual here because the consumer is an AI assistant that can only correct itself if told which argument was wrong. Applied to get_state and get_log as flagged, and also to get_apps/get_config/get_entities, which have carried the same unvalidated re.search since before this branch — leaving three tools with a worse error path than the two new ones would be a trap. Compiling the pattern once also drops ~370 re.search calls per get_state.
• Log encoding (suppressed comment). The best of the six. One non-UTF-8 byte in predbat.log would raise UnicodeDecodeError and take out both /api/log and get_log. Now decoded as UTF-8 with errors="replace", with a test that feeds it a latin-1 degree sign. Worth noting this is pre-existing — web.py has always opened the log without an explicit encoding; centralising it in this PR is what makes it a one-line fix.

Not actioned: the is_secret_key exemption breadth

The claim is that partner_token_expiry (Axle) and _token_birth (Ohme) will now be excluded from debug dumps, losing useful context. Both attributes exist, but they are set on the component objects — axle.py:74 and ohme.py:786 — not on PredBat. create_debug_yaml and get_state walk self.__dict__ plus self.inverters, and components is in DEBUG_EXCLUDE_LIST, so neither attribute has ever reached a debug dump, before or after this change.

I checked the generalisation rather than just the two examples: scanning every PredBat mixin for self.*secret* / self.*token* assignments turns up three hits, all on OctopusAPI(ComponentBase) rather than the Octopus mixin that PredBat inherits. No PredBat base attribute matches at all, which is why probing a live instance showed the widened rule newly excluding zero keys.

The underlying concern about substring matching is still fair as future-proofing, so SECRET_KEY_EXEMPT_SUFFIXES now covers _expiry, _expires, _expiration and _birth alongside _expires_at — which would handle exactly those two names if such an attribute ever moved onto the base.

Verification: run_pre_commit green (all hooks + run_all --quick in full), tools/triage_test.sh debug_cases green, and the web_mcp suite grew two groups covering the validation helpers across all five tools and the log-decoding fix.