[IngridMorstrad]

Problem / Motivation

Goal: An MCP install no longer fails when the user's global mcp.json is not UTF-8 or not readable: the agent-spec sync skips that file, as it already does for a missing or malformed one.

After the capability manager installs a server, POST /api/capability/mcp/install (and POST /api/mcp/discover/install for the capability provider) runs _sync_mcp_to_agent(name, True), which copies the server's spec out of ~/.kiro/settings/mcp.json. If that file was saved in a legacy code page, reading it raises UnicodeDecodeError: 'utf-8' codec can't decode byte 0xe9. If the gateway may not read it, reading raises PermissionError. The endpoint answers 500 {"error": "internal error"} for an install that already happened.

Why it matters

~/.kiro/settings/mcp.json is a file people edit by hand, and on Windows Notepad's "ANSI" save is one accented character away from making it invalid UTF-8. The sync's own except already says what to do with a file it cannot use, which is skip it. Today only a missing or malformed file reaches that branch. Any other read error turns into a failed request after the earlier step has already taken effect.

Not a goal

• Reading or repairing the non-UTF-8 file. It is skipped exactly like a file with invalid JSON.
• The other reads of the same file in handlers/mcp.py and handlers/mcp_probe.py. They have the same narrow except, and the open fix(dashboard): read the global mcp.json off the event loop #24093 already edits those files. See Sibling sweep.
• The kirocrew.json reads at the top of both sync functions (lines 119 and 303). That is our own file, and the user does not hand-edit it.

What changed (motivation → approach → change)

An install fails when the global mcp.json cannot be read as UTF-8 text. Root cause: both global-file reads in dashboard/handlers/mcp_agent_sync.py catch only (FileNotFoundError, json.JSONDecodeError). UnicodeDecodeError is a ValueError and PermissionError is an OSError other than FileNotFoundError, so neither reaches the skip branch. Change: both reads now catch (OSError, UnicodeDecodeError, json.JSONDecodeError).

Read	Before, with an unreadable global file	After
_sync_mcp_to_agent_unlocked, copy of one server's spec	raises, and the install answers 500	the copy is skipped and the spec is left unchanged
_sync_mcp_to_agent_batch_unlocked, gdata	raises	gdata = {}: servers already in the spec are still enabled, and servers that exist only in the global file are not copied

Backwards compatibility

Compatible: only exceptions that used to escape are caught now. A file that parsed before still parses the same way.

Removes nothing: no route, field, record or validator is removed or tightened.

Tests

Proof: PROVEN test/test_mcp_agent_sync_unreadable_global.py::test_single_sync_skips_the_copy[latin-1], test/test_mcp_agent_sync_unreadable_global.py::test_single_sync_skips_the_copy[permission-denied], test/test_mcp_agent_sync_unreadable_global.py::test_capability_install_answers_ok[latin-1], test/test_mcp_agent_sync_unreadable_global.py::test_capability_install_answers_ok[permission-denied], test/test_mcp_agent_sync_unreadable_global.py::test_batch_sync_still_syncs_the_servers_it_has[latin-1] (head 66dd8b6)

The new test/test_mcp_agent_sync_unreadable_global.py runs each test twice. One run uses a real latin-1 mcp.json ("Café" in an env value). The other uses a path whose read_text raises PermissionError. The sync calls are wrapped so that an escaping read error becomes a pytest.fail, which makes the unfixed code fail on an assertion and not on a crash.

Test	Locks in
test_single_sync_skips_the_copy	enabling a server whose spec is only in the unreadable file leaves kirocrew.json unchanged and raises nothing
test_batch_sync_still_syncs_the_servers_it_has	the batch enable still lifts disabled and mounts a server that is already in the spec, and does not copy one that exists only in the unreadable file
test_capability_install_answers_ok	POST /api/capability/mcp/install answers 200 after the package install

Red/green: with mcp_agent_sync.py reverted to origin/main, all 6 fail. The sync tests fail with Failed: sync raised on an unreadable global mcp.json: UnicodeDecodeError(...) or PermissionError(13, ...), and the install test fails with AssertionError: {"error": "internal error", ...}. With the fix, all 6 pass. prove.py --per-hunk reverts each hunk on its own, and each revert is caught.

Determinism: the tests use no clocks, sleeps or shared state, only tmp_path. On the final head, the file passed 3/3 runs at -n 4.

Local gates: black, isort, flake8, mypy (on the changed module), check_black_formatting.py, check_sync_io_in_async.py, check_subprocess_encoding.py, comment-history, brand name, docs-lint.sh, the flake-pattern ratchet and semgrep --config semgrep/ (1.78.0, diff baseline: 0 findings) all pass. In local-gate.py, the backend related set gives 124 passed. Its frontend lane could not start in this worktree because node_modules is missing, and this diff has no frontend change. test_mcp_sync_agent.py, test_mcp_handler_facade.py, test_agent_home_isolation_derived_specs.py and test_secret_uri_launch_paths.py plus the new file give 202 passed.

Manual verification

N/A — unit coverage is sufficient. The tests call the real sync functions and the real install handler against real files in tmp_path, and nothing else changed.

Screenshots / video

N/A — no UI change.

Related Issues

Closes #24186

Sibling sweep

Swept: git grep -nE "_GLOBAL_MCP_JSON\.read_text" -- src/
Sibling: src/kiro_crew/dashboard/handlers/mcp_agent_sync.py -- changed here
Sibling: src/kiro_crew/dashboard/handlers/mcp.py -- not affected -- same gap at :325, :526 (toggle), :623, :697, :790, :880 and :960, outside this PR's goal, and open #24093 edits this file
Sibling: src/kiro_crew/dashboard/handlers/mcp_probe.py -- not affected -- same gap at :374, :466, :600 and :662, outside this PR's goal, and open #24093 edits this file
Sibling: src/kiro_crew/dashboard/change_cards_readers.py:191 -- not affected -- already catches (OSError, ValueError)

The mcp.py and mcp_probe.py reads are left for a follow-up once #24093 lands. The toggle (api_mcp_toggle, mcp.py:526) reads the file itself before it calls this sync, so a non-UTF-8 file still fails the toggle at that earlier read.

Pattern harvest

Rule candidate: semgrep
Pattern: <path>.read_text(encoding="utf-8") inside a try whose handlers catch FileNotFoundError and json.JSONDecodeError but not OSError and UnicodeDecodeError (or ValueError). For a user-edited file, both are the same "cannot use it" case. On origin/main, git grep finds 12 except (FileNotFoundError, json.JSONDecodeError) sites under src/kiro_crew/dashboard/handlers/, and 10 remain after this PR.

Checklist

• At most two commits (one is the norm), with a Conventional Commits title (feat|fix|docs|style|refactor|perf|test|chore|ci|build|revert: ...)
• Existing tests pass and new tests added for new functionality
• Self-review completed; code follows project style guidelines
• Documentation updated (if applicable)
• No secrets, credentials, or internal references in the diff

[github-actions]

UX Review (Opus 5.5, fork) — ⏭️ skipped

Revision 66dd8b6a53bc758d2a6a0eeb0b587ebfa9cde9f8 touches no user-facing surface (no changes under website/ or committed screenshots), so the UX review was skipped. Does not block merge.

[github-actions]

First Principles Review (Opus 5.5, fork) — ✅ PASS

Premise-level review of 66dd8b6a53bc758d2a6a0eeb0b587ebfa9cde9f8 via the fork AI-review pipeline — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS does not.

First-Principles-Verdict: PASS

It fixes the cause in the module it touches. Unfixed siblings are declared and deferred to #24093.

Notes

• Item 1 — symptom-level relative to the class: the same narrow except (FileNotFoundError, json.JSONDecodeError) is still in handlers/mcp.py (4 sites) and handlers/mcp_probe.py (1 site). I grepped that literal across dashboard/handlers/ and got 9 sites in total, not the 12 the description claims. The description says "a non-UTF-8 file still fails the toggle at that earlier read", which is deferred and named in the description.

What this change ships

Inventory (3 items) — 3 justified

Intent: an MCP install no longer answers 500 when the hand-edited global mcp.json is not UTF-8 or not readable. FIX.

1. Enabling a server whose spec is only in an unreadable global mcp.json skips the copy instead of raising — justified
2. A batch enable treats an unreadable global mcp.json as empty and still enables servers already in the spec — justified
3. POST /api/capability/mcp/install answers 200 after the package install when the global file is unreadable — justified

[FIRST-PRINCIPLES-REVIEWED] 66dd8b6

Description read

[DESCRIPTION-READ] cca88f5c4f5e106f70726f41da16bb5ad09e3e0cc7a8346c677a852856d03825

That sha256 names the PR description this verdict read. Recompute it with .github/scripts/pr-description-capture.sh; if it differs, the description changed after this verdict was formed and any finding drawn from the description is unproven.

[github-actions]

Design Review (Opus 5.5, fork) — ✅ PASS

Design-level review of 66dd8b6a53bc758d2a6a0eeb0b587ebfa9cde9f8 via the fork AI-review pipeline — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS does not.

Design-Verdict: PASS

Nothing blocks: a narrow fix that sends two more read failures into the skip path that already existed.

Notes

• A skipped copy makes POST /api/capability/mcp/install answer 200 even though the server never reaches the agent spec. The user sees success and no tools. A malformed JSON file already behaves this way, but a log line or a warning in the response at the skip would make it visible.
• The toggle at mcp.py:526 still fails earlier on the same file. Fix the 10 sibling sites in the follow-up after fix(dashboard): read the global mcp.json off the event loop #24093, not one at a time.

[DESIGN-REVIEWED] 66dd8b6

Description read

[DESCRIPTION-READ] 115d66b4fa0edd5d82bbb5e25cdb9ba3246f2599856945d3e8e56786010aedcc

That sha256 names everything this verdict read: the PR description, plus the 1 evidence file(s) the workflow collected from it and handed this review. It covers that evidence because the media strip replaces every attachment URL with the same placeholder, so an edit swapping one attachment for another would leave the description's captured text identical while what this review judged changed. Recompute it with .github/scripts/pr-description-capture.sh over the same inputs; if it differs, the description or that evidence changed after this verdict was formed and any finding drawn from either is unproven.

[github-actions]

GPT 6.1 Review (fork) — ✅ no blocking findings

Reviewed 66dd8b6a53bc758d2a6a0eeb0b587ebfa9cde9f8 via the fork AI-review pipeline; updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 66dd8b6

[github-actions]

Opus 5.5 Review (fork) — ✅ no blocking findings

Reviewed 66dd8b6a53bc758d2a6a0eeb0b587ebfa9cde9f8 via the fork AI-review pipeline; updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] 66dd8b6

[IngridMorstrad]

On the Design Review note about logging the skip: both reads now take the existing skip branch, which has never logged for a missing or malformed global mcp.json. That is the behavior on main today, and this PR does not change it. A logger.warning there would also log on a plain missing file, the common case, so it would need its own discriminator. Leaving it out keeps this fix to the two except tuples. On the may_skip_gate_now note: as the review says, a failure there skips the copy and leaves nothing half-applied, so no change is needed.

[KiroCrewCaptain]

Tier 1 auto-approve: fix (2 files). Criteria: no conflict, no requested changes, security path denylist clean, design-doc gate clean, security checklist all-NO, no reviewer-lane findings, readiness signal fresh for this head SHA, declared category matches the diff. Category: an unreadable or non-UTF-8 global mcp.json no longer fails the MCP install's agent-spec sync (closes #24186), with a test.