Kiro ACP sessions run StudyLoop's own studyloop agent - #48
Merged
Merged
Conversation
… Kiro agent Web ACP sessions launch `kiro-cli acp` with no `--agent`, so each one runs under whatever the learner's default Kiro agent is. Until today that was Kiro's built-in default (its own system prompt, the learner's personal steering and skills, every server in the global mcp.json), because kiro-cli 2.24.0 reserves `kiro_default` and ignored the learner's kiro_default.json. Renaming that file made the learner's personal agent the default -- and so, silently, the mentor's configuration. Owner steer (2026-09-26): the test for a valid StudyLoop agent is done with a dedicated agent named `studyloop`, created at install time and used as required. These tests pin that agent (persona-neutral, StudyLoop's two MCP servers only, the persona agents' hook and denylist, no pre-approvals -- the ACP path had none under the built-in default and decision 2 widens no grant), its install link, manifest entry and nightly check, the ACP argv, and a doctor check proving kiro-cli validates and lists it. 17 fail for the stated reasons; the grok argv guard passes. The two pyright ignores on the not-yet-defined doctor check are RED-only and go at GREEN.
`studyloop install agents` now links agents/kiro/studyloop.json to ~/.kiro/agents/studyloop.json, and every Kiro ACP launch is `kiro-cli acp --agent studyloop`, so the learner's default Kiro agent -- whatever prompt, steering, skills or MCP servers it loads -- no longer configures a StudyLoop session (owner steer, 2026-09-26). The agent is persona-neutral: the persona is ACP's invisible first turn, and a second one here would compete with it. It reaches only StudyLoop's `studyloop` and `session-db` servers (entries copied from study-mentor.json) and carries the persona agents' session-export stop hook and execute_bash denylist verbatim. `allowedTools` is empty: under Kiro's built-in default the ACP path pre-approved nothing (read from `kiro-cli agent create --from kiro_default`), and decision 2 (#34) widens no grant without the owner. `studyloop doctor` gains agent_kiro_studyloop_loads, which asks kiro-cli itself: `agent validate --path` must accept the file, and `agent list` must list `studyloop` with no built-in of the same name shadowing it -- the way 2.24.0's reserved `kiro_default` silently ignored a learner's own file. A missing install warns with fix_auto, so `doctor --fix` reinstalls it. The manifest entry was added in place (every other entry's date unchanged) and the regenerator tracks it; the nightly install job checks it landed; the live Kiro ACP test skips with the fix when the agent is not installed. The install contract's definition set covers it. Two RED-only pyright ignores removed; one RED assertion named CheckResult.fix, the field is fix_hint. .secrets.baseline refreshed by a whole-repo scan with the hook's own detect-secrets 1.5.0: 72 -> 72 result files, the one change is the new manifest hash plus the four-line shift of the entries below it. Touched suites 231/231; full suite 7377 passed, failing ids minus the committed environmental set = the known agent-session-tools sqlite3 timeout.
The install guide's Kiro section names the new `studyloop` agent: what it carries, that every Kiro ACP session starts with it, and that doctor checks kiro-cli can load it. Troubleshooting gains the `invalid agent config: kiro_default.json` notice the owner hit when starting "Plan with Architect": kiro-cli 2.24.0 reserves the name, StudyLoop neither reads nor writes the file, and the rename that keeps a customisation. CHANGELOG [Unreleased].
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Doctor auto-fix and agent discovery need correction, and the troubleshooting recipe should prevent overwrites.
Review effort: Lite
Findings: 2
Open (2)
What changed in this PR
Adds a dedicated studyloop Kiro agent for ACP sessions, including installation, validation, documentation, and launch wiring.
Changes:
- Adds and installs
agents/kiro/studyloop.json. - Launches Kiro ACP with
--agent studyloop. - Adds doctor checks, manifest/workflow coverage, tests, and documentation.
Outstanding issues include unreliable fresh-home auto-fix, unchecked kiro-cli agent list failures, and a troubleshooting recipe that may overwrite files.
| File | Description |
|---|---|
scripts/update-agent-manifest.py |
Updates manifest generation. |
packages/studyloop/tests/test_web_acp_dogfood_kiro.py |
Updates Kiro ACP expectations. |
packages/studyloop/tests/test_kiro_studyloop_agent.py |
Tests agent, installation, launch, and doctor behavior. |
packages/studyloop/tests/test_install_agent_contracts.py |
Covers installer contracts. |
packages/studyloop/src/studyloop/web/routes/session/_transport.py |
Passes the dedicated agent to ACP. |
packages/studyloop/src/studyloop/installers.py |
Installs the agent link. |
packages/studyloop/src/studyloop/doctor/agents.py |
Validates installation and discovery. |
packages/studyloop/src/studyloop/cli/_doctor.py |
Registers the doctor check. |
packages/studyloop/src/studyloop/adapters/kiro.py |
Defines the ACP agent name. |
docs/troubleshooting.md |
Documents Kiro configuration issues. |
docs/agent-install.md |
Documents agent installation. |
CHANGELOG.md |
Records the behavior change. |
agents/manifest.json |
Tracks the agent definition hash. |
agents/kiro/studyloop.json |
Defines the dedicated Kiro agent. |
.secrets.baseline |
Updates scan metadata. |
.github/workflows/nightly-install.yml |
Verifies installation nightly. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| " -- web (ACP) Kiro sessions need it" | ||
| ), | ||
| "studyloop install agents --tool kiro", | ||
| fix_auto=True, |
Comment on lines
+247
to
+251
| listed = subprocess.run( | ||
| [binary, "agent", "list"], | ||
| capture_output=True, | ||
| text=True, | ||
| timeout=_KIRO_AGENT_CHECK_TIMEOUT, |
…roduce The first check was written against output I assumed, and its fixtures assumed the same things, so the tests passed and the check was wrong. Probed against real kiro-cli 2.24.0 on 2026-09-26: 1. `agent list` writes its whole table to stderr; stdout is empty, so the check reported "does not list" on every machine. 2. `agent validate` exits 0 on an invalid file (a wrong-typed field, or a truncated JSON body); the verdict is an `Error:` line on stderr, so the check never flagged a bad file. 3. `acp --agent <name>` with no such agent does not fail: the session silently runs the built-in `kiro_default`. So the check is the only thing that notices, and its message should say that. Fixtures are now the captured output (paths redacted): ANSI colouring, the `Workspace:`/`Global:` header, a leading `Error:` line for another agent's missing prompt URI, and wrapped description lines. 4 fail for those reasons; 16 pass, including the new description-line-is-not-a-row guard.
…them GREEN for 97bc957. `agent validate`'s verdict is its `Error:` line (the exit code stays 0 on an invalid file, so both now count), and `agent list` is parsed from stderr, where 2.24.0 writes the table (stdout too, in case a release moves it). Rows are matched at column 2 after `" "` or the default marker, so a wrapped description line that mentions `studyloop` is never taken for the agent. The not-listed message now says what that costs: web Kiro sessions silently fall back to kiro-cli's built-in default. Checked against the real kiro-cli 2.24.0, not only the fixtures: a valid, discovered file passes; a wrong-typed file warns with kiro-cli's own reason; a valid file kiro-cli does not discover warns with the fallback. One RED assertion expected "falls back"; the message's grammar is "fall back", so the assertion now pins the full phrase.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Web (ACP) Kiro sessions started
kiro-cli acpwith no--agent, so each one ran under whatever the learner's default Kiro agent was. On the owner's machine that was Kiro's built-in default until 2026-09-26: its own system prompt, the learner's personal steering and skills, and all seven MCP servers in the globalmcp.json. That happened because kiro-cli 2.24.0 reserveskiro_defaultand ignored~/.kiro/agents/kiro_default.json. Renaming that file made the learner's personal agent the default, and so the mentor's configuration. A live ACP handshake confirms it: with no--agent,session/newreportscurrentModeId: default-plus.Owner steer: the test for ensuring a valid agent for StudyLoop should be done with a new dedicated agent called studyloop which is created at install time and used as required.
agents/kiro/studyloop.json: persona-neutral, because the persona is ACP's first turn and is self-contained (build_canonical_personareads onlyagents/shared/personas/<mode>.md, which names no skill). Its only servers arestudyloopandsession-db, with entries copied fromstudy-mentor.json, and it carries the persona agents' session-export stop hook andexecute_bashdenylist verbatim.allowedTools: []: under the built-in default the ACP path pre-approved nothing (read withkiro-cli agent create --from kiro_default), and decision 2 (Learning-tier plan (jev-judge): four owner decisions before S1-0 starts #34) widens no grant without the owner._TOOL_LINKS["kiro"]links it to~/.kiro/agents/studyloop.json. The manifest tracks it (entry added in place, every other date unchanged), and the nightly install job checks it landed.["kiro-cli", "acp", "--agent", "studyloop"], and the live handshake reportscurrentModeId: studyloop. Grok's argv is unchanged, and a guard test pins that.studyloop doctorgainsagent_kiro_studyloop_loads, which asks kiro-cli directly.agent validate --pathmust not report anError:, andagent listmust liststudyloopas a row with no built-in of the same name shadowing it. A missing install warns withfix_auto, sodoctor --fixreinstalls it.invalid agent config: kiro_default.jsonnotice, and CHANGELOG.What the probes against real kiro-cli 2.24.0 found
The first version of the doctor check (
711459c1) parsed output kiro-cli does not produce, and its fixtures assumed the same things, so its tests passed. Fixed in a second RED/GREEN (97bc957fthen710473a9), with fixtures captured from real output:agent listwrites its whole table to stderr; stdout is empty.agent validateexits 0 on an invalid file, whether a field has the wrong type or the JSON is truncated. The verdict is anError:line on stderr.acp --agent <name>naming an agent that doesn't exist does not fail: the session silently runs the built-inkiro_default. So this doctor check is the only thing that notices an uninstalled or unloadable agent, and its message says so.The rebuilt check was then run against the real kiro-cli in three states: a valid, discovered file passes; a wrong-typed file warns with kiro-cli's own reason; a valid file kiro-cli doesn't discover warns with the fallback.
Tested
05b428cd: 17 fail for the stated reasons. GREEN711459c1. RED97bc957f: 4 fail on the three real-output facts. GREEN710473a9.mkdocs build --strictclean; ruff, format and pyright clean.2ee95b30: 7377 passed. Failing ids minus the committed environmental set (full-suite-control-item4-2026-09-18.md) leave one: the pre-existingagent-session-toolssqlite3 timeout (Housekeeping: skill guide published, jev plan on main, openspec tree un-ignored and archived, one-clock fix #35). The two later commits touch only the doctor check and its test file, both inside the touched suites above..secrets.baseline: a whole-repo scan with the hook's own detect-secrets 1.5.0 kept 72 result files. The only change is the new manifest hash, plus a four-line shift in the entries below it.Upgrade note
Existing installs need
studyloop install agents --tool kiro(orstudyloop doctor --fix) once. Until then, Kiro ACP sessions silently run kiro-cli's built-in default (finding 3), andstudyloop doctorwarns.