feat(powers): agent-facing power_list and power_steering tools (read-only) by kyleseaman · Pull Request #652 · kirodotdev/KiroCrew

kyleseaman · 2026-07-28T13:49:40Z

What

First slice of Powers activation: two agent-facing tools on the existing kirocrew-core MCP server that let an agent see what Powers are installed and read a Power's guidance on demand.

power_list()                    -> installed Powers + declared MCP server NAMES + steering files
power_steering(power, file)     -> the contents of one steering/*.md

Neither starts a process. The two that do — power_learn and power_use — are deliberately not here; see Scope below.

Stacked on #408. This branch is cut from feat/powers-registry, because it needs that PR's PowersStore. Until #408 merges, the diff shown here includes it. Review #408 first; the commit to review on this PR is a13cd769 alone.

Why this shape — the design changed

Every earlier version of this repo's activation plan was push: merge a Power's MCP servers into the rendered ~/.kiro/agents/kirocrew.json, materialize its docs into a skill so SkillsLoader could trigger them, and gate both behind a trusted flag in installed.json.

Reading the IDE's own implementation (kiro-team/kiro-extension, src/extension/powers/**) shows upstream does neither. 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.

Following that shape deletes the hardest parts of the old plan rather than solving them:

The includeMcpJson constraint never applies. It dominated the old design — 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 here.
No power-<power>-<server> namespace, so the injectivity problem (power-git- also prefixes power-git-helper-srv) and the ownership record its purge required both vanish.
No skill materialization, so third-party markdown never enters a session unasked and cannot be keyword-triggered.
No trusted / enabled fields, which is the consequence that matters for feat(powers): browse, install and remove Kiro Powers (inert; activation split out) #408: the installed.json it ships needs no migration.

Scope: this slice never starts a process

power_list parses mcp.json for its top-level mcpServers keys and nothing else. The command, args and env of a declared server are never read on this path — those belong to the spawn path in the next slice. So #408's boundary moves from "mcp.json is never parsed" to "parsed for names, never for commands", and no argv path exists yet.

test_powers_tools.py asserts that on the serialized payload rather than on the response shape, so a future field that happens to carry a command through cannot pass silently. A second test asserts power_learn / power_use are absent from the advertised tool list — the same discipline #408 used to assert the absence of an activation toggle.

Consent: deliberately none, here

Neither tool raises a prompt, and the reasoning is recorded in the spec rather than left implicit. Both are reads of content the user chose to install, performed only when the agent names a Power, with nothing executed — there is no decision for a human to make that the install did not already make.

This is also the property the push design could not offer: it fed guidance in through SkillsLoader keyword triggers whether or not the agent wanted it, which is exactly why it needed a gate that this shape does not.

Consent attaches to execution. When power_use lands it will raise an approval naming the Power, the server and the resolved command, with per-session Trust — the same conclusion PR #518 reached when it deleted bespoke trust grants and routed through core approvals.

Security controls

Steering filename treated as untrusted input, not as a parameter the model got right: plain .md leaf only, no separators, no leading dot, and the path resolved through the store's existing _power_path barrier so there is one implementation of that rule instead of a second weaker one.
Symlink at the target refused. Install vets the bundle, but the tree stays writable by the same user afterwards, so the read path cannot assume install-time vetting still holds.
Mandatory redaction on both tools' output (powers_providers/redact.py) — a Power's text is third-party content on its way into a transcript.
Bounded reads: 128 KiB per steering file (reported as truncated), 256 KiB when parsing mcp.json for names. An unbounded read is a context-exhaustion lever held by whoever wrote the bundle.
SEL audit on both calls.
A malformed mcp.json degrades to "no servers" rather than breaking the whole listing — the Power still exists and is still removable.

Verification

20 new tests; 1,519 pass across the powers or validation or mcp_core or security selection
isort, flake8, mypy clean
Revert-verified: defeating the path guard, the symlink refusal and the redaction fails 7 of 20

One honest note on those tests. My first version of the traversal cases matched any PowerToolError and was vacuous — ../secret.md reached the filesystem, missed, and raised "no such steering file", so it passed with the path guard removed. They now pin the refusal reason and create the traversal target, so a case can only pass through the guard that is meant to stop it. That took the revert-failure count from 2 to 7.

Next slice

power_learn + power_use: sandboxed spawn reusing the hardening McpToolClient already applies (wrap_argv(mode="standard"), cgroup_scope_argv, resource_limit_preexec, stderr to a file), argv from the bundle's own mcp.json, consent prompt naming the resolved command, per-call timeout and output bound, SEL event per invocation, and a bash-layer write guard for powers/<name>/mcp.json — which becomes load-bearing once that file is the source of an argv read on every call.

Full design: docs/system-specs/modules/powers.md, Deferred: activation (pull model) and Upstream Powers implementation (observed, not inferred).

github-actions · 2026-07-28T13:56:24Z

GPT 5.6 Review — 🔴 changes requested (blocking)

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

This comment is updated in place on each push.

BLOCKING -- src/kiro_crew/powers_tools.py:130 -- Agent tools bypass the sensitive-path read gate
for record in store.list_powers(): / if store.load_power(power) is None:
Symlinked installed POWER.md -> power_list/power_steering -> load_power() opens the target directly -> protected credential files are read outside safe_read_file_bytes_nolink.
Fix: Revert these tool handlers until their metadata reads use the no-link safety gate.

[GPT-REVIEWED] 5d645f5
[BLOCK-MERGE] 5d645f5
False positive or not applicable? A repository writer can comment:
/ai-review override gpt 5d645f5013b62f2e3eabaeb1199e7f8b59f1e083: <one-sentence reason>

github-actions · 2026-07-28T14:09:16Z

Arbiter — ⏳ review pending

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

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

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

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

github-actions · 2026-07-28T14:18:07Z

Design Review (Fable 5) — ✅ PASS

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

Design-Verdict: PASS

Pull-model tool surface mirrors upstream, deletes the push design's config-mutation problems, and splits scope exactly at the process-execution boundary — sound and proportionate.

Suggestions

power_steering reaches into store._power_path (private) and re-implements the steering-dir/symlink/leaf checks outside the store; moving the steering read into PowersStore would keep the path-confinement rule owned by one module instead of split across two — worth doing before power_learn/power_use add a third caller.
The advertised installed_only parameter ("Reserved; only true is supported today") whose only non-default value errors will produce failed agent calls for nothing; drop it from the schema until the registry-backed listing exists.

[DESIGN-REVIEWED] 5d645f5

github-actions · 2026-07-28T14:24:01Z

Opus 5 Review — ✅ no blocking findings

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

Review details

No findings.

[OPUS-REVIEWED] e065c11

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

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

github-actions · 2026-07-30T15:01:56Z

UX Review (Fable 5) — 🟡 CONCERNS

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

UX-Verdict: CONCERNS

Prominent Install CTAs promise agent capability, but Powers are inert this release — users learn that only after installing, from a tooltip or post-install note.

Watch

The promise/reality gap is disclosed too late. Empty state says "Browse the registry to add MCP tools and expert guidance to your agent" and every card carries a primary Install button, yet the installed result is an Inactive bundle whose note admits "Running its tools arrives in a later release." The only pre-install disclosure hides in an InfoTip behind a "?" icon. Every first-time user of this surface hits it, every install, until execution ships. Fix: promote one visible line onto the Browse view ("Installed Powers are inactive until tool execution ships") and soften the empty-state promise.
onSuccess: setView('installed') yanks the user out of Browse after every install — installing several Powers means re-clicking Browse and re-finding your place each time. Frequent for the browse flow, friction not failure. Fix: stay in Browse and let the card flip to the Installed badge (the feedback already exists in place).
Error/empty copy speaks implementation, not task: "The registry is still warming up (HTTP 503). It becomes available once the provider layer is live" and "The marketplace mirror returned no entries." A user can't act on "provider layer." Fix: "The Powers registry isn't reachable right now — try again shortly." / "The registry returned no Powers."

Suggestions

Header reads "Powers (0)" directly above a Browse grid of ~15 cards (browse-dark.png) — the count is installed-only; move it into the tab label: Installed (0).
The view switcher uses role="tab"/aria-selected but no arrow-key navigation; either add it or use plain aria-pressed toggle buttons like the scope chips.
PageHeader subtitle "browse the registry, install to disk, remove when done" — drop the mechanism: "Browse and install capability bundles for your agent."

[UX-REVIEWED] 5d645f5

kyleseaman · 2026-07-30T15:24:37Z

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

Adds a Powers surface to the left rail's Apps group: browse the upstream registry (official `kirodotdev/powers` monorepo + a mirror of the `kiro.dev/powers` marketplace), install a bundle, list what is installed, and remove it. Installed Powers are INERT, and that is the point rather than an omission. An installed bundle is unreachable by construction: * No MCP server is registered. The bundle's `mcp.json` is never parsed for specs — only tested for presence, to label the Power `mcp` vs `knowledge` — so a declared command has no path to `_set_kirocrew_entry` or any other execution site. * No skill is materialized. `POWER.md` and `steering/*.md` stay inside `powers_dir()`, which nothing else reads: `skills.py`, `context.py` and `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 does not implement. The UI says so plainly — an Inactive badge and a note that nothing is registered and no guidance is loaded — instead of showing a dead toggle. Tests assert the ABSENCE of any activation affordance. Activation (trust grant, enable/disable, `power-<power>-<server>` MCP registration with ownership-exact purge, the generated docs skill) is a follow-up PR. It is not a flag flip: `includeMcpJson` is false, so sessions read the rendered `~/.kiro/agents/kirocrew.json` and every existing MCP mutation path rebuilds it. An activation that wrote only `<data home>/mcp.json` would be a no-op on enable and would leave a revoked server live. `docs/system-specs/modules/powers.md` records that constraint under "Deferred: activation". Security controls retained here: * `fetch.py` — HTTPS-only host allowlist, no `git clone`, traversal and symlink rejection, byte/file/depth bounds, bounded timeout, temp tree removed on every failure path including cancellation. * Allowlist copy of exactly the contract files, so pointing a `folder` install at an allowed ancestor cannot relocate `~/.ssh` under the powers dir. * `resolve_install_source` refuses sensitive paths and audits the denial (the success-path SEL event would not fire on a refusal). * `powers/` is write-protected at the agent file-edit gate: `.marketplace-cache.json` decides which repository an id resolves to, and `installed.json` is the provenance shown in the UI. Reads stay allowed. * Atomic install (stage / rollback / commit) with the staged `POWER.md` re-parsed under the lock, since 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. Marketplace parsing is bounded by neighbouring launchers rather than a fixed window (a fixed window pairs each card with its predecessor's repository), provider failures surface as stale-with-banner rather than as an empty catalog, and unavailability is not latched. Install and remove are each ONE blocking transaction: a single callable that takes the cross-process lock, performs every mutation, rolls itself back on failure, and releases the lock, awaited under `asyncio.shield`. Compensation that lives in the coroutine cannot be made correct — a cancelled task re-raises from every subsequent await (proven on 3.10 in CI), so awaited compensation silently never runs, while waiting synchronously to avoid that stalls the event loop. With one worker there is nothing to compensate from outside and nothing to settle, so `_submit_blocking`, `_settle_for_compensation`, `_settle_blocking`, `_run_drained` and `_powers_transaction` are deleted rather than tuned. Confinement comes from a pinned descriptor rather than a check: `_root_lock` opens the powers root `O_NOFOLLOW|O_DIRECTORY` once and every rename and delete goes through it, so a root swapped for a symlink mid-transaction cannot redirect a mutation onto files outside the store. That retires `_assert_root_not_symlinked`, which was one operation away from the mutation it guarded. `_SUPPORTS_DIR_FD` gates the POSIX path; Windows keeps the path-based check, where creating a symlink requires elevation. Every provider body is read through a looping `read_capped` helper rather than one `StreamReader.read(cap + 1)`. `read(n)` returns whatever is buffered when it wakes and does not loop to fill n, so a single call truncated any body larger than one wire chunk: measured live, 57 KiB of a 649 KiB marketplace page and 9.8 KiB of a 25.6 KiB GitHub JSON document. The symptoms were not read errors but provider errors — the scrape found zero cards, the official provider failed `json.loads`, both surfaced as "provider unavailable", and browsing the real registry returned nothing. The per-file byte cap was equally unenforceable, since a short read never reached it. The unit suite stubs all three HTTP seams, so the regression tests run against a loopback server. Because that fix makes the official provider reachable for the first time, provider overlap now matters: dedup is by canonical repository URL with provider order deciding ownership, and the losing duplicate donates any facet the winner left blank. The official provider lists a GitHub directory, so `description`/`author`/`category` are structurally empty for it and overlapping cards previously rendered authorless. `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 its card shows AWS as the author. The spec records that divergence. Registry cards carry the publisher icon the marketplace listing already exposes (76 of 76 cards). The URL is scraped from a third-party page and becomes an `img` source, so its origin is an ALLOWLIST enforced at one chokepoint, `marketplace.valid_icon_url`: https only, exactly the Kiro asset host, and a `/powers/icons/` path prefix. It is applied both when scraping and when reading the disk cache back, because the cache is on-disk state the spec already treats as attacker-reachable, and a refused icon degrades the card rather than dropping the Power. Icons also justify the blank-facet merge added earlier: 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 when the host fails to serve the image. Every mutation inside a transaction is anchored to the pinned root descriptor, not just the renames: the staging mkdir, the recursive delete, the contract-file copy and the `installed.json` write. The gap was not theoretical — a lexical `shutil.rmtree` walk reopens each component by name, so a root swapped mid-walk deleted an external tree, and a lexical state write put the record describing the store outside it. The recursive delete is hand-written against `dir_fd` because `shutil.rmtree` gained that parameter later than the Python versions CI runs, and the copy receives a descriptor for the staging directory so a decoy of the same pid-derived name cannot receive the bundle. Provider imports in the dashboard handler moved to module scope inside a `try/except`, which satisfies the top-level-imports rule while keeping the degradation the function-local form existed for: the failure is caught once at import, and each handler checks a flag instead of catching its own ImportError. Transaction state READS are anchored too. An anchored write over a lexical read is worse than neither: the record lands in the right place with content read from a decoy, so an install rebuilds `installed.json` from foreign state and erases every other Power's provenance. The read-only `list_powers` / `load_power` paths keep the plain read, where a decoy yields a wrong listing but cannot destroy state. The install SOURCE is pinned as well as the store. `steering/` is opened once with `O_NOFOLLOW|O_DIRECTORY` and enumerated through that handle, and contract files are opened relative to a pinned source descriptor: a check-then-glob re-resolves the directory name, and per-file `O_NOFOLLOW` does not object to what it then finds, because after a swap those are ordinary regular files that merely live outside the source the caller offered. Crash recovery no longer destroys bytes. With both `dest` and `.backup-<name>` present the interrupted transaction's intent is not recoverable from disk, so the backup is quarantined under a timestamped name with a warning instead of deleted — it may be the only copy of the bundle the live record still describes. On the remove side a leftover `.removing-<name>` is reconciled before the record is dropped, and if it still cannot be deleted the record is kept, so bytes are never left untracked. `github_url` is re-validated on cache read as well, and the Source link is routed through `safeHttpUrl` and omitted when rejected. The icon URL gained that treatment earlier while the repository URL did not, which was the worse omission of the two: an icon becomes an `img src`, whereas the repository URL becomes an `href` the user clicks, so a poisoned cache entry would have been click-to-execute. The backend skips such an entry rather than blanking the field, since the URL is both the dedup key and the install target and a card without a trustworthy one is not usable. Two gaps in that confinement code are closed as well. The state write now uses a random temp name with `O_CREAT|O_EXCL` instead of a fixed `.installed.json.tmp` with `O_TRUNC`: `O_NOFOLLOW` refuses a symlink at that path but says nothing about a hardlink, so a preplanted link would have been truncated and then filled with store state. And the root check rejects Windows directory junctions, not only symlinks — `Path.is_symlink()` returns False for an NTFS reparse point, and the path check is the only guard on the platform without dir_fd support, so the one platform that depended on it had the incomplete test. The deferred activation design is rewritten around the model the IDE actually uses. Reading `kiro-team/kiro-extension` shows upstream neither merges a Power's MCP servers into agent config nor injects its docs into context: it exposes `listPowers` / `usePower(power, server, tool, args)` / `readPowerSteering(power, file)` and the agent pulls. Following that shape deletes rather than solves the hardest parts of the previous plan — the `includeMcpJson` rebuild constraint, `power-<power>-<server>` namespacing with its ownership-exact purge, skill materialization, and the `trusted`/`enabled` fields, which means `installed.json` as shipped here needs no migration. Consent becomes a per-session approval naming the resolved command, matching the decision PR #518 reached when it deleted bespoke trust grants. Recorded in the spec under `Deferred: activation (pull model)` and `Upstream Powers implementation (observed, not inferred)`, with the observed upstream layout and the divergences from it tabulated as choices. The install response record is read back inside the transaction, while the lock is still held. Reading it afterwards left a window in which a concurrent `remove_power` for the same name could drop the record, so an install that had already committed reported a failure — and under `python -O`, where the old assertion is stripped, the same window produced `None["kind"]` instead. `load_power` takes the transaction's pinned root handle for its provenance read so an in-transaction read is anchored like every other one, and the impossible case now raises a typed error rather than asserting, because an assert is not present under optimisation. Uninstall prepares its record write before destroying the bundle. The write previously ran only after the delete, so a full filesystem could succeed at deleting and then fail with ENOSPC, losing the bundle while a stale record remained. `_stage_write_at` allocates and fsyncs the new `installed.json` up front and `_commit_staged_at` finishes with a rename inside one directory, which needs no space. The ordering the spec commits to is unchanged — the record is still not committed until the bytes are gone, so a failed delete cannot orphan a bundle — while the window that ordering left open is closed. An uncommitted staged file is discarded on every failure path. Platforms without `dir_fd` keep the unsplit write and its residual window, recorded rather than hidden. Rebased onto main. Two integration points needed real resolution rather than a mechanical merge: `surfaces/builtins.tsx` gained a `selectSubagentActivityCount` import alongside this branch's `PowerIcon` (both are used, so both are kept), and main's new security-posture drift guard requires every redactor call site to be either a registered sink or an explicitly-reasoned allowlist entry. The two Powers egress boundaries are registered as sinks — the provider listing, which redacts scraped marketplace metadata, and the Powers HTTP handlers, which redact every API body — while `powers_providers/redact.py` is allowlisted because it only defines the helpers, exactly as `security.py` does for the base scanners. Record staging now runs on every platform, not only where `dir_fd` is available. The previous revision applied it on POSIX and recorded the rest as a known residual — but that residual was a real data-loss window kept for convenience: staging needs a temp file in the same directory and an atomic replace, and `os.replace` provides both on Windows. Confinement and durability are now independent concerns: the pinned descriptor is still POSIX-only and still gates confinement, while the record is durable before the bundle is destroyed everywhere. `atomic_write` is no longer imported here because stage-then-commit replaces it on both branches. Records with no bundle are reconciled at the start of every transaction. The remove ordering deliberately destroys the bytes before committing the record, so an interruption in that window leaves a record naming a Power whose files are gone; `load_power` already hid it, but "retry and it clears" put the repair on the user. `_prune_absent_records` now repairs it on the next install or uninstall of any Power. Names with a pending `.removing-*` or `.backup-*` tree are skipped because they are mid-recovery, not absent — running the prune before the backup-recovery step stranded a bundle awaiting restore. The alternative the reviewer prescribed, committing the record before deleting, is declined for the fifth time on the same asymmetry: a stale record is discoverable and now self-healing, while orphaned bytes are neither, because `list_powers` enumerates `installed.json` and a tree with no record is invisible to it. Round 37: uninstall now commits the record BEFORE destroying the bundle. This reverses the ordering the previous rounds defended, because the objection that kept it no longer holds. The refusal rested on orphaned bytes being unreclaimable -- `list_powers` enumerates `installed.json`, so a tree with no record is invisible to every reader. That is true of a bundle left at its own name. It is NOT true of one renamed aside first: `.removing-<name>` is derived from the record's own name and sits directly under the root, so `_sweep_orphaned_removals` finds it by prefix with no record to point at it. The rename-aside that was already there is what makes committing first safe. What the old ordering cost: it protected against a crash but made a FAILED uninstall destructive. With the bundle already deleted, a record replacement that fails -- a held-open `installed.json` on Windows being the concrete case -- returned an error for a Power that was in fact gone, and the bytes were unrecoverable, so no reconciliation could undo it. Now the commit is the only step that decides the outcome: before it, a failure restores the tree and loses nothing; after it, the Power is removed and a failed delete is reclaimable garbage that is logged and swept, not raised. `_reconcile_store` runs both repairs at the start of every transaction -- `_prune_absent_records` for records with no bundle (still reachable via a hand-deleted bundle) and `_sweep_orphaned_removals` for the new residual. A `.removing-*` whose name IS still recorded is left to `remove_power`, which finishes that interrupted delete deliberately. A bundle with no record at all is still deleted strictly: nothing is authoritative there, so a failed delete is a failed removal. Three tests pinned the old contract and are re-expressed rather than deleted, each saying in its docstring what changed and why. The inverted one (`a failed delete keeps the record`) is now `test_failed_delete_after_commit_reports_success_and_is_reclaimed`, and its original intent -- a failed removal must lose nothing -- moved to `test_failed_record_commit_restores_the_bundle`, which is where the property actually lives now. Both fail on revert, the second with "record lost on a failed removal": the destructive behaviour itself. Also fixed, from the rebase onto current main (121 commits): - `App.test.tsx` asserted the rail's overflow count as the literal "4 more". Adding an Apps surface makes it 5. Matched by shape now, since the test is about dismissing the hover label, not about the count. - main added a gate requiring every built-in surface to carry an i18n `labelKey`. Powers had none, so `nav.powers` is added to the surface and to 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. Also round 37: a same-named Power from a DIFFERENT source no longer silently replaces the installed one. 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 repository can both declare `kb`. Installing the second replaced the first bundle AND its provenance record, leaving the store describing a Power the user never chose, from a source they never picked. That is now `PowerSourceConflict`, mapped to 409 -- the request is well-formed and the STATE is the problem, so the caller can act on it, which a 400 would misreport and the generic 500 arm hid entirely behind "install failed". The dashboard already unwraps `{"error": ...}` and renders it, so the reason reaches the user unchanged. The rule is narrower than the prescription, deliberately, and the tests are what forced it: `folder` -> `folder` is exempt because 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. Rejecting it broke two existing reinstall tests 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. Same-source reinstalls stay allowed, so the upgrade path is untouched. The check runs inside the transaction and after crash recovery: before the lock it would race a concurrent install, and before recovery it would compare against a record whose bundle is about to be restored. Five tests, revert-verified against the check alone rather than against the missing exception class -- reverting the class makes the suite fail with AttributeError, which would pass for any rename. The handler test drives the real `install_from_dir`; a first version patched `materialize_power_bundle`, a name this handler never calls, so nothing was patched and the assertion read a 400 from the unpatched fetch. Round 38: the interrupted-uninstall RETRY branch uses the durable path too. Round 37 changed the main path to commit the record before destroying the bundle but left the recovery branch on the old ordering, and that branch is the worse place for it: it handles the state left by a process that died after the rename-aside, where `.removing-<name>` is the ONLY copy of the bytes. It deleted them and dropped the record afterwards, so a record replacement that failed -- a held-open `installed.json` on Windows again -- destroyed the bundle with nothing to restore from. The branch now splits on whether a record still claims the name. If it does, the tree is renamed back to its own name and execution falls through to the durable path, which commits while the bytes still exist. If it does not, the removal already committed and the leftover is swept and reported as already removed, since raising there reports a failure for a removal that succeeded. `test_record_survives_when_the_leftover_tree_cannot_be_removed` asserted the contract this replaces ("a failed delete keeps the record") and is re-expressed as `test_retrying_an_interrupted_uninstall_cannot_destroy_the_bundle`, which pins the property that actually matters: a failed retry loses nothing. It fails on revert with "the only copy of the bundle was destroyed". A second test covers the other half of the branch. Also round 38: hardlinked contract files are refused on install. The sensitive-path refusal is path-based, so a hardlink walked 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 -- the symlink guard compares `lstat` identity against the opened `fstat`, and a hardlink IS the file, so they agree and there is no link to detect. `st_nlink > 1` is now rejected, which is the rule `hooks.safe_read_file_bytes_nolink` already applies on the read side and the reason the state write uses `O_EXCL`. Two tests, the refusal one revert-verified; the second pins that an ordinary single-link bundle still installs, since the guard's risk is over-rejection. Round 39: every POWER.md read is capped, not just checked after the fact. `parse_power_md` has always refused over `MAX_POWER_MD_BYTES`, but it did so after `read_text()` had already pulled the whole file into the process. The size is not fixed at install time either: the installed bundle stays editable, and the install source is caller-owned before any copy budget applies. So anything able to write into a bundle could exhaust memory in the MCP process through `power_list`, and the guard that was supposed to stop that was downstream of the allocation. All three read sites now go through `_read_power_md`, which 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 asserted the REFUSAL, which is not new: all three passed with the fix reverted. They asserted the wrong thing because the observable behaviour is unchanged -- what changed is which read path runs. They now ban `Path.read_text` for POWER.md with a `RuntimeError` sentinel (not `OSError`/`PowerFormatError`, either of which `load_power` swallows and which would therefore be indistinguishable from the capped refusal), and three of the four fail on revert with "unbounded read of <path>". One test covers the ordinary-size case too, so a fix that only capped after a size check would not satisfy the suite. Also: main added bn/es/fr/hi/pt/ru locales since the last push, and `catalogParity` requires every catalogue to cover `en` exactly. `nav.powers` is added to all of them in English, and the Chinese guess from round 37 is reverted to match: Powers is the Kiro feature name, and `i18n-shard.mjs` reports keys identical to English as "expected for proper nouns/product names". A native speaker should still make the call per language. Round 40: the same cap for `mcp.json`. `_declares_mcp_servers` read the whole file, and it runs on `GET /api/powers` for every installed Power. Same reachability as the `POWER.md` case: the bundle stays editable after install, so the copy-time budget does not bound what is read later. A dedicated `MAX_MCP_JSON_BYTES` (256 KiB) rather than the 8 MiB whole-bundle budget, because this file is a server map, not a document. Over-cap returns "declares nothing" instead of raising. This function answers a yes/no question used to label a card in the listing, and install-time validation is what refuses malformed bundles -- raising here would let one oversized file break the whole listing. 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 when the file is already large. Round 41: a junctioned source `steering/` is refused, not followed. `Path.is_symlink()` is False for an NTFS directory junction, so the no-descriptor branch's check 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. Same class as the reparse-point guard already at the store root, now applied on the branch that depends on a path check; the POSIX branch pins the directory with `O_NOFOLLOW` and never reaches it. A symlinked `steering/` is now refused rather than silently skipped. Skipping looked safe and is exactly what let the junction through, and it would also install a partial bundle from a source tampered with mid-install. The tests drive `_copy_steering` directly, and the first version did not. Going through `install_from_dir` proved nothing twice over: the install path pre-validates the source and already refuses a *symlinked* `steering/` with its own error, which a loose `match="symlink"` accepted with this fix reverted, and the POSIX branch never reaches the path check anyway so the `_SUPPORTS_DIR_FD` patch did not select the branch under test. The junction itself is not constructible on Linux -- `is_symlink()` being False for one is the defect -- so one test simulates that shape and asserts the reparse check is consulted before the glob. Both fail on revert. Round 42: the POWER.md existence probe moves into the executor. `install_from_dir` stat'ed the source `POWER.md` on the event loop before handing the rest of validation to `maintenance_executor()`. A single 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. The probe, the tree walk and the parse are now one blocking callable, so there is no ordering left in which a caller-supplied path is touched from the loop. The regression test asserts the THREAD the probe runs on rather than latency: a timing test would be flaky and would not identify which call was at fault. It captures the loop's thread id in the coroutine, so any probe recorded on it is by definition blocking the gateway. It fails on revert. A second test pins that the caller-visible error for a missing POWER.md is unchanged. Also fixes one of my own round-39 tests, which failed on the Windows shards only: it pads `POWER.md` to just under the cap, and text-mode writes translate every newline to CRLF on Windows, inflating the file past the cap. The write is now byte-exact with `newline=""`.

…only) First slice of Powers activation, built on the pull model the Kiro IDE itself uses rather than the push model this repo's spec previously described. Reading `kiro-team/kiro-extension` (`src/extension/powers/**`) shows upstream neither merges a Power's MCP servers into agent configuration nor injects its guidance into agent context. It exposes tools — `listPowers`, `usePower(power, server, tool, args)`, `readPowerSteering(power, file)` — and the agent pulls what it needs. Following that shape deletes rather than solves the hardest parts of the previous plan: no rendered-agent-config rebuild (so the `includeMcpJson` constraint never applies), no `power-<power>-<server>` namespace and therefore no ownership-exact purge, no skill materialization, and no `trusted`/`enabled` fields — which means the `installed.json` written by the browse/install change needs no migration. This commit adds the two tools that never start a process: * `power_list` — installed Powers with each one's declared MCP server NAMES and steering files. It parses `mcp.json` for its top-level keys only; the `command`, `args` and `env` of a server are never read here, because those belong to the spawn path. A test asserts on the serialized payload that no argv leaks, so a future field cannot carry one through silently. * `power_steering` — one `steering/*.md` file, on demand, bounded at 128 KiB and passed through the mandatory redactor because a Power's guidance is third-party text on its way into a transcript. Neither raises a consent prompt, and the spec records why: both are reads of content the user chose to install, performed only when the agent names a Power, with nothing executed. Consent attaches to execution and arrives with `power_learn` / `power_use` in the next slice, together with the sandboxed spawn and the per-invocation audit. That split is the same boundary the browse/install change used, and it keeps the change that first executes third-party code reviewable on its own. Path handling treats the steering filename as untrusted rather than as a parameter the model got right: plain `.md` leaf only, no separators, no leading dot, resolved through the store's existing `_power_path` barrier so there is one implementation of that rule instead of a second weaker one, and a symlink at the target is refused because the tree stays writable by the same user after install vetting. Tests are revert-verified: defeating the path guard, the symlink refusal and the redaction fails 7 of the 20. An earlier version of the traversal cases matched any error and was vacuous — `../secret.md` reached the filesystem, missed, and raised "no such steering file", so it passed with the guard removed. They now pin the refusal reason and create the traversal target, so a case can only pass through the guard meant to stop it. Round 1 review fixes. Both reads are now bounded at the syscall, not filtered afterwards. The caps previously ran on the result of `read_bytes()`, so they limited what was returned while the whole file was already resident — a hostile bundle could still force an arbitrarily large read. `_read_bounded` reads at most `limit + 1` bytes through a descriptor opened `O_NOFOLLOW`, which also means a symlinked `mcp.json` reports no servers instead of being followed. This is the same lesson the browse/install path learned as `read_capped`. The steering DIRECTORY is confined as well as the leaf. Checking `is_symlink()` only on the file missed the redirectable component above it: a symlinked `steering/` sends every read inside it elsewhere, and the files found there are ordinary regular files that `O_NOFOLLOW` on the leaf has no objection to. Install pins its source directory for exactly this reason and the read path should have carried that across. The provider import moves to module scope inside a `try/except`, which satisfies `top-level-imports` while keeping the degradation the local form existed for: the failure is captured once at import and the handlers return a tool error instead of the server failing to load. Also fixes a Windows-only test failure: the steering fixture is written in text mode, which translates newlines, so an exact string comparison was asserting the platform's line ending rather than the tool's behaviour. Two of the three new tests were vacuous first. Counting bytes through `os.read` does not observe `Path.read_bytes`, which routes through `_io`, so the counter stayed at zero and both bounded-read tests passed with the fix reverted. They now make the unbounded call itself raise, so passing requires the bounded path. All three fail on revert. Registers `powers_tools.py` in the redaction-sink registry that main's drift guard requires. It is an output boundary even though the immediate reader is the agent rather than a human: `power_list` metadata and `power_steering` file contents are third-party text entering a transcript. The guard's dual-pass detector recognises wrapper functions by name, and `redact_external` / `redact_payload` were not among them, so this sink was reported as running a single scanner. It is not: `redact_external` runs `redact_credentials` and then `redact_exfiltration_urls` on the result, and `redact_payload` maps it over every string in a payload. The two wrappers are added to that list rather than the assertion being weakened, so a wrapper that stopped running both scanners would still fail the Powers redaction tests. Corrects the inert-install copy, which this change falsifies. The card and the InfoTip both promised that no guidance enters agent context — true of the browse/install change, false once `power_steering` can return a Power's markdown on request. A UI asserting a security property the code no longer has is worse than saying less, so the copy now states what remains true: nothing is registered, nothing loads automatically, and the agent can read guidance when it asks. A test pins the corrected claim and asserts the old one is gone; an existing assertion became case-sensitive-brittle when the sentence order changed and is now case-insensitive, since it is about the claim rather than its capitalisation. Round 3 review fixes. Steering and mcp.json reads now go through `hooks.safe_read_file_bytes_ nolink` instead of a bespoke bounded reader. `O_NOFOLLOW` guards only the FINAL path component, so a hardlink at the leaf or a nested directory swapped between check and open could still deliver another file's bytes. The shared gate opens first and `fstat`s the same inode, rejects `st_nlink > 1` and non-regular files, resolves the opened descriptor's real path inside the given root, refuses sensitive paths, and fails closed when it cannot verify. Existence is checked before the gate so "no such file" stays distinguishable from "refused" — the gate returns None for both, and telling an agent a missing file was a security refusal is misleading. Behaviour change: that helper raises on an oversize file rather than truncating, so `truncated` is gone and an over-cap steering file is refused. That is the better contract — guidance cut off mid-document and presented as the Power's guidance is worse than an explicit refusal. Tool error text is redacted before it is returned, and the whole payload passes through `redact_payload`. The message embeds the caller's own arguments, so a filename carrying a credential would have been persisted verbatim in the transcript by the very refusal meant to protect it. Two of the three new tests were vacuous first and are noted as such in their docstrings: the redaction test used a `.md` name that reached the "power is not installed" check (which names only the power) instead of the extension refusal that echoes the filename, and the previous bounded-read test patched `os.read`, which `Path.read_bytes` does not route through. All three fail on revert now.

github-actions Bot added the readiness: checking Automated validation is still running label Jul 28, 2026

github-actions Bot added readiness: action required A blocking check or review needs attention and removed readiness: checking Automated validation is still running labels Jul 28, 2026

kyleseaman force-pushed the feat/powers-activation branch from a13cd76 to 977ce93 Compare July 28, 2026 13:58

kyleseaman mentioned this pull request Jul 28, 2026

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

Closed

github-actions Bot added readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention labels Jul 28, 2026

github-actions Bot added readiness: action required A blocking check or review needs attention and removed readiness: checking Automated validation is still running labels Jul 28, 2026

kyleseaman force-pushed the feat/powers-activation branch from 977ce93 to 2c5cfe5 Compare July 28, 2026 16:07

kyleseaman force-pushed the feat/powers-activation branch from 2c5cfe5 to c9a4b78 Compare July 28, 2026 16:10

kyleseaman force-pushed the feat/powers-activation branch from c9a4b78 to f70110b Compare July 28, 2026 16:52

kyleseaman force-pushed the feat/powers-activation branch from f70110b to 05cbf10 Compare July 28, 2026 16:54

github-actions Bot removed the readiness: action required A blocking check or review needs attention label Jul 28, 2026

kyleseaman force-pushed the feat/powers-activation branch from 3fd86c8 to 977c892 Compare July 30, 2026 15:24

kyleseaman force-pushed the feat/powers-activation branch from 977c892 to e729e1a Compare July 30, 2026 16:03

kyleseaman force-pushed the feat/powers-activation branch from e729e1a to 2d7de2c Compare July 30, 2026 16:34

kyleseaman force-pushed the feat/powers-activation branch from 2d7de2c to e065c11 Compare July 30, 2026 16:58

Kyle Seaman added 2 commits July 30, 2026 17:33

kyleseaman force-pushed the feat/powers-activation branch from e065c11 to 5d645f5 Compare July 30, 2026 17:33

Navigation Menu

Uh oh!

feat(powers): agent-facing power_list and power_steering tools (read-only) - #652

feat(powers): agent-facing power_list and power_steering tools (read-only)#652
kyleseaman wants to merge 2 commits into
mainfrom
feat/powers-activation

kyleseaman commented Jul 28, 2026

Uh oh!

github-actions Bot commented Jul 28, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Jul 28, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Jul 28, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Jul 28, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Jul 30, 2026 •

edited

Loading

Uh oh!

kyleseaman commented Jul 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

kyleseaman commented Jul 28, 2026

What

Why this shape — the design changed

Scope: this slice never starts a process

Consent: deliberately none, here

Security controls

Verification

Next slice

Uh oh!

github-actions Bot commented Jul 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

GPT 5.6 Review — 🔴 changes requested (blocking)

Uh oh!

github-actions Bot commented Jul 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Arbiter — ⏳ review pending

Uh oh!

github-actions Bot commented Jul 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Design Review (Fable 5) — ✅ PASS

Suggestions

Uh oh!

github-actions Bot commented Jul 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Opus 5 Review — ✅ no blocking findings

Uh oh!

github-actions Bot commented Jul 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

UX Review (Fable 5) — 🟡 CONCERNS

Watch

Suggestions

Uh oh!

kyleseaman commented Jul 30, 2026

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

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

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

Not from this diff

Verification

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

github-actions Bot commented Jul 28, 2026 •

edited

Loading

github-actions Bot commented Jul 28, 2026 •

edited

Loading

github-actions Bot commented Jul 28, 2026 •

edited

Loading

github-actions Bot commented Jul 28, 2026 •

edited

Loading

github-actions Bot commented Jul 30, 2026 •

edited

Loading

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

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

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