feat(powers): browse, install and remove Kiro Powers (inert; activation split out)

[kyleseaman]

What

Adds a Powers surface to the left rail's Apps group: browse the upstream registry, install a bundle, list what's installed, remove it.

This PR was split. It previously also contained activation (trust grant, enable/disable, MCP registration, generated docs skill). That half is deferred to a follow-up so this one is reviewable — see Why it was split below.

Installed Powers are inert — by construction, not by a flag

This is the security property the PR rests on, so it's stated as a claim a reviewer can check:

Half	Why it can't reach anything
MCP servers	The bundle's mcp.json is never parsed for specs — only tested for presence, to label the Power mcp vs knowledge. A declared command has no path to _set_kirocrew_entry or any other execution site.
Guidance docs	No skill is materialized. POWER.md / steering/*.md stay inside powers_dir(), and nothing else reads that directory — skills.py, context.py, agent.py contain no reference to it. Third-party markdown cannot enter agent context.

Consequently installed.json carries no trusted / enabled field. A trust flag that gated nothing would assert a control the code doesn't implement.

The UI says this plainly rather than leaving the absence unexplained: an Inactive badge plus a note that no MCP server is registered and no guidance is loaded. There's deliberately no greyed-out toggle, since that would imply an activation path that doesn't exist. PowersTab.test.tsx asserts the absence of any switch or trust control, and TestInertInstall asserts no MCP config is written, no SKILL.md exists anywhere, and the record has only {source, installedAt}.

Why it was split

The combined PR wasn't converging under review: findings went 9 → 10 → 24 across three rounds, because each round's transactional hardening became the next round's finding surface. Nearly all of that complexity existed to make enable/disable/revoke safe — so it left with activation.

Reviewing activation separately also surfaced a real bug that would have shipped: includeMcpJson is false, so sessions read the rendered ~/.kiro/agents/kirocrew.json, not <data home>/mcp.json. Every other MCP mutation path calls rebuild_agent_config() (handlers/mcp.py, mcp_custom.py) and uninstall additionally strips rendered entries, because the rebuild merges additively. The activation code did neither — so enable was a no-op and revoke left the server live. That bug is now moot rather than fixed: the redesigned activation (see Follow-up below) writes no MCP config at all, so there is nothing to rebuild. It is recorded because it is the reason the combined PR could not converge, and docs/system-specs/modules/powers.md keeps it under Deferred: activation (pull model) as the constraint the new shape sidesteps.

One review finding was resolved by deletion rather than hardening: the bash-side Powers write matcher (flagged as bypassable via cd + relative path, open(...,'w'), variable redirects) existed solely to stop trusted: true forgery. With no trust flag, it's gone. The file-edit gate protection stays, because .marketplace-cache.json still decides which repository an id resolves to.

Fixed while capturing screenshots: the registry never worked live

Re-shooting the browse screenshots against a real instance returned zero Powers, which turned out not to be a capture problem.

All three provider bodies were read with a single await resp.content.read(cap + 1). StreamReader.read(n) returns whatever is buffered when it wakes and does not loop to fill n, so every body larger than one wire chunk was silently truncated. Measured against the live upstreams:

Upstream	One read(cap + 1)	Chunk loop
kiro.dev/powers/	57 KiB	649 KiB
api.github.com/.../contents/	9.8 KiB	25.6 KiB

The symptoms did not look like read bugs. The marketplace scrape parsed a partial document, found no cards, and marked itself unavailable; the official provider fed truncated JSON to json.loads and raised ProviderUnavailableError("Unterminated string starting at: line 1 column 5577"). Both surfaced as provider unavailable, so browsing the real registry rendered an empty catalogue. The per-file byte cap was equally unenforceable — a short read never reached it, so an oversized file was accepted as a truncated one.

Fixed with a read_capped helper that loops to the cap; callers keep their own len(body) > cap check, so overflow policy (raise vs. truncate) stays at each call site.

Why 95 tests missed it. Every existing test stubs _http_get_json / _http_get_bytes / _fetch_html — the three functions that contain the read. The seam was mocked one level above the defect. The new TestBoundedStreamRead therefore runs a loopback HTTP server, since a mocked stream can be made to return a whole body in one call, which is exactly the behaviour that does not hold on a socket. All five fail with the fix reverted, reproducing the production error text.

Consequence: provider overlap became reachable

With the official provider working for the first time, the merge path matters. Dedup is by canonical repository URL and provider order decides ownership (official first) — but a dropped duplicate was taking its metadata with it. The official provider lists a GitHub directory, so description / author / category are structurally empty for it, and the 26 of 82 overlapping Powers rendered authorless. The losing duplicate now donates any facet the winner left blank; populated fields are never overwritten.

scope is deliberately not merged, and the resulting divergence is documented rather than hidden: the providers disagree (monorepo membership vs. authorship facet), so an AWS-authored Power that is also mirrored into the official monorepo answers to the Official chip while its card shows AWS as the author.

Publisher icons

Registry cards show the publisher icon the marketplace listing already exposes — present on 76 of 76 cards. Without it the browse grid was a table of names next to a directory that looks like a storefront.

The URL is scraped from a third-party page and ends up as an img source, so its origin is an allowlist, not a coherence check, enforced at one chokepoint (marketplace.valid_icon_url): https only, exactly the Kiro asset host, and a /powers/icons/ path prefix. Rejected: scheme downgrade, foreign host, suffix-spoofed host (…kiro.dev.evil.com), path escape, and javascript:. It is applied twice — when scraping and when reading the disk cache back — because the cache is on-disk state this spec already treats as attacker-reachable, and a poisoned cache entry would otherwise put an arbitrary origin in front of the user's session. A refused icon degrades the card; it never drops the Power. No CSP change was needed: img-src already allows https:.

Icons are also what justifies the blank-facet merge above. The official provider lists a GitHub directory and has no icon to report, yet it wins the dedup for the 26 overlapping Powers — so without enrichment a third of the catalogue would render the fallback while the marketplace held a real icon for it. The client falls back to the Kiro Powers mark when the field is absent, and again on onError when the host fails to serve the image, so the grid never shows broken-image glyphs. aws-mcp in the screenshot is a genuine fallback: it exists only in the official monorepo.

The capture harness counts icons with naturalWidth > 0, so a screenshot full of placeholders cannot pass as the feature working.

Security controls retained

• fetch.py — HTTPS-only host allowlist (github.com, api.github.com, raw.githubusercontent.com), no git clone, traversal + symlink rejection, 8 MiB / 4 MiB / 200 files / depth-8 bounds enforced incrementally, bounded timeout, temp tree removed on every failure path including cancellation.
• Allowlist copy of exactly POWER.md + mcp.json + steering/*.md. Vetting only the selected root isn't enough: an allowed ancestor (a home dir) can hold a valid POWER.md and ~/.ssh, and a recursive copy would relocate it under the powers dir. An allowlist makes that unrepresentable.
• resolve_install_source refuses sensitive paths and audits the denial — the success-path SEL event wouldn't fire on a refusal, so a repeated probe of credential paths would otherwise leave no trace.
• powers/ write-protected at the agent file-edit gate (reads still allowed — the dashboard renders it).
• Atomicity — install and remove are each one blocking transaction: a single callable that takes the cross-process lock, mutates, rolls itself back on failure, and unlocks. Compensation in the coroutine cannot be made correct (a cancelled task re-raises from every later await, so awaited compensation never runs; waiting synchronously stalls the loop), so five helpers that existed to manage that are deleted rather than tuned. Install re-parses the staged POWER.md under the lock because the source is caller-owned and mutable; remove destroys the staged bytes before dropping the record, so a failed delete can never orphan a bundle.
• Confinement by pinned descriptor — _root_lock opens the powers root O_NOFOLLOW|O_DIRECTORY once and every rename/delete goes through it, so a root swapped for a symlink mid-transaction cannot redirect a mutation outside the store. A validate-then-mutate check cannot promise that however tightly the two are placed. _SUPPORTS_DIR_FD gates the POSIX path; Windows keeps the path check, where creating a symlink requires elevation.

Screenshots

Powers in the Apps section of the left rail:

Browse the registry — real data, 82 entries fanned out across both providers (32 from the official kirodotdev/powers monorepo, 76 from the marketplace mirror, 26 overlapping and deduplicated by repository URL), with scope chips:

Provider outage surfaces as stale-with-banner, not as an empty catalogue. Captured with the marketplace upstream made unresolvable and its disk cache aged past the TTL, so the provider takes the real transport-failure branch and serves expired entries while the official provider keeps working:

The Installed view — one MCP Power, one Knowledge Power, both showing the Inactive badge and the inert note, with no toggle or trust control anywhere:

These are real: both Powers were installed through the actual POST /api/powers/install folder path on an isolated instance, and the API returned kind=mcp for the bundle shipping mcp.json and kind=knowledge for the steering-only one — so the new presence-test labelling is exercised end-to-end, not only in unit tests. The capture harness asserts switches === 0 on the rendered page, making the inert claim a build-time check rather than something you have to take on trust.

Verification

• 95 Powers tests (test_powers.py, test_powers_registry.py, test_powers_security_paths.py); 689 pass across the security or powers selection
• mypy clean, isort clean, flake8 clean
• tsc clean; PowersTab + PowersSurface suites green
• The two new backend guarantees are revert-tested — the staged-POWER.md identity check and the remove ordering each fail when the guard is removed

Follow-up: activation (feat/powers-activation) — redesigned around the IDE's model

None of the following is in this diff. It is written down so the inert install reads as a deliberate stage rather than an omission.

This section previously described a push design: merge the Power's MCP servers into the rendered agent config, materialize its docs into a skill, and gate both behind a trusted flag in installed.json. That design is withdrawn. Reading the IDE's own implementation (kiro-team/kiro-extension, src/extension/powers/**) shows upstream does neither — it exposes tools and lets the agent pull:

Upstream tool	Shape
listPowers	returns installed Powers with mcpServers names and keywords, without starting anything
usePower	(powerName, serverName, toolName, arguments) — namespaced dispatch
readPowerSteering	(powerName, fileName) — one .md, on demand, path-validated
configurePowers	ships behind ENABLE_WEBUI = false

Their steering rules are independently identical to ours: .md only, no separators, no leading dot, validated join.

What the pull model deletes from the plan

Four tools on the existing kirocrew-core MCP server (power_list, power_learn, power_use, power_steering) and nothing written to ~/.kiro/agents/kirocrew.json. Each item below was a required deliverable of the push design and is now simply absent rather than solved:

• The includeMcpJson constraint disappears. It dominated the old plan — sessions read the rendered config, so every writer had to call rebuild_agent_config() and uninstall had to strip rendered entries by hand. Nothing is written, so nothing must be rebuilt, and enable can no longer be a silent no-op.
• No power-<power>-<server> namespace, so the injectivity problem (power-git- also prefixes power-git-helper-srv) and the ownership record a non-prefix purge needed both vanish. Power and server stay call arguments.
• No skill materialization. Guidance comes back as a tool result for the one file the agent named — third-party markdown never enters a session unasked, never competes for the steering budget, and cannot be keyword-triggered.
• No trusted / enabled fields, so installed.json as shipped in this PR needs no migration. The "readers must treat a missing key as false" obligation this PR's spec previously imposed on the follow-up is withdrawn.
• No enable/disable transaction, so the install/remove transaction does not grow two more state transitions over the same lock.

Consent replaces the trust flag

The first power_learn / power_use for a Power in a session raises an approval naming the Power, the server, and the resolved command; per-session Trust auto-approves that Power for that session. This is the same conclusion PR #518 reached when its round-4 change deleted bespoke trust grants and routed through core approvals — a second bespoke trust store for Powers would repeat the mistake that change corrected. A durable "always allow" is deliberately out of scope for the first activation change.

Where the risk moves

The push design spread risk across config merging, purge correctness and context injection. The pull design collapses it into one chokepoint — the process power_use spawns — which must carry spec validation before spawn (no shell, no argv from tool arguments), environment discipline rather than ambient inheritance, per-call timeout plus the existing mandatory redactor on output, a SEL event per invocation, and a bash-layer write guard for the bundle's mcp.json. That last one carries over from the push design with a sharper rationale: mcp.json becomes the source of an argv KiroCrew spawns, and it is read on every call rather than once at enable.

One deliberate divergence from upstream

The IDE has no consent prompt — install is one click and the tools are callable. KiroCrew adds one because the IDE always has a human at the keyboard while KiroCrew runs turns from cron, Slack and Discord. This divergence is intentional and should not be "fixed" toward parity.

Layout divergences, recorded as choices

Upstream lives at ~/.kiro/powers (KIRO_POWERS_HOME overrides) with bundles under installed/<name>/ and a versioned envelope — {version: "1.0.0", installedPowers: [{name, registryId, autoInstalled?}], dismissedAutoInstalls: [...]} — plus registries/, registry-repos/ and repos/. This module uses our data home (~/.kiro/crew/powers), a name-keyed map with {kind, ref} provenance, and bundles at <name>/. Different trees, so no collision — and no interop: a Power installed through KiroCrew is invisible to the IDE. Acceptable while nothing is activated; first thing to revisit if kiro-cli ships a native runtime. Their Power type carries iconUrl, which is the field this PR's cards use.

Full design, including the tool table and the open questions, is in docs/system-specs/modules/powers.md under Deferred: activation (pull model) and Upstream Powers implementation (observed, not inferred).

Publisher icons

Registry cards show the publisher icon the marketplace listing already exposes — present on 76 of 76 cards. Without it the browse grid was a table of names next to a directory that looks like a storefront.

The URL is scraped from a third-party page and ends up as an img source, so its origin is an allowlist, not a coherence check, enforced at one chokepoint (marketplace.valid_icon_url): https only, exactly the Kiro asset host, and a /powers/icons/ path prefix. Rejected: scheme downgrade, foreign host, suffix-spoofed host (…kiro.dev.evil.com), path escape, and javascript:. It is applied twice — when scraping and when reading the disk cache back — because the cache is on-disk state this spec already treats as attacker-reachable, and a poisoned cache entry would otherwise put an arbitrary origin in front of the user's session. A refused icon degrades the card; it never drops the Power. No CSP change was needed: img-src already allows https:.

Icons are also what justifies the blank-facet merge above. The official provider lists a GitHub directory and has no icon to report, yet it wins the dedup for the 26 overlapping Powers — so without enrichment a third of the catalogue would render the fallback while the marketplace held a real icon for it. The client falls back to the Kiro Powers mark when the field is absent, and again on onError when the host fails to serve the image, so the grid never shows broken-image glyphs. aws-mcp in the screenshot is a genuine fallback: it exists only in the official monorepo.

The capture harness counts icons with naturalWidth > 0, so a screenshot full of placeholders cannot pass as the feature working.

Security controls retained

• fetch.py — HTTPS-only host allowlist (github.com, api.github.com, raw.githubusercontent.com), no git clone, traversal + symlink rejection, 8 MiB / 4 MiB / 200 files / depth-8 bounds enforced incrementally, bounded timeout, temp tree removed on every failure path including cancellation.
• Allowlist copy of exactly POWER.md + mcp.json + steering/*.md. Vetting only the selected root isn't enough: an allowed ancestor (a home dir) can hold a valid POWER.md and ~/.ssh, and a recursive copy would relocate it under the powers dir. An allowlist makes that unrepresentable.
• resolve_install_source refuses sensitive paths and audits the denial — the success-path SEL event wouldn't fire on a refusal, so a repeated probe of credential paths would otherwise leave no trace.
• powers/ write-protected at the agent file-edit gate (reads still allowed — the dashboard renders it).
• Atomicity — install and remove are each one blocking transaction: a single callable that takes the cross-process lock, mutates, rolls itself back on failure, and unlocks. Compensation in the coroutine cannot be made correct (a cancelled task re-raises from every later await, so awaited compensation never runs; waiting synchronously stalls the loop), so five helpers that existed to manage that are deleted rather than tuned. Install re-parses the staged POWER.md under the lock because the source is caller-owned and mutable; remove destroys the staged bytes before dropping the record, so a failed delete can never orphan a bundle.
• Confinement by pinned descriptor — _root_lock opens the powers root O_NOFOLLOW|O_DIRECTORY once and every rename/delete goes through it, so a root swapped for a symlink mid-transaction cannot redirect a mutation outside the store. A validate-then-mutate check cannot promise that however tightly the two are placed. _SUPPORTS_DIR_FD gates the POSIX path; Windows keeps the path check, where creating a symlink requires elevation.

Screenshots

Powers in the Apps section of the left rail:

Browse the registry — real data, 82 entries fanned out across both providers (32 from the official kirodotdev/powers monorepo, 76 from the marketplace mirror, 26 overlapping and deduplicated by repository URL), with scope chips:

Provider outage surfaces as stale-with-banner, not as an empty catalogue. Captured with the marketplace upstream made unresolvable and its disk cache aged past the TTL, so the provider takes the real transport-failure branch and serves expired entries while the official provider keeps working:

The Installed view — one MCP Power, one Knowledge Power, both showing the Inactive badge and the inert note, with no toggle or trust control anywhere:

These are real: both Powers were installed through the actual POST /api/powers/install folder path on an isolated instance, and the API returned kind=mcp for the bundle shipping mcp.json and kind=knowledge for the steering-only one — so the new presence-test labelling is exercised end-to-end, not only in unit tests. The capture harness asserts switches === 0 on the rendered page, making the inert claim a build-time check rather than something you have to take on trust.

Verification

• 95 Powers tests (test_powers.py, test_powers_registry.py, test_powers_security_paths.py); 689 pass across the security or powers selection
• mypy clean, isort clean, flake8 clean
• tsc clean; PowersTab + PowersSurface suites green
• The two new backend guarantees are revert-tested — the staged-POWER.md identity check and the remove ordering each fail when the guard is removed

Follow-up: activation (feat/powers-activation)

None of the following is in this diff. It is written down so the inert install reads as a deliberate stage rather than an omission, and so a reviewer can judge whether the seams this PR leaves are the right ones. The engineering constraints are recorded in docs/system-specs/modules/powers.md under Deferred: activation; what follows adds the product shape and separates what is decided from what is not.

Must ship together

Activation is not a flag, because five things are only correct in combination:

1. A trust grant gating both halves. MCP servers (a local subprocess) and guidance docs (which enter agent context through SkillsLoader keyword triggers, treated as security-relevant in this repo). Gating only MCP would make "Disabled" untrue for a Knowledge Power.
2. power-<power>-<server> namespacing with an ownership-exact purge. Both segments admit hyphens, so prefix matching is unsound — power-git- also matches power-git-helper-srv, owned by git-helper. installed.json gains a record of the exact entries each Power registered.
3. Rendered-agent-config rebuild on install, enable, disable, revoke and remove — includeMcpJson is false, so writing only <data home>/mcp.json is a no-op on enable and leaves a revoked server live (detail in Why it was split).
4. SEL audit events for every permission decision: power_trust_grant / power_trust_revoke, power_enable / power_disable.
5. A shell-level write guard for powers_dir(). While install is inert the file-edit gate is proportionate; once a trust flag is authoritative and bundle contents are executable, a bash-side matcher becomes load-bearing.

Proposed user-facing flow

Installed-and-Inactive stays the resting state after install — activation is always a second, explicit act.

• Enable is the one new control on an installed Power. Its first use raises a consent step that names what is being granted: the specific MCP servers the bundle declares, or that its markdown will enter agent context.
• Knowledge Powers take the same consent as MCP ones. Third-party guidance steering an agent is a capability grant, not a display preference, so it does not get a quieter path.
• Disable deregisters and rebuilds; Revoke drops the grant so the next enable re-prompts.
• The current Inactive badge and inert note are replaced by real state, not supplemented — this PR deliberately ships no greyed-out toggle, so activation adds the control rather than lighting one up.

Open questions, not yet decided

• Trust scope — per-Power, or per-source (all Powers from one repo/publisher)? Per-source is fewer prompts and a coarser blast radius.
• Propagation to live sessions — the config rebuild is process-level; whether an already-open session picks up a newly enabled Power without a restart needs to be stated as a guarantee, not left to whatever the implementation happens to do.
• Bulk revoke — whether there is a single "disable all Powers" control, and whether it is reachable when the dashboard is the thing misbehaving.
• Disable semantics — deregister only, or also stop a running server subprocess.

Migration

Records written by this PR carry only {source, installedAt}. Activation's readers must treat a missing trusted / enabled / mcpServers key as false/empty, so anything installed by this version keeps loading and lands as Inactive — upgrading never confers an implicit grant.

[github-actions]

GPT 5.6 Review — 🔴 changes requested (blocking)

GPT 5.6 found at least one blocking issue that must be resolved before merging c31980857ed7a7e84af4dec08837c77d06d7ac84.

This comment is updated in place on each push.

BLOCKING -- src/kiro_crew/powers.py:1349 -- Filesystem probe runs on the event loop
dest = self._power_path(name)
Network-mounted KIROCREW_HOME -> install/remove -> Path.is_symlink() blocks -> gateway requests and heartbeat freeze.
Fix: Move _power_path resolution into each existing executor transaction.
[GPT-REVIEWED] c319808
[BLOCK-MERGE] c319808
False positive or not applicable? A repository writer can comment:
/ai-review override gpt c31980857ed7a7e84af4dec08837c77d06d7ac84: <one-sentence reason>

[github-actions]

Design Review (Fable 5) — 🟡 CONCERNS

Advisory design-level review of c31980857ed7a7e84af4dec08837c77d06d7ac84 — updated in place on each push; does not block merge.

Design-Verdict: CONCERNS

Sound staged design, but the on-disk store diverges from the upstream IDE's layout at the cheapest moment to converge — before any user data exists.

Watch

• Store-layout divergence is a soft one-way door. The PR itself records that upstream lives at ~/.kiro/powers with a versioned envelope while this module uses ~/.kiro/crew/powers with a name-keyed {kind, ref} map ("no interop: a Power installed through KiroCrew is invisible to the IDE… first thing to revisit if kiro-cli ships a native runtime"). Once users hold installed.json records, converging costs a migration; today it costs a path and a schema. Given the follow-up already reshapes activation around the IDE's observed model, deferring this divergence is the one choice that gets more expensive by waiting.
• User value is entirely deferred to the unmerged activation branch. Every install lands permanently "Inactive"; the UI explains it, but if feat/powers-activation stalls or its open questions (trust scope, session propagation) reshape it, the dashboard is advertising a storefront whose installs do nothing. Humans should confirm the follow-up is committed-to before this ships to users, or accept the interim.

Suggestions

• Adopt upstream's directory layout and record envelope now (paths + schema only, keep your provenance fields), so activation inherits interop instead of a migration.

[DESIGN-REVIEWED] c319808

[github-actions]

Opus 5 Review — ✅ no blocking findings

Reviewed c0b6fbd58057d1bb031b6f871d9238ba66c767b6 — this comment is updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] c0b6fbd

Verdict parsed from the review's SHA-scoped output markers for commit c0b6fbd58057d1bb031b6f871d9238ba66c767b6.

False positive or not applicable? A repository writer can comment:
/ai-review override fable c0b6fbd58057d1bb031b6f871d9238ba66c767b6: <one-sentence reason>

[kyleseaman]

Round 1 dispositions — all four GPT 5.6 findings and all 10 CodeQL alerts addressed in ab6743df. Every finding was legitimate; no rebuttals.

HIGH — stale MCP entries survive a reinstall (powers.py:479)

Fixed, and the root cause was broader than the reported symptom. Entries were being derived from the bundle's mcp.json, so any server a previous bundle registered but the replacement no longer lists was left registered — and if it had already been trusted and enabled, it kept executing while the Power was reset to trusted=False, enabled=False.

Now existing_power_server_names() enumerates the live MCP config by power-<name>- prefix and purge_power_mcp_entries() clears them before the replacement's servers are registered disabled. remove_power() had the same latent weakness and now purges by prefix too.

Regressions: test_reinstall_purges_servers_absent_from_new_bundle (trusts + enables the old server first, so it fails loudly if the stale entry survives) and test_remove_purges_by_namespace_not_bundle.

HIGH — folder install path not checked against is_sensitive_path() (handlers/powers.py:133)

Fixed. Added resolve_install_source(), which resolves symlinks and .. to an absolute real path, refuses protected locations via the shared security.is_sensitive_path() guard, and requires a real directory. Applied in the handler before anything reads or walks the path, and again in the store as defense in depth.

Regressions: test_resolve_install_source_refuses_sensitive_path, test_install_refuses_sensitive_source, test_resolve_install_source_rejects_missing_and_file.

HIGH — blocking filesystem work on the event loop (powers.py:466)

Fixed. _assert_safe_tree() (walks the whole bundle up to the file/byte caps), the rmtree/copytree swap, the skill materialization, and remove_power's recursive deletes now run in maintenance_executor() via run_in_executor, matching the pattern in messaging/identity.py and mcp_gateway/gatewayd.py.

MEDIUM — fetched temp bundle leaked (handlers/powers.py:150)

Fixed with a finally that removes the fetched tree on success and failure alike (the removal itself is also offloaded). Only the fetched dir is removed — a caller-supplied folder source is never deleted.

Regressions: test_fetched_bundle_removed_on_success and test_fetched_bundle_removed_on_failure. These are the first handler-level tests for this feature; the store had coverage but the handler did not.

CodeQL — 10 × py/path-injection in powers.py

Real, and one shared root cause: name originates in third-party POWER.md content or a URL path segment, and reached filesystem paths at ten sites with validation only at parse time, too far from use to be a barrier.

Consolidated into a single validate-and-confine chokepoint, PowersStore._power_path(): it re-validates against the strict name pattern at the point of use, then requires the resolved path to sit directly under the powers root, so no traversal or symlinked component can redirect a read, write, or delete outside it. _skill_dir() got the same treatment for the skills root. All ten call sites now route through one of the two; the only remaining path joins are constants.

Regression: test_power_path_confined_to_powers_root, parameterized over ../escape, a/b, /abs, ., .., and "", asserting both helpers reject each.

Verification

• test/test_powers.py → 41 passed (13 new this round); test/test_powers_registry.py → 27 passed
• full backend suite (-n 8) → 16,946 passed, 65 skipped, 5 xfailed
• flake8, isort --check-only, mypy src/kiro_crew/ (CI-parity venv, 479 files) → all clean
• No frontend files changed this round, so tsc -b / vitest (384 files, 4,422 passed) are unchanged from the previous green run

The 11 remaining backend failures are pre-existing and unrelated — they reproduce identically on a pristine origin/main worktree (macOS-targeted test_sandbox_argv, hardlink-inode, Windows model-registry).

[github-actions]

Arbiter — ⏳ review pending

Arbiter is waiting for Opus-5 review output for c31980857ed7a7e84af4dec08837c77d06d7ac84; this replaces any stale verdict from the previous commit.

Second-order review for c31980857ed7a7e84af4dec08837c77d06d7ac84; this comment is updated in place on each push.

False positive or not applicable? A repository writer can comment:
/ai-review override arbiter c31980857ed7a7e84af4dec08837c77d06d7ac84: <one-sentence reason>

For a broader accepted-risk deferral, apply defer-longterm and explain why.

[kyleseaman]

Round 2 dispositions — all three GPT 5.6 findings addressed in 4ff7709a. All legitimate; no rebuttals.

MEDIUM — marketplace powers were un-installable (fetch.py:166)

Confirmed and fixed. This was the most consequential of the three: it made the feature's headline path dead. PowersTab submits {kind: 'registry', ref: p.id, provider: p.provider}, so a marketplace card sent a bare id with provider="marketplace", hit the non-official rejection, and surfaced as HTTP 500. Every marketplace-listed Power — i.e. the whole 76-entry mirror the Browse view exists to show — failed to install. Only official-monorepo slugs worked.

Fixed with _resolve_ref(), which maps a provider-scoped id to a fetchable ref before parsing: marketplace ids resolve through MarketplacePowersProvider.fetch_detail() to that card's canonical GitHub tree URL; official slugs and explicit URLs pass through unchanged; an unknown provider is refused.

Resolution deliberately happens server-side rather than by having the client post githubUrl (the other option the finding offered). The frontend already holds that URL, but trusting it would let a caller pair a marketplace id with an arbitrary repo. Resolving from the provider means the id the user clicked unambiguously determines what gets installed. No frontend change was needed.

Regressions: test_marketplace_id_resolves_to_card_repo (asserts exa resolves to exa-labs/kiro-power-exa, not the official monorepo), test_marketplace_id_without_source_is_refused, test_unknown_provider_refused, test_official_and_url_refs_pass_through.

HIGH — blocking file writes on the event loop (fetch.py:262)

Fixed. Both the per-file write_bytes (up to 200 files × 4 MiB) and the per-directory mkdir now run in maintenance_executor(). This is the same class as round 1's finding on powers.py; round 1 covered the install path but not the download path.

HIGH — reinstall destroyed the existing Power before copying (powers.py:579)

Fixed, and worth noting this one was introduced by my round-1 refactor moving the delete-then-copy into _replace_tree — the sequencing hazard was pre-existing but I carried it forward unexamined.

The bundle is now copied into a sibling .staging-<name>-<pid> directory and validated (POWER.md still present) before anything touches the live tree; the swap is then os.replace(dest, backup) → os.replace(staging, dest), with the backup restored if the swap fails, and scratch dirs cleaned up in a finally. The live bundle is never removed until its replacement is fully written. Scratch names are dot-prefixed so they can never collide with a valid power name (is_safe_power_name rejects a leading dot), and list_powers() enumerates installed.json rather than the directory, so they are never visible as Powers.

Regression: test_failed_reinstall_preserves_existing_bundle — completes the copy then raises OSError, asserting the original POWER.md content and the install record both survive.

Verification

• test/test_powers.py + test/test_powers_registry.py → 75 passed (18 new this round)
• full backend suite (-n 8) → 16,950 passed, 65 skipped, 5 xfailed
• flake8, isort --check-only, mypy src/kiro_crew/ (CI-parity venv, 479 files) → all clean
• No frontend files changed this round, so tsc -b / vitest (384 files, 4,422 passed) stand from the last green run

12 backend failures remain, all pre-existing and unrelated — verified by reproducing them on a pristine origin/main worktree. They are host-environment cases: this machine cannot create user namespaces (unshare(CLONE_NEWUSER) → EPERM), which fails the test_sandbox_argv and test_sandbox_no_isolation families, plus macOS-targeted, hardlink-inode, and Windows model-registry tests.

[kyleseaman]

Round 3 — GPT 5.6 now passes. Addressed three items in 7e26ead7; two remain open pending a maintainer decision (called out at the bottom).

Arbiter item 2 / Opus MEDIUM — cross-power purge prefix collision

Fixed, and the fix needed a schema addition. power-<power>-<server> is ambiguous because both segments allow hyphens, so purging git matched power-git-helper-srv, owned by git-helper.

Ownership is now authoritative rather than inferred: installed.json records the exact namespaced entries each Power registered ("mcpServers": [...]), and existing_power_server_names() subtracts entries claimed by any other installed Power from its prefix candidates. This keeps the purge-by-prefix property the stale-bundle case requires while making the boundary exact.

Worth noting the first attempt at this fix failed its own test: I filtered against installed.json before checking that the record actually persisted mcpServers — it didn't, it was derived from the bundle at load time. The regression test caught it immediately, which is why persisting ownership is now part of the fix rather than an assumption.

Regression: test_purge_does_not_delete_another_powers_server — installs git-helper then git, removes git, asserts power-git-helper-srv survives.

Design Review finding 2 — marketplace cache in the shared temp dir

Fixed: the cache moved from Path(tempfile.gettempdir()) / "kirocrew-powers-marketplace.json" to powers_dir() / ".marketplace-cache.json".

This one is a direct consequence of my round-2 change and deserves flagging: before _resolve_ref existed, the cache was display-only. Making it the resolution source for which repository gets installed silently promoted a /tmp file with a predictable name into an install-decision oracle that any local process could pre-seed. The reviewer caught a hazard I introduced while fixing something else.

Opus LOW — handler pre-check on the event loop

Fixed for consistency: the handler's resolve_install_source pre-check now runs in maintenance_executor(), matching the store's own vetting.

Verification

• test/test_powers.py + test/test_powers_registry.py → 76 passed
• full backend (-n 8) → 16,951 passed, 65 skipped, 5 xfailed
• flake8, isort --check-only, mypy (479 files) → clean
• no frontend changes this round; tsc / vitest (4,422 passed) stand

12 pre-existing failures remain, all reproduced on pristine origin/main (this host cannot create user namespaces, plus macOS/Windows-targeted cases).

---

Two items deferred pending a maintainer decision

1. Arbiter item 1 / Design finding 1 — the knowledge half of a Power bypasses the trust gate. Confirmed and I am not disputing it: _materialize_skill runs unconditionally at install, so an untrusted, disabled Power's third-party POWER.md and steering/*.md are live in agent context via SkillsLoader keyword triggers, while the UI reports Untrusted / Disabled. The PR's stated invariant ("install never auto-enables") is therefore only true of the MCP half. For a Knowledge Power the enable toggle is currently cosmetic.

The fix direction is clear (materialize on enable, remove on disable/untrust) but there is a real product question inside it: should enabling a Knowledge Power require a trust grant? Today trust means "may this run a local subprocess". If prompt injection into agent context is also in scope, trust should gate docs too — which adds a consent click for docs-only Powers and diverges further from the IDE's one-click flow. That is a maintainer call, not mine, so I have not guessed at it.

2. The last CodeQL alert (1 high, py/path-injection at powers.py:350). Down from 10. The remaining one is Path(src).resolve(strict=True) inside resolve_install_source, i.e. the folder install kind accepting an arbitrary API-supplied path by design. It is guarded by is_sensitive_path() plus a directory check, but CodeQL does not recognise that as a barrier and there is no containment root that would be meaningful for "a folder I cloned a Power into". Notably nothing currently uses this kind — PowersTab only ever sends kind: 'registry'. Options are to drop folder from the API for now (smallest surface, zero present cost, returns later with the native folder picker this repo prefers), confine it to an allowlisted root, or rebut as by-design. I would rather not suppress a high-severity alert without agreement.

Once both are settled I will land them together with docs/system-specs/modules/powers.md (Arbiter item 3 / Design finding 3), so the spec states the final trust semantics rather than being written twice — including the upstream-convergence intent the Arbiter asked to fold in.

[kyleseaman]

Round 4 — all four GPT 5.6 findings plus all three Arbiter items addressed in e19a918c (rebased onto current main, 25 commits). No rebuttals; every finding was legitimate.

HIGH — sensitive-path check covered only the selected root (powers.py:590)

Correct, and the suggested fix is better than what I had. Vetting the selected root with is_sensitive_path() does nothing about descendants: an allowed ancestor such as a home directory can hold a valid POWER.md while also holding ~/.ssh or governance files, and copytree would relocate all of it under the agent-readable powers directory.

Rather than add a per-descendant check, the copy is now an allowlist: _copy_power_files() copies exactly POWER.md, mcp.json, and steering/*.md and nothing else, refusing symlinks and size-capping each file. Nothing outside the Power contract is read or written, which makes the whole class unrepresentable instead of merely guarded — and it removes the need to reason about what a "sensitive descendant" is.

Regression: test_install_copies_only_contract_files seeds the source with id_rsa, .ssh/id_ed25519, notes.txt and a non-markdown file inside steering/, then asserts the installed tree contains only POWER.md and steering/ok.md.

HIGH — backup deleted before MCP and metadata writes commit (powers.py:647)

Correct; my round-2 atomic swap stopped one step short. The backup was removed in _replace_tree's finally, i.e. before purge_power_mcp_entries, _set_kirocrew_entry, and _write_installed ran, so a failure in any of those left the previous bundle already destroyed.

Split into _stage_tree / _rollback / _commit: the swap moves the live tree aside, every post-swap write runs inside a try, a failure restores the previous tree, and the backup is deleted only after all writes succeed.

Regression: test_failed_reinstall_preserves_existing_bundle now fails a post-swap write (_write_installed) rather than the copy, and asserts the original bundle and record both survive.

HIGH — rmtree on the event loop in the fetch failure handler (fetch.py:337)

Fixed: offloaded to maintenance_executor(). Third occurrence of this class in this PR (install path in round 1, download writes in round 2, failure cleanup now) — the spec section now states the rule for the module so the next contributor doesn't reintroduce it.

MEDIUM — docs materialized at install, bypassing the trust gate

This is the same issue as Arbiter item 1 and Design finding 1. Three reviewers converging on it is answer enough, so it is now implemented rather than deferred.

_materialize_skill no longer runs at install. _apply_skill_state follows the enabled flag: enabling materializes the generated skill, disabling removes it, and revoking trust retracts it. Install is now genuinely inert — no MCP entry enabled, no skill on disk.

The trust question I raised earlier is resolved in the stricter direction, and the reason is that the weaker option is not self-consistent: set_enabled now requires trust for every Power kind, not only MCP ones. Docs are third-party content injected into agent context, which this repo treats as a prompt-injection surface; gating only the MCP half would leave "Disabled" untrue for a Knowledge Power. It also matches what the UI already did (the enable toggle only ever rendered once trusted), so the backend was the lenient half, not the UI.

Frontend copy corrected accordingly — the trust prompt previously claimed a docs-only Power "runs third-party code as a local subprocess" and then contradicted itself with "no MCP servers". It now states the actual risk per kind and that the Power stays inert until trusted and enabled.

Regressions: test_knowledge_power_install_no_mcp (asserts no skill at install, present after trust+enable, gone after disable), test_untrust_retracts_docs_skill, test_enable_requires_trust_for_knowledge_power_too.

Arbiter item 3 / Design finding 3 — missing spec

Added docs/system-specs/modules/powers.md (+ index row). It documents the seven-route API, the installed.json schema with contract-vs-incidental fields called out, the trust semantics above, the namespacing and why purge cannot be prefix-only, install atomicity, the fetch.py security controls, the marketplace coupling (including that per-card category is absent upstream), and the upstream-convergence intent the Arbiter asked for: detect-and-defer if kiro-cli ships a native Powers runtime, with the ownership record making the handover auditable and the trust gate preserved rather than dropped.

Arbiter item 2 (purge collision) was fixed in round 3.

Verification

• test/test_powers.py + test/test_powers_registry.py → 79 passed
• full backend (-n 8) → 17,224 passed, 62 skipped, 5 xfailed
• flake8, isort --check-only, mypy (480 files) → clean
• tsc -b clean; vitest run → 389 files, 4,503 passed, 3 skipped

Rebased onto current main (25 commits). The 4 remaining backend failures are pre-existing and unrelated — all reproduce on a pristine origin/main worktree (3 in test_dashboard_origin.py from main's recent commits, 1 in test_skills.py).

CodeQL went green this round. I want to be precise about why: I did not target it. The last alert was on resolve_install_source, and the executor indirection added for the Opus LOW likely broke the dataflow path CodeQL was tracing. The allowlist copy above is the change that actually addresses the underlying concern.

[kyleseaman]

Round 5 — all five GPT 5.6 findings plus Claude's advisory MEDIUM addressed in 34186e19. No rebuttals; every finding was legitimate.

HIGH — blocking I/O on the event loop (powers.py:647, marketplace.py:221/316)

Fixed. The install path parsed an API-selected mcp.json inline, and the marketplace TTL cache did its disk read and write from _entries() on the loop. All three now go through maintenance_executor().

This is the fourth site of this class in the PR (install tree-walk/copy, download writes, failure cleanup, now bundle parse + cache I/O). The module spec section now states the rule explicitly so the next contributor doesn't reintroduce it.

MEDIUM — install resolved through an uncached provider (fetch.py:183)

Fixed, and the consequence was worse than a cache miss: browsing used the process-wide provider (constructed with the persistent cache) while install constructed a bare one. During a marketplace outage a card could stay visible from cached data yet installing it would fail — install disagreeing with what the user was shown. Resolution now goes through _get_registry().get("marketplace"), falling back to a bare provider only when the registry isn't initialised (unit tests).

MEDIUM — availability latched off permanently (base.py:146)

Fixed. available_providers filters on is_available() before search() runs, so a transient failure setting _available = False meant _entries() could never execute again and the provider could never restore itself until a gateway restart. is_available() now reports available again after a 300s cooldown so the next search re-probes and re-latches.

Regression: test_marketplace_availability_recovers_after_cooldown.

MEDIUM — catalog advertised directories with no POWER.md (official.py:145)

Fixed. The name-only denylist would advertise any future infrastructure directory as installable, with the failure surfacing only at install time on the missing required file.

Verification uses one recursive git-tree request rather than ~32 per-directory contents calls, which would burn the unauthenticated rate limit. A truncated or unavailable tree falls back to the name heuristic rather than showing an empty catalog.

Worth noting this finding also caught a documentation lie: docs/system-specs/modules/powers.md already claimed the official provider filtered "on the presence of POWER.md". It didn't. The code now matches the spec rather than the other way round.

Regressions: test_official_search_requires_power_md (asserts a scripts/ dir with no POWER.md is excluded) and test_official_search_falls_back_when_tree_truncated.

MEDIUM — trust could not be revoked through the API (handlers/powers.py:242)

Fixed. PowersStore.set_trusted() has always implemented revocation (it force-disables), but the endpoint hard-rejected {"granted": false}, so a compromised Power's persisted grant was unrevocable via the API — contrary to the documented lifecycle. The handler now accepts a boolean and passes it through.

Claude MEDIUM (advisory) — "Installed" badge missed registry-installed Powers

Fixed, at the root rather than the symptom. The install mutation sent ref: p.id (a slug) so source.ref held a slug, while isInstalled compared it against p.githubUrl — that branch could never match, leaving dedup reliant on name === id, which holds for the official provider but not for marketplace cards.

Rather than only changing the comparison, the backend now records the resolved ref as provenance (resolve_power_ref is exported and called before fetch; resolution is idempotent for refs that are already URLs). So source.ref is the repository the bundle actually came from, which is both more accurate provenance and makes the repo comparison meaningful. The frontend compares repositories case-insensitively, keeping the name check as a secondary signal.

This changes a contract field's semantics, so the spec's installed.json section was updated to say so.

Claude's LOW (read path list_powers() does small bounded sync I/O) is deferred deliberately: it sits below the rule's "large IO/walk" bar and matches other dashboard read handlers. Noted in the spec as a symmetry option if the installed set grows.

Also in this push

Powers iconography is now consistent: the registry/installed empty states used a generic lucide Zap; they use the Kiro ghost+bolt PowerIcon (the same mark as the rail row), so every place the subject is a Power shows the Powers mark.

Verification

• test/test_powers.py + test/test_powers_registry.py → 83 passed (4 new this round)
• full backend suite → 17,232 passed, 63 skipped, 5 xfailed
• isort, flake8, mypy (480 files) → clean
• tsc -b clean; vitest run → 390 files, 4,512 passed, 3 skipped
• Rebased onto current main.

Two unrelated red checks, neither touched by this PR:

• Backend Tests (Windows) (4) — test_token_auth.py::test_signing_secret_concurrent_first_init_converges asserts POSIX mode bits (33206 & 511 == 384) plus a WinError 5 rename in the same module. This is the known Windows portability gap that open PR fix(test): close three Windows CI gaps in the new Windows backend line #444 (fix/windows-test-gaps) addresses; this PR does not touch token_auth.
• 3 local test_dashboard_origin.py port-default failures reproduce on a pristine origin/main worktree (host-specific: this machine's gateway port differs from the asserted default).

[kyleseaman]

Follow-up: evidence for the "Windows red is inherited" claim

My round-5 comment asserted the Backend Tests (Windows) (4) failure was pre-existing. That assertion was published before I had actually verified it — the check I intended to run was blocked mid-round by a local safety policy and I stated the conclusion anyway. Verified now:

• main is red on the same shards. CI run 30175122734 (head 9f0a3c0f, branch main): Backend Tests (Windows) (4) → failure, Backend Tests (Windows) (3) → failure; shards 1 and 2 pass.
• This PR does not touch the failing module. git diff origin/main..HEAD --name-only lists 29 files — the Powers backend/frontend, its two test modules, the spec, and screenshots. Nothing under the auth-test module that raises the POSIX mode-bit assertion.

So the Windows failures on this PR are inherited from the base branch, and the tracked fix is open PR #444 (fix/windows-test-gaps). The conclusion is unchanged; it is now evidenced rather than asserted.

[kyleseaman]

Round 6 — all three GPT 5.6 findings addressed in 96553730 (rebased onto current main). All legitimate; no rebuttals. Two of the three are incomplete work of mine from earlier rounds rather than fresh ground.

HIGH — rollback restored only the bundle tree (powers.py)

Correct, and this is the third iteration on the same reinstall-atomicity problem: round 2 added staging, round 4 held the backup until the writes commit, and this round completes it. The purge runs before the metadata write, so a post-swap failure left a previously-working install with its MCP entries removed or partially replaced — and possibly an unintended new server registered — while installed.json still described the old bundle.

The prior state is now snapshotted before any mutation: each Power-owned MCP entry's full spec (via a new _read_kirocrew_entry, preserving each entry's enabled/disabled state) plus the installed.json record. On failure _restore_config drops whatever the attempt registered and re-applies the snapshot, restoring the record (or removing it if the Power was new), alongside the existing tree rollback.

Regression: test_failed_reinstall_restores_mcp_and_metadata — installs, trusts and enables a Power, then fails the metadata write during a reinstall whose bundle ships a different server, and asserts the old entry is back, the new one is absent, and trusted/enabled/mcpServers are unchanged.

HIGH — set_enabled() emitted no SEL event (powers.py:841)

Correct and a genuine gap against the blocking backend-security-controls rule. Trust grant/revoke, install and remove were all audited; activation was not — despite enabling being the step that actually starts third-party subprocesses and injects guidance into agent context. power_enable / power_disable events now emit after the state change succeeds, so the log records applied outcomes rather than attempts.

Regression: test_enable_and_disable_are_sel_audited.

MEDIUM — trust revocation unreachable from the UI (client.ts:725)

Correct, and this one is squarely my fault: round 5 fixed the endpoint to accept a boolean and I described the lifecycle as complete, but left the client hardcoding granted: true with no UI affordance. The contract said revocation existed; the product didn't offer it. Users could only de-authorise by uninstalling.

trustPower(name, granted = true) now takes the flag, and trusted Powers show a Revoke trust control beside Remove. Revoking force-disables and retracts both halves, matching the backend.

Regression: a trusted power can have its trust revoked without uninstalling, plus the existing grant test updated to assert the new argument.

Writing that test surfaced a latent flaw in the existing test harness worth flagging: several tests set mockApi.powers.mockResolvedValue(...) after renderTab(), so the component never sees the mocked data. They pass only because the value they assert against happens to match the beforeEach default (an untrusted Power). My revoke test failed for exactly this reason until the render was ordered correctly. The pre-existing tests still pass, but some are asserting less than they appear to.

Verification

• test/test_powers.py + test/test_powers_registry.py → 85 passed (2 new backend)
• full backend suite → 17,234 passed, 63 skipped, 5 xfailed
• isort, flake8, mypy (480 files) → clean
• tsc -b clean; vitest run → 390 files, 4,513 passed, 3 skipped (1 new frontend)
• Rebased onto current main; 29 PR-owned files.

Spec updated for both semantics: the rollback now documents restoring tree + MCP + record, and the trust section documents the power_enable/power_disable audit events plus revocation being reachable from API and UI.

Unrelated reds, verified not introduced here: main itself is failing Backend Tests (Windows) (3) and (4) (evidence in the previous comment), and three local test_dashboard_origin port-default failures reproduce on a pristine origin/main worktree.

[kyleseaman]

Round 7 — all nine GPT 5.6 findings addressed in 94e514c5 (rebased onto current main). All legitimate; no rebuttals.

HIGH — Powers state was not write-protected (powers_providers/__init__.py:80)

The most serious of the set, and correct. powers/ was in neither the sensitive nor the write-protected path list, so agent file tools could write it directly: overwriting .marketplace-cache.json remaps a familiar power id to an attacker repository (that cache decides which repo an id installs), and editing installed.json forges trusted: true, bypassing the SEL-audited trust endpoint entirely.

Both files are now in _WRITE_PROTECTED_HOME_PATHS — readable (the dashboard renders them) but not writable through file tools. Additionally, the resolved marketplace URL is re-validated through the https-only github host allowlist before use, because a value read back from the on-disk cache is no more trustworthy than caller input.

HIGH — inline SVG icon violated the blocking use-lucide-icons rule (PowerIcon.tsx:73)

Correct, and the suggested fix (fall back to lucide Zap) conflicts with an explicit product requirement: the icon must be Kiro's ghost+bolt Powers mark, which lucide does not ship.

Resolved without a rebuttal by using the repo's own established pattern for brand marks. GithubLogo.tsx / GitlabLogo.tsx render an .svg asset through a CSS mask, so no inline SVG appears in any .tsx. kiro-power.svg now holds the mark and PowerIcon.tsx is a mask component painting currentColor. The rule's regex (<svg[^>]*viewBox over src/**/*.tsx) no longer matches anything this PR adds — including my own doc comment, which tripped it on the first attempt and had to be reworded.

The url() is quoted and mask properties use longhands: Vite inlines small SVGs as a data: URI whose payload contains single quotes, which silently breaks an unquoted mask-image shorthand (the same trap hit in #446).

HIGH — power-<power>-<server> namespace is not injective (powers.py:788)

Correct, and my round-3 fix was only half the problem. I made purge ownership-exact but left install able to clobber: power git + server helper-srv and power git-helper + server srv both render power-git-helper-srv. Installing the second would overwrite the first's spec, and re-enabling the original would then launch the replacement's command under the original's trust grant.

Install now refuses when a generated name is already owned by another installed Power, rather than overwriting. Regression: test_install_refuses_server_name_owned_by_another_power asserts the trusted power's spec survives the attempt.

HIGH — cancellation abandoned the staging worker (powers.py:731)

Correct. Cancelling the await does not stop the executor thread, so an unshielded cancel released the MCP lock while the worker was still replacing the live bundle — new files paired with old MCP entries and metadata, no rollback, racing whatever acquired the lock next.

The staging future is now asyncio.shield-ed and re-awaited through cancellation before a shielded rollback runs; the post-swap handler catches BaseException so cancellation triggers the same restore as an error, and the rollback calls are themselves shielded. Note the staging-cancel path deliberately rolls back only the tree: no MCP or metadata write has happened at that point.

HIGH — enable activated MCP before skill materialization could fail (powers.py:900)

Correct. A malformed installed POWER.md made _apply_skill_state() raise after the record said enabled=true and the servers were already active — a partially activated security state reported as a failure.

set_enabled is now one transaction: metadata write, MCP flip and skill materialization run together in the executor, with prior record and MCP specs snapshotted and restored on any failure. Regression: test_failed_enable_rolls_back_mcp_and_metadata asserts the record reads disabled and the server stays disabled: true.

HIGH — synchronous I/O on the event loop (powers.py:661 and the listed ranges)

Correct. The install path read and parsed POWER.md (up to 256 KiB) inline, and several store/config operations ran synchronously inside async methods. The POWER.md read/parse moved to the executor, and set_enabled's whole transaction now runs there (above), leaving only lock coordination on the loop.

MEDIUM — silent delete failure reported success (powers.py:974)

Correct. rmtree(..., ignore_errors=True) let a locked file pass, after which the MCP entries and installed.json record were dropped and success returned — orphaning the bundle and generated skill with nothing tracking them. Deletion failures now propagate, and both trees are verified absent before the metadata delete proceeds. Regression: test_remove_does_not_report_success_when_delete_fails.

MEDIUM — branch names containing / mis-parsed (fetch.py:159)

Correct. tree/feature/x/power was split as branch feature + path x/power, targeting the wrong ref and 404-ing. Resolution now consults the repo's real branch list and prefers the longest match, falling back to the old split when the list is unavailable (so the common main/master case costs no extra request — the lookup only happens when the path is genuinely ambiguous). Regression: test_branch_with_slash_resolves_against_real_branches.

MEDIUM — provider failure indistinguishable from an empty catalog (base.py:192)

Correct. _search_one returned [] for both outcomes, and stale was derived from is_available() alone — and the official provider always reports available. A GitHub timeout therefore rendered an empty, non-stale registry and the UI claimed no Powers exist instead of surfacing an outage. _search_one now returns (results, ok), the registry records last_failed_providers per fan-out, and list_registry folds that into stale and per-provider availability. Regression: test_provider_failure_is_distinguishable_from_empty.

Verification

• test/test_powers.py + test/test_powers_registry.py → 90 passed (5 new this round)
• full backend suite → 17,254 passed, 63 skipped, 5 xfailed
• isort, flake8, mypy (480 files) → clean
• tsc -b clean; vitest run → 390 files, 4,513 passed, 3 skipped
• Rebased onto current main (it had moved 11 commits); 31 PR-owned files.

Two self-inflicted breakages caught by the gates during this round, both fixed: a scripted edit hit the parse_power_md call in the synchronous load_power() instead of the async install path, and relocating the new ref-disambiguation helper orphaned _parse_ref's bare-slug branch behind a return.

Unrelated reds unchanged: main is red on Windows shards 3 and 4 (evidence posted earlier), and three local test_dashboard_origin port-default failures reproduce on a pristine origin/main worktree.

Screenshots are stale — they predate the Revoke trust control, the PowerIcon empty states, and the icon's asset/mask rewrite. Worth re-capturing before human review.

[kyleseaman]

Round 8 disposition — 7c075dae

Arbiter passed. All eight blocking HIGH items are fixed with regression tests; two are rebutted below with evidence. Mediums are dispositioned at the end.

Fixed — security gates

security.py — Powers trust state was writable from a shell (GPT HIGH, security.py:2535). Confirmed: _build_sensitive_regex builds its path alternation from _SENSITIVE_HOME_DIRS only, so _WRITE_PROTECTED_HOME_PATHS was enforced solely at the file-edit gate (hooks.py:527). echo '{"x":{"trusted":true}}' > ~/.kiro/crew/powers/installed.json, tee, and sed -i all bypassed it. The config.json precedent does not transfer — the config loader clamps inflated values at load time, whereas a forged trusted: true is authoritative. Now the whole powers/ subtree is write-protected at both gates (also closing the bundle-mcp.json hole, which re-points an already-trusted Power at another command without touching trust state), via a new write-verb/redirect-anchored matcher that deliberately has no verb-independent branch so reads keep working.

Missing regression coverage for that control (Claude HIGH). Fair — the round-7 control shipped untested. test/test_powers_security_paths.py asserts write-blocked and read-allowed for installed.json, .marketplace-cache.json and a bundle mcp.json, across both crew-home prefixes, at both gates (redirect, append, tee, sed -i, rm), plus an over-blocking guard.

Fixed — store correctness

• Generated skill could destroy user data (powers.py:609). Enable did a blind mkdir(exist_ok=True) + overwrite; disable did rmtree(ignore_errors=True). A user-authored skills/powers/<name> was therefore overwritten and then recursively deleted. Now materialization refuses a directory without the .power-generated ownership marker, teardown deletes only generated files and leaves anything else in place, and removal failures raise instead of being swallowed.
• Two unaudited denial paths (powers.py:450, :951). Both raised before the only SEL emit, so denied installs from protected paths and denied enables of untrusted Powers left no audit trace. Both now emit a denied event before raising.
• Revocation could report success while content stayed live (powers.py:891). Metadata was committed before deactivation, and deactivation enumerated the mutable bundle. Now the transaction deactivates first and commits only on success — so a failed retraction leaves the record reading trusted (accurate) rather than claiming a revocation that did not happen — and enumerates the recorded mcpServers ownership list via a new _deactivate_recorded_mcp, so a trimmed manifest cannot hide a live server. Also folded in Claude's related Medium: the revoke path now runs in the same maintenance_executor transaction as set_enabled rather than inline on the loop.
• Cancellation released the MCP lock mid-write (powers.py:990). set_enabled and remove_power now use the same shielded-await idiom install_from_dir already used, so the worker is awaited before the lock unwinds.
• Failed uninstall destroyed MCP state first (powers.py:1074). Reordered: trees are staged aside and deleted for real, and only once they are provably gone are the MCP entries and record dropped; a mid-way failure restores the bundle. (My first attempt at this deleted the staged tree after dropping metadata, which the round-7 regression correctly caught.)
• Folder installs validated a different snapshot than they committed (powers.py:671). The post-lock path now re-reads mcp.json from the staged bundle, re-validates names, and registers those exact specs; the pre-lock read is now only early feedback. A rejection rolls the tree swap back.
• Ownership collision check was incomplete (powers.py:684). Rechecked under the lock. Worth noting why the original missed unowned entries: existing_power_server_names derives ownership from the live config by name prefix, so a power-<x>-<y> entry belonging to no Power already looked self-owned. The new check uses the installed record's mcpServers list instead, and refuses when a live entry exists that this Power's record does not claim.

Rebutted

PowerIcon.tsx violates use-lucide-icons (GPT HIGH, pass 2). Not applicable as written. The rule's check is a regex for inline <svg ... viewBox in src/**/*.tsx; this component contains no inline SVG — it imports an SVG asset and paints it through a CSS mask, which is the pattern the repo already ships for its own brand mark in GithubLogo.tsx. Frontend Lint & Type Check passes green on this head, and Design Review raised no objection. Substituting a lucide glyph would also mean the Powers surface no longer carries Kiro's own ghost+bolt mark, which is a deliberate product decision, not an oversight. Verified live rather than assumed: the mask resolves to a base64 data: URI with mask-size: contain, and an element crop measures 16.2% accent-pixel coverage (partial ⇒ the mask is shaping paint, not filling or blanking the box).

temp-screenshots/powers/*.png are ~2 MB of unreferenced binaries (both reviewers, Medium). Intended, and required. temp-screenshots/<feature>/ is this repo's prescribed location for PR review images: it is deliberately outside every packaged path (docs/ and src/kiro_crew/** ship in the wheel, sdist and desktop DMG), the directory is pruned periodically, and the PR body embeds the images by commit-SHA-pinned URL so they keep resolving after cleanup. They have no runtime consumer by design. The Arbiter reached the same conclusion independently. Screenshots are mandatory for a user-visible UI change, so removing them would break the contract this PR is required to satisfy.

Mediums — deferred as follow-ups

The Arbiter judged every remaining Medium non-blocking and enumerated them as suggested issues; I agree with that split and have not bundled them into this PR, to keep an already-large diff reviewable. They are: HTTP-layer input/error mapping (non-dict bodies → 400, outages → 503), provider/catalog robustness (GitHub 5xx as unavailable, expired-cache availability flag, truncated oversized HTML, official/marketplace dedupe, bare IDs for kind=registry), PowersTab polish (canonical-URL install matching, wait for isSuccess, per-row trust errors), Windows CRLF POWER.md parsing, malformed-vs-absent mcp.json, non-dict MCP specs recorded in the ownership list, and shielding the fetch.py write worker. Happy to fold any subset into this PR if you would rather not split it.

Verification

17419 passed backend (the 3 test_dashboard_origin.py failures are pre-existing host-port artifacts, reproducible on pristine main), 4555 passed frontend, flake8 clean, mypy clean on 481 files, tsc -b clean.

[kyleseaman]

Round 18 — 1e7a10dd

Both blockers fixed. One is a user-visible product bug rather than a hardening nit.

Mutable metadata replaced the record identity. load_power returned meta.name — read from the POWER.md inside the installed bundle, which stays editable after install. Editing it to declare bar made GET /api/powers report bar for the Power installed as foo, so the dashboard's delete called /api/powers/bar, got a 404, and foo became unremovable through the UI. Now returns the record key, which is what every route and installed.json agree on. Revert-verified.

A crash between the two renames stranded the bundle. The backup path carried the pid (.backup-<name>-<pid>), so a SIGKILL landing between dest -> backup and staging -> dest left the live bundle under a name no later process would ever look for: the Power vanished from the listing while its bytes sat on disk indefinitely. The backup path is now deterministic, and each transaction recovers an orphan before staging. Staging keeps its pid deliberately — it is pre-commit scratch and never the only copy of anything, so a stale one is safe to discard.

A test that proved nothing, again

My first version of the recovery test asserted a successful reinstall after planting an orphan. That passed with the recovery removed, because a succeeding install replaces the tree either way — the recovery made no observable difference on that path.

Recovery only becomes observable when the next install fails: without it the rollback sees no predecessor, discards the orphan, and the Power is gone for good. The test now does that, and fails with orphaned bundle was lost by a failed reinstall when the recovery is removed.

That is the fourth time in this PR's review history that revert-testing caught one of my own tests asserting something the fix was not responsible for. The pattern is consistent enough to name: when a fix is about recovering state, exercising the happy path proves nothing, because the happy path overwrites that state either way.

Round status

Backend Tests (3.10, 3) and Coverage Gate went green on the previous head, confirming the round-17 compensation fix.

Two reviewer runs on that head were harness failures rather than findings on this diff: GPT returned "review incomplete" with no verdict and needed a re-run, and Claude ran 32 minutes before being cancelled with no output after initialisation — the same stall signature already recorded on several other PRs in this repo.

Gates: 1,367 backend tests on the powers/security/provider selection across three consecutive runs (this seam has produced order-dependent flakes, so one clean run is not evidence), mypy clean, flake8 / isort clean.

[kyleseaman]

Round 19 — fe5d97ce

Both blockers fixed. One needed a synthesis rather than the prescribed fix, and one of the findings is a real disclosure problem I had previously described incorrectly.

Cancellation cleanup blocked the event loop. The concern is legitimate — a synchronous wait of up to 10s on the loop is unacceptable. But the prescribed fix, "drain the shielded future asynchronously", is the exact shape CI already disproved: on Python 3.10 a cancelled task re-raises from every subsequent await, so an async-only drain silently skips compensation and strands the bundle. That was observed twice, in rounds 15 and 17, as Backend Tests (3.10, 3) failing on my own assertions.

So compensation now tries the async drain first — an ordinary failure never touches the loop — and falls back to the synchronous settle only when awaiting is impossible, i.e. on a cancelled transaction. The bound also dropped from 10s to 2s, since the work being waited on is a single rename. That keeps the loop clear in the normal case without reintroducing the 3.10 failure.

Root confinement was open to a symlink race. Legitimate, and the fix is subtraction rather than addition: _power_path called self._dir.resolve(), and that resolution was the entire exposure — a shell could swap powers/ for a symlink during the call, let resolve() capture the external target, then restore the real directory before the mutating worker re-checked it, so the check passed while every derived path pointed outside the store. The root is now used as configured, unresolved, so there is nothing to capture; paths stay lexically under it and _assert_root_not_symlinked() inside the mutating callable rejects a symlinked root at the moment it matters.

A pinned root directory handle was the suggested fix, but dirfd-relative operations are not portable to Windows, which this store also supports. Removing the resolution closes the same window with no platform split.

The screenshots are stale — this finding is correct and I previously said otherwise

browse-*.png and browse-stale-dark.png predate several changes this PR later made: the page subtitle ("activated on demand" → browse/install/remove), the stale-registry banner wording, the PowerIcon rewrite through BrandGlyph, the lucide clear-search glyph, and the PowerIcon empty states. Only installed-*.png and sidebar-powers-dark.png were re-captured against the current build.

I stated earlier in this PR's history that the captures were current. That was wrong for the browse set, and the reviewer is right to flag activation-era copy still visible in them. Re-capturing needs the harness rebuilt (SPA build, isolated instance, seeded data), so I am flagging it rather than quietly leaving it — the PR contract wants screenshots that match the diff, and right now three of six do not.

Declined

handlers/powers.py function-local imports, for the third time, on unchanged evidence: they sit inside try/except guards implementing documented graceful degradation, and hoisting them converts an empty-stale fallback into an import-time failure for the whole dashboard.

Round status

40 checks passed on the previous head, including Claude and the Arbiter — the first round where both cleared.

Gates: 1,367 backend tests on the powers/security/provider selection across two consecutive runs, mypy clean, flake8 / isort clean.

[kyleseaman]

Round 20 — 46166bdb

Both blockers fixed, and this time by removing the seam rather than patching it. The two findings shared one root cause, which is why they kept alternating across rounds 15–19: compensation lived in the coroutine, after the worker, so cancellation could always interleave. Every fix for one exposed the other — an awaited drain cannot run on a cancelled task, and a synchronous wait stalls the loop.

remove_power is now a single self-contained blocking transaction. Lock acquisition, every mutation, its own rollback, and the unlock all happen inside one callable. There is nothing left to compensate from the coroutine and nothing to wait for on the event loop:

• Cancelling the await cannot produce a half state, because the worker owns its rollback and runs to completion in its thread.
• The lock is taken and dropped in the same callable, so it can neither leak on cancellation nor be released while mutations are still in flight — which was the second half of the first finding.
• No concurrent.futures.wait on the loop, and no _settle_blocking on this path at all.

Root confinement is now atomic where the platform allows it. The transaction opens the powers root once with O_NOFOLLOW | O_DIRECTORY and performs renames and stats through that descriptor (src_dir_fd / dst_dir_fd). A descriptor keeps referring to the directory that was validated, so swapping the path for a symlink afterwards cannot redirect a later rename or delete — which is what no check-then-act sequence could promise, however tightly the two were placed.

Windows has no dir_fd support, so it keeps the path-based check. That is a deliberate, documented split rather than an oversight: creating a symlink on Windows requires elevation or developer mode, so the residual exposure there is materially narrower, and the alternative was to hold the whole platform to the weaker guarantee.

Behaviour change worth calling out

The caller no longer waits for rollback. On a cancelled uninstall the coroutine returns immediately while the worker finishes putting the tree back. That is intentional — correctness belongs to the worker, not the caller — but it means a test cannot assert the filesystem state the instant await returns. Four tests instrumented the old multi-worker shape and were adapted: three patched Path.rename where the transaction now uses os.rename with a descriptor, and the assertions now poll the invariant with a deadline instead of sleeping. A fixed sleep was flaky under load, which two consecutive full-suite runs caught before this was pushed.

Gates: 1,367 backend tests on the powers/security/provider selection across three consecutive runs, 83 Powers tests across three, mypy clean, flake8 / isort clean.

[kyleseaman]

Round 21 — 78a45b4e

Rebased onto current main (was 41 behind, now 0) and relaxed two over-specified test assertions. No production code changed this round.

The two cancellation tests were asserting timing, not the invariant

test_cancel_after_staging_restores_the_bundle failed on the 3.10 shard with DID NOT RAISE CancelledError. That is a consequence of the round-20 rewrite working as intended, not a regression: the uninstall is now a single fast executor call, so the transaction can finish before the cancel lands, at which point task.cancel() is a no-op and the task returns normally.

Both outcomes — cancelled mid-flight, or completed first — are legitimate, and the invariant is the same either way: no .removing-* residue and no half state. Wrapping the await in pytest.raises(CancelledError) turned an invariant test into a timing test that only passed where the scheduler happened to cooperate. Both wrappers now tolerate either outcome and keep the invariant assertions unchanged.

The docstring claim that the cancellation "always lands on the staging await" was also wrong after the rewrite and is corrected: the signal aims at that window, but nothing asserts it arrives there.

The two Windows failures were inherited, and the rebase clears them

test_dashboard_chat.py::TestEmptyResponseRetry::test_first_empty_response_requeues_message failed on Windows shards 2 and 3. That file is not in this PR's scope — confirmed with a three-dot diff (origin/main...HEAD), which shows 34 changed files, none of them that one. A two-dot diff does list it, because the branch was 41 behind and main had since changed it; that view renders main's edits as deletions and is misleading for scope questions.

Main carries the fix, so the rebase resolves it. With main's version now in the tree, the local run covers it: 1,933 backend tests pass across the powers/security/provider/dashboard-chat selection.

Verification on the rebased tree

• 1,933 backend tests (selection above), 83 Powers tests stable across three consecutive runs
• 4,751 frontend tests, tsc clean — re-run because the rebase pulled 41 commits
• mypy, flake8, isort clean
• 0 behind main, MERGEABLE

[kyleseaman]

Round 22 — the three stale browse screenshots, and what re-shooting them found

Head 6ed9bdc5. The three browse screenshots were disclosed as stale (they predated the subtitle change, the stale-banner rewording, the BrandGlyph rewrite and the lucide clear-search glyph). Re-capturing them against a real isolated instance returned zero Powers, which was not a capture problem.

The defect

All three provider bodies were read with one await resp.content.read(cap + 1). StreamReader.read(n) returns whatever is buffered when it wakes; it does not loop to fill n. Measured against the live upstreams:

Upstream	One read(cap + 1)	Chunk loop
kiro.dev/powers/	57 KiB	649 KiB
api.github.com/repos/kirodotdev/powers/contents/	9.8 KiB	25.6 KiB

Neither failure looked like a read bug:

• marketplace — parsed a truncated document, recognised no cards, logged marketplace HTML yielded no cards; provider marked unavailable.
• official — fed truncated JSON to json.loads and raised ProviderUnavailableError: Unterminated string starting at: line 1 column 5577 (char 5576).

Both surfaced as provider unavailable, so the browse view rendered an empty catalogue against the real registry. The per-file byte cap was also unenforceable: a short read never reached it, so an oversized file was accepted as a truncated one rather than rejected.

Fixed with base.read_capped(stream, cap + 1), which loops to the limit. Callers keep their own len(body) > cap check, so overflow policy (raise vs. truncate) stays at each call site. Registry output went 0 items → 82 items, both providers available.

Why 95 tests missed it

Every existing test stubs _http_get_json / _http_get_bytes / _fetch_html — the three functions that contain the read. The seam was mocked one level above the defect, so no test could observe it. TestBoundedStreamRead therefore runs a loopback HTTP server: a mocked stream can be made to return a whole body in one call, which is precisely the behaviour that does not hold on a socket. All five fail with the fix reverted, reproducing the production error text.

Consequence: provider overlap became reachable

The official provider had never returned data in practice, so nothing about the merge was observable. It is registered first, and dedup is by canonical repository URL, so for the 26 of 82 overlapping Powers the official entry won — and the official provider lists a GitHub directory, so description / author / category are structurally empty for it. Every overlapping card rendered authorless.

The losing duplicate now donates any facet the winner left blank; a field the winner populated is never overwritten. scope is deliberately not merged — the providers disagree (monorepo membership vs. authorship facet) rather than one being blank — so an AWS-authored Power mirrored into the official monorepo answers to the Official chip while showing AWS as its author. That divergence is recorded in docs/system-specs/modules/powers.md rather than papered over.

spark-troubleshooting-agent legitimately appears twice: the official monorepo copy and the aws-samples original are different repositories, and dedup is by URL.

Screenshots

All three re-captured and re-pinned to 6ed9bdc5. The harness asserts the state each shot claims:

• theme actually applied (a silently failed switch previously produced two identical "dark" files)
• zero visible [role="dialog"] — server-side onboarding state is now completed via PUT /api/config/theme, because useTheme treats /api/theme/boot as authoritative and clears the localStorage flags, so the earlier localStorage seeding did not suppress the first-run modal
• card count ≥ 20
• the degraded banner is asserted absent in the fresh shots and present in the stale one, so a fresh shot can never be a mislabelled outage shot

The stale shot was taken with the marketplace upstream made unresolvable and its disk cache aged past the TTL, so the provider takes the real transport-failure branch and serves expired entries while the official provider keeps working — the caption says so.

Verification

• 1,473 pass across the powers or security or provider or data_home selection
• isort, flake8, mypy clean
• 2 of the 3 dedup tests fail with the enrichment reverted; the third is a non-override guard that passes either way by construction, and is labelled as such
• No frontend source changed this round, so vitest / tsc were not re-run

Scope note

This is a behavioural fix inside the PR's own provider layer, not new surface: no route, schema or UI control was added, and installed Powers remain inert. It is in scope because the PR's central claim — that you can browse the registry — was false against the real upstreams.

[kyleseaman]

Round 23 — db966e89

All three findings addressed. The two blocking ones were legitimate and share one cause; the advisory one exposed a false claim I made last round.

Both blockers were the install path still carrying the pre-round-20 shape

Round 20 restructured remove_power into a single blocking transaction and dissolved the five-round oscillation between "compensation cannot await" and "synchronous settle blocks the loop". I did not apply the same treatment to install_from_dir, and I said so explicitly at the end of round 22 as an optional follow-up. Both findings are that omission, so they are fixed by finishing the job rather than by patching two seams.

powers.py:650 — cancellation fallback blocks the event loop. Correct. _settle_for_compensation fell back to a bounded synchronous wait, reachable whenever a cancelled install had to compensate. Rather than move the wait off-loop, the wait no longer exists: install is now one callable that takes the lock, stages, swaps, writes the record, commits or rolls back, and unlocks — awaited once under asyncio.shield. With a single worker there is nothing to compensate from outside and nothing to settle. _submit_blocking, _settle_for_compensation, _settle_blocking, _run_drained and _powers_transaction are deleted, not tuned — the mechanism that needed the synchronous wait is gone, so the wait cannot come back.

powers.py:975 — install root validation remains TOCTOU-prone. Correct, and the prescribed fix is the right one: install mutations now go through the root descriptor remove_power already pins. _root_lock opens the root O_NOFOLLOW|O_DIRECTORY once and every rename and delete in the transaction is descriptor-relative, so a root swapped for a symlink mid-transaction is unobservable. _assert_root_not_symlinked is deleted with them: it was one operation away from the mutation it guarded, which is exactly the gap. _SUPPORTS_DIR_FD still gates the POSIX path and Windows keeps the path check, where creating a symlink requires elevation.

Net effect on powers.py: 147 lines removed.

The advisory finding was right, and my round-22 disclosure was wrong

sidebar-powers-dark.png did show "Review before trusting", a Keep disabled button and a Servers: power-stripe… line — activation surface this PR removed. Round 22 stated that only the three browse shots were stale and that the sidebar one was current. That was wrong, and it was wrong in the direction that matters: the screenshot advertised a control the diff does not contain.

All six shots are now re-captured from one build against one instance, and the harness asserts the inert surface rather than trusting me to look: zero role="switch" elements, an Inactive badge per installed Power, and zero matches for Review before trusting|Keep disabled|Trust this power|Revoke trust|Servers:. A shot containing deferred activation copy now fails the capture instead of being committed.

The installed-view shots were also re-taken, because the previous pair came from an earlier build. That surfaced a second harness defect worth recording: cap_powers.js set the theme through localStorage only, and since useTheme treats /api/theme/boot as authoritative and clears those keys, a run following a light capture silently produced a light "dark" shot. Theme and onboarding are now set server-side via PUT /api/config/theme.

Test changes, stated plainly

• The two _settle_blocking unit tests are deleted — they tested a helper that no longer exists.
• Two cancellation tests patched _run_drained to simulate "no awaited compensation can run". That patch was already inert before this round: round 20 removed the last _run_drained call site, so the patched function was never invoked and the 3.10 condition those comments described was not being simulated. The patches and their now-false comments are gone; the invariant assertions stay.
• One new test, test_install_mutations_are_anchored_to_the_pinned_root, covers the descriptor property. My first version was vacuous — it broke the install in both shapes and passed with the fix reverted. The committed version makes the swapped-in root present a valid layout (matching staging dir, existing bundle) so path-based renames succeed, and asserts an external victim directory survives. It fails with the fix reverted.
• The 81 pre-existing test_powers.py tests pass unchanged against the new shape, including test_failed_reinstall_preserves_existing_bundle, test_cancel_after_staging_restores_the_bundle, test_orphaned_backup_is_recovered_on_next_install and test_install_holds_a_cross_process_lock — which is the useful evidence that the restructure preserved behaviour rather than redefining it.

Verification

• 1,472 pass across the powers or security or provider or data_home selection; 187 across the three Powers files
• isort, flake8, mypy clean
• No frontend source changed, so vitest / tsc were not re-run; the SPA was rebuilt only to serve the capture instance
• docs/system-specs/modules/powers.md rewritten under Install and remove atomicity to describe the single-transaction shape and the pinned-descriptor confinement, including why both alternatives were rejected

[kyleseaman]

Round 24 — publisher icons (34550e4c)

Registry cards now show the publisher icon the marketplace listing already exposes. This came out of comparing our browse view against kiro.dev/powers/ directly: I dumped a card's raw markup to see what we were dropping.

What the page actually has, and what we were missing

Signal	In the page	We extracted
Publisher icon	76 of 76 cards	✗ (now ✓)
Per-card description	✗ — no description text in any card	—
Per-card category	✗ — category exists only as a filter control	—
Card anchor id="<slug>"	✓ on all 76 containers	✗ (still using the launcher window)

Two things worth recording from that. The blank descriptions on our cards are faithful to upstream, not a parsing gap — the card DOM is icon, title, author, launcher, Details link, and nothing else. And there is a stable per-card anchor (id="<slug>"), which would be a simpler parse than the neighbour-bounded launcher window the spec calls fragile; I have not switched to it in this PR, but it is now written down rather than left as an unknown.

The icon URL is treated as untrusted input

It is scraped from a third-party page and becomes an img src, so the origin is an allowlist at a single chokepoint (marketplace.valid_icon_url): https only, exactly the Kiro asset host, /powers/icons/ path prefix, length-bounded. Refused: scheme downgrade, foreign host, suffix-spoofed host (prod.download.desktop.kiro.dev.evil.com), path escape, javascript:.

Validation runs twice — on scrape and on cache read. The second is the one that matters: the marketplace cache is on-disk state this spec already treats as attacker-reachable (it decides which repository an id resolves to, which is why the resolved URL is re-validated on read). An icon read back from a poisoned cache would otherwise put an arbitrary origin in front of the user's session on every card render. A refused icon degrades the card and never drops the Power.

No CSP change was required — img-src 'self' data: blob: https: already permits it. I checked before building rather than after.

Why this also vindicates the facet merge

The official provider lists a GitHub directory, so it has no icon, and it wins the dedup for the 26 overlapping Powers. Without the blank-facet enrichment from round 22, a third of the catalogue would render the fallback while the marketplace held a real icon for it. Measured on the live registry: 76 of 82 cards now show a real icon; the 6 that do not are official-only Powers with no marketplace card, and they render the Kiro Powers mark. aws-mcp in the screenshot is one of them.

Verification

• 1,485 backend tests pass on the powers or security or provider or data_home selection (13 new)
• 4,754 frontend tests, 412 files, all green; tsc clean
• Revert-verified: defeating the allowlist fails 7 of the 13 backend tests, including the poisoned-cache one; removing the render fails all 3 frontend tests
• isort, flake8, mypy clean
• Three browse screenshots re-captured. The harness now counts icons with naturalWidth > 0, so a shot full of fallback placeholders fails the capture instead of being committed — the same discipline the inert-surface assertions use.

Scope

This is new surface on an already-large PR, added at the maintainer's explicit request after reviewing the alternative (ship #408 first, icons as a follow-up). It touches the provider dataclasses, the marketplace parser, the API shape and one card component; it does not touch install, remove, the transaction, or the inert-install property.

[kyleseaman]

Round 25 — d01a6791

The blocking finding was correct and my round-23 claim was overstated. Both GPT items fixed, plus three Windows failures that were mine.

BLOCKING powers.py:842 — path-based mutations bypassed the pinned descriptor

Accurate, and it names exactly what I left behind. Round 23 said "every rename and delete goes through that descriptor". Only the renames did. Three operations stayed lexical:

Operation	What a swapped root did
staging.mkdir(parents=True)	created the staging tree outside the store
self._write_installed(...)	wrote the record describing the store outside it
shutil.rmtree(target) inside _rmtree_at	reopened each component by name and deleted an external tree

All four mutation classes are now descriptor-anchored: _mkdir_at, _atomic_write_at (temp + os.rename with src_dir_fd/dst_dir_fd), and _rmtree_fd — a hand-written dir_fd recursive delete, because shutil.rmtree gained dir_fd later than the Python versions CI runs. The previous form validated only the top-level entry through the descriptor and then handed the path to shutil, which is the window.

The copy is anchored too, which the finding did not name but the same reasoning reaches: creating staging through the handle is pointless if the copy then writes by path, because the staging name is pid-derived and therefore guessable, so a decoy of that name could receive the bundle. _copy_power_files / _copy_regular_file now take a dest_fd and create files with O_CREAT|O_EXCL|O_NOFOLLOW relative to it.

_drop_record and _write_installed take the handle as a keyword so read-repair callers outside a transaction keep the plain path write. _SUPPORTS_DIR_FD still gates all of it; Windows keeps the path forms.

The new test drives a root swap during the remove transaction with a decoy root laid out to make lexical operations succeed — an installed.json to overwrite and a .removing-kb tree to delete. With the fix reverted it fails on the recursive delete followed the swapped root, so the damage is demonstrated rather than asserted.

FINDING — function-local provider imports: fixed, not rebutted a fourth time

I declined this three times because the imports sit in try/except blocks implementing documented degradation, and I argued hoisting would turn an empty-stale fallback into an import-time failure.

That argument only holds against a plain module-scope import. A module-scope try/except satisfies the rule and keeps the behaviour: the import is attempted once at load, a failure is caught there, _PROVIDERS_AVAILABLE records it, and each handler checks the flag instead of catching its own ImportError. All four sites are gone; the sentinel exception type is preserved so the except clauses still never match when the package is missing.

The three Windows failures were mine, and the cause is a lesson

test_remove_keeps_the_record_when_deletion_is_impossible, test_cancel_after_staging_restores_the_bundle and test_cancel_compensation_does_not_depend_on_awaiting failed on shard 3 while passing on Linux. They patched shutil.rmtree and os.rename — the concrete calls, which differ by platform: the anchored path deletes via _rmtree_fd and renames with descriptors, the fallback uses shutil.rmtree and os.replace. So each test exercised the seam on one platform and silently no-op'd on the other.

They now patch _rmtree_at / _rename_at, the module helpers every platform routes through. One of the wrappers was also taking the wrong positional args (_rename_at takes four), which is why the event never fired and the test timed out rather than failing an assertion.

The hoist moved a second patch seam the same way: four handler tests patched kiro_crew.powers_providers.<fn>, which no longer reaches the handler now that it holds its own reference. They patch kiro_crew.dashboard.handlers.powers.<fn> instead. Same lesson twice in one round — patch the name the code under test actually calls.

Verification

• 1,486 pass across the powers or security or provider or data_home selection; 83 in test_powers.py
• isort, flake8 clean on src/ and test/; mypy clean across 495 files (the one local vector_memory error is a faiss-present artefact CI does not see)
• Revert-verified: reverting the anchoring fails the new test with external damage; the four repointed handler tests fail against the pre-hoist patch target
• No frontend change, so vitest / tsc were not re-run
• docs/system-specs/modules/powers.md corrected — the confinement paragraph now enumerates all five anchored operations instead of claiming only renames were the concern

[kyleseaman]

Round 26 — d2525ee9

One blocking finding, fixed, plus the last Windows failure.

BLOCKING powers.py:1015 — transaction state read bypassed the pinned root

Correct, and it is the symmetric half of what I fixed last round. I anchored every write and left _load_installed resolving the path lexically, which is not a partial fix but a worse one: the record lands in the right place with content taken from a decoy root, so an install rebuilds installed.json from foreign state and erases every other Power's provenance. A file that is intact, correctly located, and wrong.

_load_installed now takes the handle inside a transaction and reads through os.open(..., dir_fd=root_fd) with O_NOFOLLOW on the leaf, so a symlink planted at installed.json cannot redirect it either. _drop_record threads it through as well.

list_powers and load_power deliberately keep the plain read: they are read-only paths with no handle in scope, where a decoy yields a wrong listing but cannot destroy state. Recorded in the spec so the distinction is deliberate rather than an omission for a future round to re-flag.

The new test was vacuous first, again

My first version swapped the root during the copy. That made a later lexical staging check fail, so the install aborted before the read and the test passed with the fix reverted. The committed version hooks _rename_at and swaps immediately after the staging→destination rename — the only window where the state read is the next thing to happen. Reverted, it fails with the first Power's provenance was erased.

That is the third vacuous-first test in this PR. The pattern is consistent enough to name: when a test simulates an attack by breaking the environment mid-operation, the break usually aborts the operation before it reaches the step under test. The fix each time was to make the hostile state valid enough that the wrong code succeeds, so the damage — not an exception — is the signal.

Last Windows failure

test_remove_keeps_the_record_when_deletion_is_impossible patched os.rename. The no-dir_fd platforms rename with os.replace, so the patch never fired on Windows and no OSError was raised. Repointed at _rename_at, the same helper the other three now use. That was the fourth instance of this class in two rounds; the file now has no patches of concrete filesystem calls left.

Verification

• 1,487 pass across the powers or security or provider or data_home selection
• isort, flake8, mypy clean
• Revert-verified as described above
• No frontend change, so vitest / tsc were not re-run

[kyleseaman]

Round 27 — 2a9f7e72

Both blocking findings fixed. Windows shard 3 is green — the four repointed test seams held.

BLOCKING powers.py:370 — parent symlink race in the steering copy

Legitimate, and the per-file protection genuinely does not cover it. steering_src.is_dir() and not is_symlink() followed by glob("*.md") re-resolves the directory name, so a caller-owned source can swap steering/ for a symlink in between — and O_NOFOLLOW on each file does not object, because after the swap those are ordinary regular files that simply live somewhere the caller never offered.

The prescribed fix was to reject folder installs until steering traversal is pinned. I pinned the traversal instead, which keeps the feature and removes the race rather than trading one for the other: steering/ is opened once with O_NOFOLLOW|O_DIRECTORY and enumerated through that handle (os.listdir(fd)), each file is opened with dir_fd=, and the fstat-vs-stat identity check is descriptor-relative too — leaving it as os.lstat(path) would have re-resolved the very path the descriptor exists to avoid. A symlinked or non-directory steering/ is now refused rather than skipped, since skipping would silently install a partial bundle from a source that was tampered with mid-install.

This mirrors the destination-side anchoring from rounds 25–26; the source side was the remaining half.

BLOCKING powers.py:999 — crash recovery could destroy or orphan bundles

Also correct, on both halves.

Ambiguous backup. With dest and .backup-<name> both present, the previous transaction died after the swap — and whether its record write committed is not recoverable from the filesystem. The old recovery deleted the backup, which is the only copy of the bundle the live record may still describe. It is now moved to .orphaned-backup-<name>-<ts> with a logged warning. Bytes are never destroyed to tidy up, and a stale backup still cannot block the install.

Orphaned .removing-*. Remove dropped the record for a missing bundle without checking for a leftover scratch tree, so a crash between the rename-aside and the delete left bytes that nothing tracks — list_powers() enumerates installed.json, so an orphan is invisible to the UI. The interrupted delete is now completed first, and if it still fails the record is kept, which is the same rule the ordinary delete-failure path follows and which this branch bypassed entirely.

On the tests

The steering test took three attempts to stop being vacuous, and the reason is worth stating because it is the same trap each time. Swapping at install start is caught by the existing is_symlink() check; swapping at the destination mkdir never fires, because that path uses os.mkdir(dir_fd=) rather than Path.mkdir. The race window contains exactly one observable operation — _open_dir_at for the destination steering directory — so that is the hook. Reverted, the test now fails with the protected file's contents inside the store.

The three recovery tests construct the interrupted on-disk states directly rather than trying to win a race with a real SIGKILL, which is both deterministic and honest about what is being asserted. All three fail with the fix reverted.

Verification

• 1,491 pass across the powers or security or provider or data_home selection; 88 in test_powers.py
• isort, flake8, mypy clean
• Windows shard 3 green on the previous head, confirming the seam repointing
• Revert-verified: 4 tests fail with these two fixes reverted
• No frontend change, so vitest / tsc were not re-run
• Spec updated with the source-pinning rationale and a Crash recovery never destroys bytes section enumerating the three recovery states

[kyleseaman]

Round 28 — 7366daf8

One blocking finding, fixed at both layers. Windows is green; the previous round's red shard was inherited from main and the rebase cleared it.

BLOCKING PowersTab.tsx:211 — unvalidated cached URL reached an href

Correct, and it is an inconsistency I introduced myself. Round 24 added valid_icon_url on the cache read but left github_url on that same path as a bare str(it["github_url"]). That is the worse of the two to leave unguarded: an icon URL becomes an img src, while the repository URL becomes an href the user clicks, so a poisoned cache entry would have been click-to-execute rather than cosmetic.

Fixed at both layers, because they fail differently:

• Backend — valid_tree_url() re-validates against the same _TREE_RE shape the scrape is anchored on (https://github.com/<owner>/<repo>/tree/…). A cached value that does not match did not come from a scrape this code performed. The entry is skipped rather than blanked: the URL is both the dedup key and the install target, so a card without a trustworthy one is not usable. ValueError joins the existing skip list so one poisoned entry cannot take the whole cache down — there is a test for exactly that.
• Frontend — the link is routed through the repo's existing safeHttpUrl helper and omitted when rejected, replaced by a plain "Source unavailable". This follows the precedent already in the tree: McpBrowserModal.tsx gives a registry repo_url the same treatment. A missing link is the right degradation; a link that runs is not.

Rejections covered: javascript:, data:, scheme downgrade, foreign host, suffix-spoofed host (github.com.evil.com), and a non-tree GitHub URL.

Note on the previous round's Windows failure

test_kiro_prerequisite.py::test_rejects_non_runnable_candidate was not mine — main had already fixed it in 1a3c6d47 ("skip the exec-bit rejection test on Windows hosts") and this branch was 28 behind. Confirmed with git merge-base --is-ancestor in both directions before rebasing rather than assuming. Rebased to 0 behind; the check is green.

Verification

• 1,500 backend pass across the powers or security or provider or data_home selection (9 new)
• 5,177 frontend tests, 428 files, all green; tsc clean
• Revert-verified: 8 of 9 backend tests fail with the validation reverted, and the frontend omission test fails with the guard reverted
• isort, flake8, mypy clean

One environment note worth recording: the full vitest run initially failed 38 files on Failed to resolve import "@radix-ui/react-select". That is not a code regression — the rebase pulled a new dependency onto main and this worktree's node_modules was stale. npm ci cleared it, and the suite is green above. Worth knowing that a rebase can invalidate the frontend gate without touching any source line.

[kyleseaman]

Round 29 — 5cec1819

Both blocking findings fixed. Both were in the confinement code I added in rounds 25–27, which is worth stating plainly rather than presenting as new discoveries.

BLOCKING powers.py:763 — predictable temp file permitted hardlink truncation

Correct. _atomic_write_at created a fixed .installed.json.tmp with O_CREAT|O_TRUNC and no O_EXCL. O_NOFOLLOW refuses a symlink at that path and says nothing about a hardlink, so a preplanted link would have been truncated and then filled with store state. I added that code two rounds ago while closing a different confinement gap.

Now a random suffix (secrets.token_hex(8)) plus O_CREAT|O_EXCL: the name cannot be pre-created, and if it somehow exists the open fails rather than adopting it.

BLOCKING powers.py:664 — Windows directory junctions bypassed the root check

Also correct, and pointed at the platform that depends on the check. Path.is_symlink() returns False for an NTFS directory junction, which the filesystem follows exactly like a symlink. POSIX pins the root with O_NOFOLLOW and never needed the path test; Windows has no dir_fd support, so the path test is its only guard — and it was the incomplete one. A junctioned root would have let _rmtree_at delete a matching directory outside the store.

_assert_not_reparse_point() now rejects both, testing st_file_attributes & FILE_ATTRIBUTE_REPARSE_POINT alongside the symlink check. The attribute is absent on non-Windows stat results, so there only the symlink test applies.

On the tests, honestly

The hardlink test is properly revert-verified: with the fix reverted it fails on the state write truncated a preplanted hardlink, so real truncation is demonstrated.

The two reparse-point tests are weaker, and I would rather say so than imply otherwise: reverted, they fail with AttributeError: module has no attribute '_assert_not_reparse_point' — they would fail for any rename, not only for a behaviour regression. A real junction cannot be created on this Linux host, so the attribute is simulated. The negative case (a plain directory must not be rejected) is the part that carries actual signal.

Verification

• 1,503 backend pass across the powers or security or provider or data_home selection (3 new)
• isort, flake8, mypy clean
• No frontend change this round, so vitest / tsc were not re-run (5,177 green on the previous head)

A note on the shape of this review cycle

Rounds 25–29 have all been the same theme: each pass hardens the store's filesystem confinement, and the next pass finds a remaining seam in what the previous one added — writes, then reads, then the source side, then crash recovery, then the temp file and the Windows fallback inside the fix itself. Every finding has been legitimate and I am not asking for any of them to be waived. But the fixes are now well past the surface this PR set out to add, and a reviewer may reasonably want to judge whether the remaining hardening belongs here or in a follow-up focused on powers.py alone. I will keep fixing whatever comes back; flagging the trend in case the maintainer wants to scope it instead.

[kyleseaman]

Round 31 — deferred activation redesigned around the IDE's actual model (8c745e78)

No production code changed. This round withdraws a design, and the reason is that I had been guessing about upstream and guessed wrong.

What I got wrong

Every version of this PR's Deferred: activation section described a push design: merge the Power's MCP servers into the rendered ~/.kiro/agents/kirocrew.json, materialize its docs into a skill so SkillsLoader can trigger them, and gate both behind a trusted flag in installed.json. The spec even claimed upstream parity would be a question of "upstream's own namespacing if it differs from power-<power>-<server>" — which presumed upstream had namespacing to differ about.

Reading the IDE's implementation (kiro-team/kiro-extension, src/extension/powers/**) shows it does neither of those things. It exposes tools and the agent pulls:

Upstream tool	Shape
listPowers	installed Powers with mcpServers names and keywords, without starting anything
usePower	(powerName, serverName, toolName, arguments) — namespaced dispatch
readPowerSteering	(powerName, fileName) — one .md, on demand, path-validated
configurePowers	ships behind ENABLE_WEBUI = false

Their steering rules turn out to be independently identical to ours: .md only, no separators, no leading dot, validated join against the steering directory.

Why this matters for this PR, not just the follow-up

The push design's requirements were the reason the combined PR could not converge — nearly all of the transactional complexity existed to make enable/disable/revoke safe. Following the pull model deletes those requirements rather than solving them:

• The includeMcpJson constraint that dominated the old plan is gone. Nothing is written to agent config, so nothing must be rebuilt and enable cannot be a silent no-op. (That was the real bug the split surfaced.)
• No power-<power>-<server> namespace, so the injectivity problem and the ownership record a non-prefix purge required both vanish.
• No skill materialization, so third-party markdown never enters a session unasked and never competes for the steering budget.
• No trusted / enabled fields — so installed.json as shipped in this PR needs no migration. I had written the opposite into the spec ("when activation lands it adds them, and its readers must treat a missing key as false"). That obligation is explicitly withdrawn in the spec rather than quietly dropped.
• No enable/disable transaction, so the install/remove transaction that rounds 20–29 hardened does not grow two more state transitions over the same lock.

Consent, and one deliberate divergence

The trust flag is replaced by the approval mechanism the rest of the product uses: the first power_learn / power_use for a Power in a session raises an approval naming the Power, the server and the resolved command; per-session Trust covers the rest of that session. This is the same conclusion PR #518 reached when its round-4 change deleted slot._trust / _enforce_trust_ttl and routed through core approvals — a second bespoke trust store here would repeat the mistake that change corrected.

Upstream has no consent prompt: install is one click and the tools are callable. KiroCrew adds one anyway, because the IDE always has a human at the keyboard while KiroCrew runs turns from cron, Slack and Discord. The spec records this as intentional so a later reviewer does not "fix" it toward parity.

One requirement I nearly lost, and kept

Rewriting the section initially dropped the push plan's item 5, the bash-layer write guard. It belongs more under the pull model, with a different trigger: mcp.json becomes the source of an argv KiroCrew spawns, and it is read on every call rather than once at enable, so a prompt-injected shell write to powers/<name>/mcp.json is command injection wearing the Power's name. Restored with that rationale.

Layout divergences, now recorded as choices rather than unknowns

Upstream: ~/.kiro/powers (KIRO_POWERS_HOME overrides), bundles under installed/<name>/, and a versioned envelope — {version: "1.0.0", installedPowers: [{name, registryId, autoInstalled?}], dismissedAutoInstalls: [...]} — plus registries/, registry-repos/, repos/. Here: our data home, a name-keyed map with {kind, ref} provenance, bundles at <name>/. Different trees, so no collision — and no interop, which is the first thing to revisit if kiro-cli ships a native Powers runtime. Their Power type carries iconUrl, the field this PR's cards already use.

Verification

• Spec-only change: docs/system-specs/modules/powers.md (+167/−10). No source file touched.
• 177 Powers tests still pass; flake8 clean.
• The round-30 finding (assert record is not None after the lock is released, so a concurrent delete makes a committed install report 500) is not addressed in this push. It is legitimate and small — build the response record inside _transaction — and is the next thing I will do unless the maintainer would rather see it separately.

[kyleseaman]

Round 32 — 78afe2b9

The round-30 finding is fixed. It was legitimate, and the fix is the one that was prescribed.

powers.py:1236 — a concurrent delete made a committed install report 500

load_power ran after _transaction released the store lock, so a remove_power for the same name could drop the record in that window. The assertion then fired and an install that had already committed reported HTTP 500. Worth adding to the original report: under python -O the assert is stripped, so the same window produced None["kind"] instead — a different exception, the same lie to the caller.

The response record is now read back inside the transaction, while the lock is held, so nothing can interleave between committing the record and reading it. Two supporting changes:

• load_power takes the transaction's pinned root handle for its provenance read, so an in-transaction read is anchored like every other one (rounds 25–26). Its bundle-file reads stay path-based, which is the same read-only posture list_powers uses and is now stated in the docstring rather than left as an accident.
• The impossible case — record missing while the lock was held, which would mean something mutated the store without taking the lock — raises a typed PowerFormatError instead of asserting. An assertion is not present under -O, so that path must not be allowed to degrade into None["kind"].

The test drives the actual window

test_concurrent_delete_cannot_fail_a_committed_install fires a real remove_power from a second store over the same directory, from inside the transaction at the last operation before the record read — the exact window the fix closes. It runs on a separate thread because the delete takes the same cross-process lock; with the fix, that thread cannot acquire it until the install finishes, which is the property under test.

Reverted, it fails with AssertionError at powers.py:1236 — the same failure the reviewer described, reproduced rather than approximated.

Verification

• 1,504 pass across the powers or security or provider or data_home selection
• isort, flake8, mypy clean
• Revert-verified as above
• No frontend change, so vitest / tsc were not re-run

Stack note

The follow-up activation work is open as draft #652 (power_list / power_steering, the read-only half of the pull model) and is stacked on this branch. I rebased it onto this head, so the two stay coherent; #652 carries exactly one commit of its own.

[kyleseaman]

Round 33 — 832e2ad7

The finding is right about the failure it names, and the fix is not the one prescribed. This is the fourth report on the uninstall ordering, so I want to be explicit about what I accepted and what I declined.

Accepted: the ENOSPC window was real

_rmtree_at succeeds, _drop_record then fails for lack of space, and the bundle is gone while a stale record remains. My own comment called this "the residual case" and waved it off as self-healing. That was too comfortable: the record self-heals, but the bundle does not come back.

Declined: retaining the tree until the record commits

That is the ordering declined in rounds 13, 17 and 19, and restating those grounds a fourth time would not move anything. The concrete objection is unchanged and now sharper: once the record is committed and the tree delete fails, the bytes are unreclaimable. remove_power returns early for a name with no record, so the round-27 .removing-* reconciliation never fires on it. That ordering trades a self-healing record inconsistency for orphaned data nothing can find.

Instead: prepare the write before the destructive step, commit after

_atomic_write_at is split. _stage_write_at allocates the space, writes the new installed.json and fsyncs it; _commit_staged_at finishes with a rename inside one directory, which needs no space. Uninstall now stages the record before _rmtree_at and commits it after.

Both properties hold at once:

• The record is still not committed until the bytes are gone, so a failed delete cannot orphan a bundle — the invariant rounds 13–19 were protecting.
• The step that used to fail with ENOSPC no longer allocates anything, so the reported failure cannot occur.

The failure tail is also better than either previous option: a crash between staging and the rename leaves the old record plus no bytes — the case load_power already reports as missing and a retry clears — and an uncommitted staged file is discarded on every failure path, since it describes a removal that did not happen.

On platforms without dir_fd the unsplit write still runs after the delete. That residual window is recorded in the spec rather than hidden.

Tests

Two, both revert-verified. test_full_disk_at_record_write_does_not_lose_the_bundle simulates ENOSPC and asserts the bundle and its record both survive; test_staged_record_is_discarded_when_the_delete_fails asserts no .installed.json.* residue.

The first was initially weak in a way worth flagging: it patched only the new helper by name, so reverting failed with AttributeError — which would have "passed" for any rename rather than for the behaviour. It now patches both the staged and unstaged write sites, so on the old code the failure lands after the delete and the assertions catch the actual data loss.

Verification

• 1,506 pass across the powers or security or provider or data_home selection
• isort, flake8, mypy clean
• Spec updated under Install and remove atomicity

[kyleseaman]

Round 35 — 7c8ad37a

One blocking finding, and it was mine to answer for: powers.py:1399, the no-dir_fd residual that round 33 documented instead of fixing.

The residual was real data loss, not a platform footnote

Round 33 staged the record write before the delete so a full filesystem could not destroy a bundle it then failed to record. I applied that only where dir_fd is available and wrote the rest off as "recorded rather than hidden". That was the wrong call: on Windows the old ordering stayed, so the exact failure the round-33 fix existed to prevent was still reachable.

It is fixed rather than re-documented, because the split was convenience rather than necessity. Staging needs a temp file in the same directory and an atomic replace — os.replace provides both on Windows. So _stage_write_at / _commit_staged_at / _discard_staged_at now take root_fd: int | None and run on every platform, and atomic_write is no longer imported here at all.

The useful consequence is a cleaner separation: confinement and durability are now independent. The pinned descriptor is still POSIX-only and still gates confinement; the record is durable before the bundle is destroyed everywhere. The spec's platform-split paragraph is replaced accordingly.

The test asserts the loss, not the exception

test_full_disk_does_not_lose_the_bundle_without_dir_fd forces _SUPPORTS_DIR_FD = False and injects ENOSPC. My first version patched only the new helper, so reverting failed on the pytest.raises wrapper — the old branch never reached that helper at all, which is a weaker signal than it looks. It now injects at both write sites and asserts the invariant directly. Reverted, it fails with AssertionError: bundle lost on the no-dir_fd path.

A second test covers the ordinary removal on that branch, so generalising the staging cannot have broken the success path.

Verification

• 1,559 pass across the powers or security or provider or data_home selection
• isort, flake8, mypy clean
• Revert-verified as above

Note on the Windows CI shard

Backend Tests (Windows) (2) is failing on test_file_change_snapshots.py, which is in neither PR's diff. The error is a hardcoded POSIX /tmp\tmpXXXX path, and it appears identically on this PR and on the stacked #652. Inherited, not caused here.

[kyleseaman]

Round 36 — aa76f089

One code finding, plus two checks that failed without producing any finding.

BLOCKING powers.py:1386 — the remove ordering, fifth report

The scenario is real and I am not disputing it: the gateway can die between _rmtree_at and _commit_staged_at, leaving installed.json naming a Power whose bytes are gone.

The prescribed fix — commit the record first, restore both if deletion fails — is declined for the fifth time, on an asymmetry that has not changed:

	Stale record (current ordering)	Orphaned bytes (prescribed ordering)
Visible to the UI?	No — load_power returns None, so list_powers hides it	No
Findable at all?	Yes — the name is in installed.json	No — list_powers enumerates installed.json, so a tree with no record is invisible
Reclaimed?	Yes, automatically (below)	Never

What was fair in the finding is that my previous answer — "load_power reports it as missing and retrying clears it" — put the repair on the user. So the residual is closed from the other end: _prune_absent_records runs at the start of every transaction, so the next install or uninstall of any Power repairs it. The stale record is now self-healing by construction rather than by someone happening to retry the exact Power that was interrupted.

Reconciliation skips names with a pending .removing-* or .backup-* tree, because those are mid-recovery rather than absent. That distinction is not theoretical: running the prune before the orphaned-backup recovery dropped the record of a bundle sitting in the rollback slot, and test_orphaned_backup_is_recovered_on_next_install caught it immediately.

Two tests, both revert-verified: one drives the interruption and asserts an unrelated Power's removal repairs it (reverted: stale record was not reconciled), one asserts a bundle awaiting restore keeps its record.

The other two failures produced no findings

• Opus 5 Review — cancelled at the 30-minute ceiling, outcome=cancelled, failing closed. No code finding emitted. Same stall class seen on feat(chat): ask_question tool — blocking clickable question card #464 and fix(approvals): stop background approvals hijacking unrelated conversations #612.
• Design Review — harness error, result is_error:true, run did not complete. No finding emitted.

Both need a re-run rather than a code change, and I have re-triggered them.

Verification

• 1,561 pass across the powers or security or provider or data_home selection
• isort, flake8, mypy clean
• Backend Tests (Windows) (2) cleared on its own — that was the inherited test_file_change_snapshots.py POSIX-/tmp failure, in neither PR's diff

[github-actions]

UX Review (Fable 5) — 🟡 CONCERNS

Advisory UX-level review of c31980857ed7a7e84af4dec08837c77d06d7ac84 — updated in place on each push; does not block merge.

UX-Verdict: CONCERNS

Usable and honestly-stated, but the browse flow has a lying counter, a misleading scope chip, and installs that yank you out of browsing.

Watch

• "Powers (0)" caption above 82 cards — the h4 renders Powers ({installed.length}) over both views, so in browse-dark.png the header says zero while the grid below shows a full catalogue; every browse visit reads as a contradiction. Scope the count to the Installed tab (Installed (2) on the tab button) or hide it in browse.
• "AWS" scope chip excludes the AWS cards users can see — every aws-* card in the screenshots carries an "Official" chip (dedup gives official ownership), so clicking the AWS filter hides the very cards named/authored AWS; the filter appears broken. Frequent, persistent, misleading. Filter on author or merge scopes for filtering.
• Install steals the view — onSuccess: () => { invalidate(); setView('installed') } navigates away from Browse on every install, so installing several Powers means re-clicking Browse and re-scrolling each time. Stay in Browse (the card already flips to the "Installed" badge) with a link to the Installed view.
• Remove is one-click, no confirm, no undo — remove.mutate(p.name) deletes the bundle from disk immediately; a folder-installed Power (the screenshot's /tmp/powers-seed-bundles/… rows) is unrecoverable if the source is gone. Add a confirm restating the action ("Remove Supabase") or an undo window.
• Error copy leaks the machine — "still warming up (HTTP 503)… once the provider layer is live" ships a status code and internal jargon; the non-503 branch prints raw Error.message. Rewrite both as what-happened + retry in user vocabulary.

Suggestions

• The identical two-sentence inert note repeats inside every installed row (see installed-dark.png); render it once above the list instead of per card.
• When the registry errors with zero items, the stale banner ("every registry provider is currently unavailable") and the EmptyState ("The marketplace mirror returned no entries") both render, saying the same thing in different jargon — keep one.

[UX-REVIEWED] c319808

[kyleseaman]

Round 37 — cd378662

Two findings, both accepted. One of them reverses a position I held for six rounds, and it deserves the explicit retraction.

BLOCKING powers.py:1441 — record commit after the delete makes a failed removal destructive

Accepted, and the ordering is now reversed. I declined this five times on the argument that orphaned bytes are unreclaimable — list_powers enumerates installed.json, so a tree with no record is invisible to every reader. That argument was wrong in a way I should have caught earlier: it is true of a bundle left at its own name, and not true of one renamed aside first. .removing-<name> is derived from the record's own name and sits directly under the store root, so it can be found by prefix with no record pointing at it. The rename-aside that has been in this code the whole time is what makes committing first safe. The clause in this round's prescription — retain failed cleanup in .removing-<name> — is what made that visible to me.

And the cost of the ordering I was defending is exactly what the finding says. It protected against a crash, but it made a failed uninstall destructive: with the bundle already deleted, a record replacement that fails returns an error for a Power that is in fact gone, and the bytes are unrecoverable at that point, so no amount of reconciliation undoes it. The Windows held-open-installed.json case is a concrete instance, not a hypothetical.

Now:

• rename aside → commit the record → destroy the bytes
• before the commit, nothing is authoritative: a failure restores the tree to its own name and loses nothing
• after the commit, the Power is removed, and a delete that fails is reclaimable garbage — logged and swept, not raised, because raising reported a failure for a removal that succeeded
• _reconcile_store runs both repairs at the start of every transaction: _prune_absent_records for records with no bundle, and the new _sweep_orphaned_removals for .removing-* trees no record names. A .removing-* whose name is still recorded is left to remove_power, which finishes that interrupted delete deliberately
• a bundle with no record is still deleted strictly — nothing is authoritative there, so a failed delete is a failed removal

Three existing tests pinned the old contract and are re-expressed rather than deleted, each stating in its docstring what changed. The inverted one is now test_failed_delete_after_commit_reports_success_and_is_reclaimed; its original intent moved to test_failed_record_commit_restores_the_bundle, which is where the property lives now. On revert the second fails with record lost on a failed removal — the destructive behaviour itself.

BLOCKING powers.py:1273 (reported on #652) — same-named Power silently overwrites

Accepted. A Power's identity is the name in its POWER.md and nothing reserves it globally, so a monorepo directory and an independent author's repo can both declare kb. The second install replaced the first bundle and its provenance record, leaving the store describing a Power the user never chose.

Now PowerSourceConflict → 409. Not 400 (the request is well-formed; the state is the problem, and the caller acts by uninstalling first) and not the generic 500 arm that previously swallowed it as "install failed". The dashboard already unwraps {"error": …}, so the reason reaches the user unchanged.

One deliberate narrowing, and the tests forced it. folder → folder is exempt: that is the development loop — reinstalling from a rebuilt or relocated directory — where the path is incidental rather than provenance and there is no third party to impersonate. Applying the rule as prescribed broke test_failed_reinstall_preserves_existing_bundle and test_failed_record_write_restores_the_previous_bundle for no security gain. Every other combination involves at least one remote source, including both directions between folder and registry, since losing a recorded upstream to a local directory of the same name is the same loss reversed. Same-source reinstalls stay allowed, so the upgrade path is untouched.

Two rebase-induced failures, both real

Rebased onto current main (was 121 behind), and two checks were failing for reasons that were not in the previous diff:

• Frontend Tests — App.test.tsx asserted the nav rail's overflow count as the literal "4 more". Adding an Apps surface makes it 5. Matched by shape now; the test is about dismissing the hover label, not the count.
• main added a gate requiring every built-in surface to carry an i18n labelKey. Powers had none. nav.powers added to the surface and both catalogues. The Chinese label is a best-effort term and a native reviewer should confirm it.
• Coverage Gate needed no change — it failed closed on the frontend job, not on a coverage threshold.

On the test discipline

Two of the new tests were vacuous first and both say so in their docstrings. The conflict revert initially failed with AttributeError because I had removed the exception class along with the check — that would pass for any rename, so the revert now removes only the check. And the handler test patched materialize_power_bundle, a name this handler never calls, so nothing was patched and the assertion read a 400 from the unpatched fetch; it drives the real install_from_dir now.

Verification

• 21,902 backend pass; 5,916 frontend across 485 files
• isort / flake8 / mypy src/kiro_crew/ clean; npx tsc -b clean
• 0 behind main

[kyleseaman]

Round 38 — 0d904112 (#408) / 977c8923 (#652)

Two findings, both mine, both accepted without argument.

BLOCKING powers.py:1521 — the retry branch still had the old ordering

Correct, and it is the worse place to have left it. Round 37 moved the main path to commit-before-delete but did not touch the recovery branch, which handles the state a process leaves when it dies after the rename-aside — precisely the state where .removing-<name> is the only copy of the bytes. That branch deleted them and dropped the record afterwards, so a record replacement that failed destroyed the bundle with nothing to restore from. The fix I shipped last round made this strictly more reachable, since the sweep now leaves .removing-* trees around for the retry to find.

The branch now splits on whether a record still claims the name:

• recorded → rename the tree back to its own name and fall through to the durable path, which commits while the bytes still exist
• not recorded → the removal already committed, so the leftover is swept and reported as already removed. Raising there reported a failure for a removal that succeeded

test_record_survives_when_the_leftover_tree_cannot_be_removed asserted the contract this replaces and is re-expressed as test_retrying_an_interrupted_uninstall_cannot_destroy_the_bundle. On revert it fails with the only copy of the bundle was destroyed.

BLOCKING powers.py:552 (reported on #652) — hardlinked contract files

Also correct. The sensitive-path refusal is path-based, so a hardlink walks around it: steering/guide.md sharing an inode with a protected file was copied into the Powers tree, where an agent can read the bytes back through power_steering. Nothing existing caught it, and the reason is worth stating precisely — the symlink guard compares lstat identity against the opened fstat, and a hardlink is the file, so the two agree and there is no link to detect.

st_nlink > 1 is now rejected. That is the same rule hooks.safe_read_file_bytes_nolink applies on the read side and the same reason the state write uses O_EXCL, so the store is now consistent about extra links on both sides. The guard's own risk is over-rejection, so a second test pins that an ordinary single-link bundle still installs.

Not from this diff

• Backend Tests (Windows) (3) — test_platform_compat.py::TestProcessCommandLine::test_self_cmdline_mentions_python fails with an empty command line on the Windows runner. Neither PR touches platform_compat.py.
• PR Hygiene on feat(powers): agent-facing power_list and power_steering tools (read-only) #652 counts two commits because it is stacked on feat(powers): browse, install and remove Kiro Powers (inert; activation split out) #408; it clears when feat(powers): browse, install and remove Kiro Powers (inert; activation split out) #408 merges. The commit to review there is its own single one.

Verification

• 21,887 backend pass (the earlier test_ws_ctrl_c_delivers_sigint failure was a PTY timing flake — passes in isolation, and no terminal code is in either diff)
• isort / flake8 / mypy src/kiro_crew/ clean; npx tsc -b and 5,917 frontend tests clean
• feat(powers): agent-facing power_list and power_steering tools (read-only) #652 rebased onto this head; both worktrees npm ci'd after the rebase, since a stale node_modules silently hid the frontend gate twice before

[kyleseaman]

Round 39 — 680cc7b0 (#408) / e729e1a6 (#652)

BLOCKING powers.py:1078 — POWER.md read without a byte cap

Accepted. Worth being precise about what was actually wrong, because it is not what it looks like: parse_power_md has always refused over MAX_POWER_MD_BYTES. It just did so after read_text() had pulled the whole file into the process. So the guard existed downstream of the allocation it was meant to prevent.

And the size is not fixed at install time, which is what makes it reachable: the installed bundle stays editable after the copy, and the install source is caller-owned before any copy budget applies. Anything able to write into a bundle could exhaust memory in the MCP process through power_list.

All three read sites now go through _read_power_md — the pre-install parse, load_power, and the staged re-parse. It reads one byte past the cap and refuses rather than truncating: a truncated POWER.md either fails frontmatter parsing with a confusing error or, worse, parses as a different Power than the file describes.

The first version of these tests was vacuous, and the reason is worth recording

I wrote three tests asserting the refusal. All three passed with the fix reverted — because the refusal is not the change. The observable behaviour is identical either way; what changed is which read path runs. That is a variant of the trap I keep hitting: asserting the outcome when the fix moved the mechanism.

They now ban Path.read_text for POWER.md with a RuntimeError sentinel — deliberately not OSError or PowerFormatError, either of which load_power swallows, which would make the sentinel indistinguishable from the capped refusal and leave the test green either way. Three of four fail on revert with unbounded read of <path>. A fourth covers the ordinary-size case, so a fix that kept read_text() for small files and only capped after a size check would not satisfy the suite.

Locales

Main added bn/es/fr/hi/pt/ru since the last push and catalogParity requires every catalogue to cover en exactly, so Frontend Tests went red on a missing nav.powers in six languages. Added to all of them in English, and the Chinese label I invented last round is reverted to match: Powers is the Kiro feature name, and i18n-shard.mjs itself reports keys identical to English as "expected for proper nouns/product names". One language guessing while five stay English was the inconsistent option. A native speaker should still make the call per language.

Not from this diff

• Build Wheel on feat(powers): agent-facing power_list and power_steering tools (read-only) #652 failed with a V8 heap OOM during the frontend build. It passes on feat(powers): browse, install and remove Kiro Powers (inert; activation split out) #408 with the identical frontend and on other open PRs, so it was runner-transient; re-triggered by this push.
• Two backend tests failed in the full parallel run and a different two failed on the next run (test_mcp_gateway_wedge_ping_gate, test_dashboard_approval, test_browser_recording_skill). All pass in isolation; the diff touches none of those modules. The browser-recording one needs Node 18+ and the host default is Node 16.

Verification

• 21,959 backend pass (minus the parallel flakes above, each confirmed green in isolation)
• 5,999 frontend across 488 files; npx tsc -b clean
• isort / flake8 / mypy src/kiro_crew/ clean — run unpiped, after a tail -1 pipe hid isort errors on another PR earlier today

[kyleseaman]

Round 40 — 7b603b81 (#408) / 2d7de2c3 (#652)

BLOCKING powers.py:335 — unbounded mcp.json read

Accepted; same class as round 39 and the same reachability argument. _declares_mcp_servers read the whole file, and it runs on GET /api/powers for every installed Power, so one oversized file affects the whole listing rather than just its own card. The copy-time budget does not bound this because the bundle stays editable after install.

Capped at a dedicated MAX_MCP_JSON_BYTES (256 KiB) rather than the prescribed MAX_INSTALL_BYTES + 1. That is a deliberate tightening, not a disagreement: 8 MiB is the whole-bundle budget, and this file is a server map, not a document — no real spec approaches either bound, and the tighter one is the honest description of what the file is.

One behavioural choice worth flagging: over-cap returns "declares nothing" rather than raising. This function answers a yes/no question used to label a card, and install-time validation is what refuses malformed bundles; raising here would let one oversized file break the entire listing, which is the failure mode the surrounding code already guards against for malformed records.

Both tests ban Path.read_text for mcp.json and fail on revert — including the ordinary-size one, so the capped path cannot be a branch taken only once the file is already large.

The build OOM is not this diff — I measured it

E2E (stub ACP backend, offline) on #408 and Build Desktop / Build Wheel on #652 failed with a V8 heap OOM in the Build frontend step (~2 GB, "Ineffective mark-compacts near heap limit"). Rather than assume, I reproduced the ceiling locally:

NODE_OPTIONS="--max-old-space-size=2048" npx vite build   ->   built in 42.43s

The branch builds inside CI's heap limit, and the same frontend passed Build Wheel on #408 in the previous round and passes on other open PRs. So this is runner memory pressure, not the diff. Re-triggered by this push.

Worth noting independently of these PRs: the build already warns that chunks exceed 3,810 kB, so the frontend is close enough to the ceiling that unrelated PRs will keep hitting this. That is a main-branch concern, not something either PR should absorb.

Also not from this diff

• Backend Tests (Windows) (3) — test_platform_compat.py::TestProcessCommandLine::test_self_cmdline_mentions_python, empty command line on the Windows runner. Neither PR touches platform_compat.py.
• PR Hygiene on feat(powers): agent-facing power_list and power_steering tools (read-only) #652 — two commits because it is stacked; clears when feat(powers): browse, install and remove Kiro Powers (inert; activation split out) #408 merges.

Verification

• 21,963 backend pass, clean run with no flakes this time
• isort / flake8 / mypy src/kiro_crew/ clean, unpiped
• npx tsc -b and 5,999 frontend tests clean as of the previous round; this round's diff is backend-only

[kyleseaman]

Round 41 — c0b6fbd5 (#408) / e065c110 (#652)

GPT is now clean on #652; this round is #408 only.

BLOCKING powers.py:470 — a junctioned source steering/ was followed

Accepted. Path.is_symlink() is False for an NTFS directory junction, so the no-descriptor branch fell through to is_dir() — which a junction satisfies — and globbed the target, copying Markdown from outside the selected source into the agent-readable Powers tree. This is the same reparse-point class already guarded at the store root in round 29, and the guard belongs on this branch specifically: the POSIX branch pins steering/ with O_NOFOLLOW and enumerates through that handle, so it never consults a path at all.

One behaviour change beyond the prescription: a symlinked steering/ is now refused rather than silently skipped. Skipping looks like the safe option and is precisely what let the junction through — the branch's job became "detect a link" instead of "refuse a redirect". It would also install a partial bundle from a source tampered with mid-install, which is the failure the sibling checks already refuse.

My first three tests for this were worthless, in two separate ways

Worth writing down because both are new variants:

1. They drove install_from_dir, which pre-validates the source tree and already refuses a symlinked steering/ with its own "symlink not allowed" error. My match="symlink" happily accepted that message, so the test passed with the fix reverted — it was asserting a guard that has existed for many rounds.
2. The _SUPPORTS_DIR_FD = False patch did not select the branch under test, because on the POSIX path steering/ is opened with O_NOFOLLOW before the path check is reached. So even the branch was wrong, not just the assertion.

They now call _copy_steering directly with src_fd=None. The junction itself is not constructible on Linux — is_symlink() being False for one is the entire defect — so one test simulates that shape (forcing is_symlink() False for the steering component) and asserts the reparse check is consulted before the glob, with the copied-file check as the backstop. Two of the four fail on revert; the other two are the over-rejection guards (ordinary bundle still copies, absent steering/ still a no-op).

Verification

• 21,951 backend pass
• isort / flake8 / mypy src/kiro_crew/ clean, unpiped
• This round's diff is backend + spec only

Backend Tests (Windows) (3) was still mid-run when I looked, so I have not read its log yet rather than guessing at it — I will report it next round. PR Hygiene on #652 remains the stacked-commit count.

[kyleseaman]

Round 42 — c3198085 (#408) / 5d645f50 (#652)

Two findings, both accepted, plus one CI failure that was genuinely mine.

BLOCKING powers.py:1330 — blocking probe on the event loop

Accepted. install_from_dir stat'ed the source POWER.md on the loop and only then handed the tree walk and parse to maintenance_executor(). One stat looks free next to a tree walk, but the source is API-supplied and can be a network mount, where that stat blocks for as long as the mount takes to answer — stalling chat, the heartbeat and every dashboard request, not just this install. Probe, walk and parse are now one blocking callable, so no ordering remains in which a caller-supplied path is touched from the loop.

The test asserts the thread the probe runs on, not latency: a timing test would be flaky and would not identify the offending call. It captures the loop's thread id inside the coroutine, so anything recorded on it is by definition blocking the gateway. Fails on revert.

BLOCKING marketplace.py:429 (reported on #652) — cache write followed a symlink

Accepted, and this is the sharpest of the recent set. Path.write_text opens and truncates through a link, so a symlink at the cache path redirects a routine background registry refresh to whatever it names. The cache sits in the same directory as installed.json, which makes the reachable case the store's own record being overwritten with registry JSON — the provenance of every installed Power destroyed by a refresh that has nothing to do with the store.

Now atomic_write (temp file plus rename), which replaces the entry rather than following it. Confirmed this was the only unguarded writer in powers_providers/ rather than fixing just the flagged line.

The test asserts the victim file is byte-identical afterwards and that the link was replaced; on revert it fails with the registry payload sitting in the victim.

The Windows failure was mine

Backend Tests (Windows) (3) — TestRound39PowerMdCap::test_a_power_md_just_under_the_cap_still_installs, failing with POWER.md exceeds the 256 KiB cap. My own test from last round: it pads the file to just under the cap, and text-mode writes translate every newline to CRLF on Windows, inflating it past the cap. The write is byte-exact now (newline=""). Same class I hit on #795 — a size-sensitive fixture must not go through newline translation.

Backend Tests (Windows) (4) on #652 was test_slack_handler.py::TestToolElapsedTimer, unrelated. Build image (arm64 cross-build) and E2E are the frontend-build OOM already measured as environmental last round.

Verification

• 21,970 backend pass
• isort / flake8 / mypy src/kiro_crew/ clean, unpiped
• feat(powers): agent-facing power_list and power_steering tools (read-only) #652 restacked; both worktrees pushed

[iamwhatever]

Hey @kyleseaman 👋

This PR has merge conflicts and is currently 375 commits behind main — main has moved significantly since your last push on Jul 30. The CONFLICTING state means CI can't run and reviewers can't evaluate the current code.

What's needed to move forward:

1. Rebase onto latest origin/main and resolve the conflicts (at minimum security.py will conflict given the recent changes there)
2. Force-push the rebased single commit
3. Verify local gates still pass after the rebase (pytest, mypy, isort, flake8, tsc, vitest)

Quick check: Are you still planning to pursue this feature? The work here is substantial and impressive (23 review rounds, 121+ tests, thorough security hardening), and nothing equivalent has landed on main — so there's no duplication concern. But given the staleness and the volume of base-branch movement, I want to confirm you're still actively working on it before it drifts further.

No rush — just let us know your plans so we can prioritize review accordingly. If you need any help with the rebase conflicts, happy to assist.