Wire agent runner into product: nudge endpoint, activity feed, agent catalog#21
Merged
Conversation
…ent catalog
Make the Tier 2/3 agent-runner capabilities reachable end-to-end.
1) Steering over HTTP. Thread onSteer through createRoles -> RunnerActor/
RunnerCritic, which pass it as req.steering on every runner call so a
runner's live steer handle bubbles up. session forwards it (via the
existing RunGoalOptions/CreateRolesOptions spread); the server keeps a
per-session latest steer handle and adds POST /api/runs/:id/nudge to
inject guidance into the in-flight agent (202), with 400 (no text) /
409 (nothing steerable) and the same token guard as the rest of /api.
The handle is cleared on settle.
2) Desktop live tool feed. The run view renders runner_activity events
(which arrive via the existing store-event SSE channel) as a concise
per-tool feed in the engine log, so you can watch the actor read/edit
files in real time.
3) Desktop native-agent catalog. settings.ts gains kind "agent"
providers (Anthropic/OpenAI/Google) that emit { kind:"agent",
options:{ provider, apiKeyEnv } } runner profiles with pi-ai model
ids; they edit files directly. main.ts note/label logic updated so
agent providers show their API-key affordance and "edits files"
note correctly.
4) Desktop nudge control. api.nudgeRun + a Nudge input/button in the run
view post to the new endpoint (Enter to send), surfacing 409 when the
active backend isn't steerable.
Tests: server nudge (inject / 400 / 409 / 401) and the role-level steer
registration. Engine suite 201 green; engine + desktop typecheck clean.
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds live "nudge" steering for in-flight runs: a new ChangesLive Steering / Nudge Feature
Agent Provider Kind
Sequence Diagram(s)sequenceDiagram
participant User as User (Desktop UI)
participant nudgeRun as nudgeRun() API Client
participant Server as POST /api/runs/:id/nudge
participant steerers as steerers map
participant RunnerActor as RunnerActor (runner.run)
User->>nudgeRun: sendNudge(sessionId, text)
nudgeRun->>Server: POST /api/runs/{sessionId}/nudge { text }
Server->>Server: validate text non-empty
Server->>steerers: lookup steer fn for sessionId
alt no steer registered
Server-->>nudgeRun: 409 Conflict
nudgeRun-->>User: show error in plan area
else steer registered
Server->>RunnerActor: steer(text)
Server-->>nudgeRun: 202 Accepted
nudgeRun-->>User: clear input, log "you → nudge"
end
Note over Server,steerers: on run settle(), steerers.delete(sessionId)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@desktop/src/main.ts`:
- Around line 707-711: The nudgeInput element in the h("input", ...) creation
lacks an explicit accessible name for screen readers. Add an aria-label
attribute to the input element with a descriptive name like "Nudge the agent" to
provide a stable accessible name that screen readers can reliably announce,
independent of the placeholder text.
- Around line 735-752: The sendNudge function can be invoked multiple times
concurrently from rapid Enter key presses or button clicks while a request is
still in flight, causing duplicate nudges. Add a flag (e.g., isNudgeSending or
isSending) to track whether sendNudge is currently executing. At the start of
the sendNudge function, check if this flag is true and return early if it is.
Set the flag to true when entering sendNudge and set it back to false in the
finally block to ensure it is reset after the request completes. This prevents
re-entrancy and ensures only one nudge request can be in flight at a time.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a2890db8-488c-4aee-98a5-ce994240c09a
📒 Files selected for processing (8)
desktop/src/api.tsdesktop/src/main.tsdesktop/src/settings.tssrc/adapters/roleBindings.tssrc/adapters/runnerRoles.tssrc/server/server.tstest/runnerRoles.test.tstest/server.test.ts
…us-agents.kiro.dev/inhaq/loopwright into pi-agent-ui-integration # Conflicts: # desktop/src/main.ts # desktop/src/settings.ts
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
desktop/src/main.ts (1)
355-373:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winCLI availability falls back to “missing” when command detection is unknown.
If
detectCommands()fails,installedstays{}; thencommandMissing()returnstruefor every CLI preset. That contradicts the intended “unknown — don’t flag missing” behavior and produces false warning states.Suggested fix
let storedKeys = new Set<string>(); let knowKeys = false; let installed: Record<string, boolean> = {}; + let knowInstalled = false; if (isTauri()) { @@ try { installed = await detectCommands(["codex", "kiro-cli", "gh"]); + knowInstalled = true; } catch { /* unknown — don't flag CLI presets as missing */ } } /** Whether a preset's command is known-missing (desktop only). */ function commandMissing(p: RunnerPreset): boolean { - return p.kind === "cli" && isTauri() && installed[p.command ?? ""] !== true; + return p.kind === "cli" && isTauri() && knowInstalled && installed[p.command ?? ""] !== true; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@desktop/src/main.ts` around lines 355 - 373, When detectCommands() fails, the installed object remains as an empty object {}, causing commandMissing() to incorrectly return true for all CLI presets because installed[p.command ?? ""] evaluates to undefined !== true. To fix this, introduce a boolean flag (such as commandsDetected) that tracks whether detectCommands() completed successfully, setting it to true only when the try block succeeds. Then modify the commandMissing() function to return true only when both commandsDetected is true AND installed[p.command ?? ""] !== true, ensuring that CLI presets are not flagged as missing when command detection is unknown.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@desktop/src/main.ts`:
- Around line 1421-1423: The env resolution logic in the PRESETS loop only
handles "http-responses" kind presets via apiKeyEnv, while "agent" presets
incorrectly fall through to requiresEnv (which is undefined for agents), causing
them to be skipped by the continue statement. Update the conditional expression
that determines which environment variable property to check (currently checking
preset.kind === "http-responses" ? preset.apiKeyEnv : preset.requiresEnv) to
include proper handling for "agent" presets by adding them to the condition with
their appropriate environment variable property.
In `@desktop/src/settings.ts`:
- Around line 268-271: The legacy migration logic in the settings.ts file is
performing an identity lookup instead of translating provider ids to preset ids.
In the ternary expression that sets presetId, replace the direct use of
v.provider with a proper provider-to-preset translation. The current code uses
v.provider directly as the presetId, but legacy { provider, model } payloads
contain old provider names that won't match the new preset id format. Implement
a mapping or translation function to convert the legacy provider value to the
corresponding new preset id before passing it to getPreset().
---
Outside diff comments:
In `@desktop/src/main.ts`:
- Around line 355-373: When detectCommands() fails, the installed object remains
as an empty object {}, causing commandMissing() to incorrectly return true for
all CLI presets because installed[p.command ?? ""] evaluates to undefined !==
true. To fix this, introduce a boolean flag (such as commandsDetected) that
tracks whether detectCommands() completed successfully, setting it to true only
when the try block succeeds. Then modify the commandMissing() function to return
true only when both commandsDetected is true AND installed[p.command ?? ""] !==
true, ensuring that CLI presets are not flagged as missing when command
detection is unknown.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 427091f8-8417-4073-a88f-7c5cad800082
📒 Files selected for processing (3)
desktop/src/main.tsdesktop/src/settings.tssrc/server/server.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/server/server.ts
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
desktop/src/main.ts (1)
355-373:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winCLI availability falls back to “missing” when command detection is unknown.
If
detectCommands()fails,installedstays{}; thencommandMissing()returnstruefor every CLI preset. That contradicts the intended “unknown — don’t flag missing” behavior and produces false warning states.Suggested fix
let storedKeys = new Set<string>(); let knowKeys = false; let installed: Record<string, boolean> = {}; + let knowInstalled = false; if (isTauri()) { @@ try { installed = await detectCommands(["codex", "kiro-cli", "gh"]); + knowInstalled = true; } catch { /* unknown — don't flag CLI presets as missing */ } } /** Whether a preset's command is known-missing (desktop only). */ function commandMissing(p: RunnerPreset): boolean { - return p.kind === "cli" && isTauri() && installed[p.command ?? ""] !== true; + return p.kind === "cli" && isTauri() && knowInstalled && installed[p.command ?? ""] !== true; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@desktop/src/main.ts` around lines 355 - 373, When detectCommands() fails, the installed object remains as an empty object {}, causing commandMissing() to incorrectly return true for all CLI presets because installed[p.command ?? ""] evaluates to undefined !== true. To fix this, introduce a boolean flag (such as commandsDetected) that tracks whether detectCommands() completed successfully, setting it to true only when the try block succeeds. Then modify the commandMissing() function to return true only when both commandsDetected is true AND installed[p.command ?? ""] !== true, ensuring that CLI presets are not flagged as missing when command detection is unknown.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@desktop/src/main.ts`:
- Around line 1421-1423: The env resolution logic in the PRESETS loop only
handles "http-responses" kind presets via apiKeyEnv, while "agent" presets
incorrectly fall through to requiresEnv (which is undefined for agents), causing
them to be skipped by the continue statement. Update the conditional expression
that determines which environment variable property to check (currently checking
preset.kind === "http-responses" ? preset.apiKeyEnv : preset.requiresEnv) to
include proper handling for "agent" presets by adding them to the condition with
their appropriate environment variable property.
In `@desktop/src/settings.ts`:
- Around line 268-271: The legacy migration logic in the settings.ts file is
performing an identity lookup instead of translating provider ids to preset ids.
In the ternary expression that sets presetId, replace the direct use of
v.provider with a proper provider-to-preset translation. The current code uses
v.provider directly as the presetId, but legacy { provider, model } payloads
contain old provider names that won't match the new preset id format. Implement
a mapping or translation function to convert the legacy provider value to the
corresponding new preset id before passing it to getPreset().
---
Outside diff comments:
In `@desktop/src/main.ts`:
- Around line 355-373: When detectCommands() fails, the installed object remains
as an empty object {}, causing commandMissing() to incorrectly return true for
all CLI presets because installed[p.command ?? ""] evaluates to undefined !==
true. To fix this, introduce a boolean flag (such as commandsDetected) that
tracks whether detectCommands() completed successfully, setting it to true only
when the try block succeeds. Then modify the commandMissing() function to return
true only when both commandsDetected is true AND installed[p.command ?? ""] !==
true, ensuring that CLI presets are not flagged as missing when command
detection is unknown.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 427091f8-8417-4073-a88f-7c5cad800082
📒 Files selected for processing (3)
desktop/src/main.tsdesktop/src/settings.tssrc/server/server.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/server/server.ts
🛑 Comments failed to post (2)
desktop/src/main.ts (1)
1421-1423:
⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAgent presets are skipped in “Presets by environment key”.
At Line 1422, env resolution handles only
http-responsesviaapiKeyEnv;"agent"presets fall through torequiresEnv(undefined) and are excluded from the catalog.Suggested fix
- const env = preset.kind === "http-responses" ? preset.apiKeyEnv : preset.requiresEnv; + const env = preset.kind === "cli" ? preset.requiresEnv : preset.apiKeyEnv;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@desktop/src/main.ts` around lines 1421 - 1423, The env resolution logic in the PRESETS loop only handles "http-responses" kind presets via apiKeyEnv, while "agent" presets incorrectly fall through to requiresEnv (which is undefined for agents), causing them to be skipped by the continue statement. Update the conditional expression that determines which environment variable property to check (currently checking preset.kind === "http-responses" ? preset.apiKeyEnv : preset.requiresEnv) to include proper handling for "agent" presets by adding them to the condition with their appropriate environment variable property.desktop/src/settings.ts (1)
268-271:
⚠️ Potential issue | 🟠 Major | ⚡ Quick winLegacy migration is doing identity lookup instead of provider→preset translation.
At Line 269,
v.provideris used directly aspresetId. For legacy{ provider, model }payloads from the old provider/model shape, provider ids won’t reliably match new preset ids, so migrated writer/reviewer choices can silently fall back to defaults.Suggested fix
function coerceChoice(value: unknown, fallback: ModelChoice): ModelChoice { if (value === null || typeof value !== "object") return { ...fallback }; const v = value as Record<string, unknown>; - // Tolerate the legacy { provider, model } shape by mapping provider->preset. - const presetId = typeof v.preset === "string" ? v.preset : typeof v.provider === "string" ? v.provider : ""; + // Tolerate the legacy { provider, model } shape by mapping provider->preset. + const legacyProvider = typeof v.provider === "string" ? v.provider : ""; + const providerToPreset: Record<string, string> = { + // Keep this in sync with actual v1 provider ids. + openai: "openai-responses", + kiro: "kiro-cli", + codex: "codex-cli", + anthropic: "anthropic-agent", + google: "google-agent", + }; + const presetId = + typeof v.preset === "string" + ? v.preset + : providerToPreset[legacyProvider] ?? legacyProvider; const preset = getPreset(presetId); if (!preset) return { ...fallback }; const model = typeof v.model === "string" ? v.model : ""; return { preset: preset.id, model }; }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.function coerceChoice(value: unknown, fallback: ModelChoice): ModelChoice { if (value === null || typeof value !== "object") return { ...fallback }; const v = value as Record<string, unknown>; // Tolerate the legacy { provider, model } shape by mapping provider->preset. const legacyProvider = typeof v.provider === "string" ? v.provider : ""; const providerToPreset: Record<string, string> = { // Keep this in sync with actual v1 provider ids. openai: "openai-responses", kiro: "kiro-cli", codex: "codex-cli", anthropic: "anthropic-agent", google: "google-agent", }; const presetId = typeof v.preset === "string" ? v.preset : providerToPreset[legacyProvider] ?? legacyProvider; const preset = getPreset(presetId); if (!preset) return { ...fallback }; const model = typeof v.model === "string" ? v.model : ""; return { preset: preset.id, model }; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@desktop/src/settings.ts` around lines 268 - 271, The legacy migration logic in the settings.ts file is performing an identity lookup instead of translating provider ids to preset ids. In the ternary expression that sets presetId, replace the direct use of v.provider with a proper provider-to-preset translation. The current code uses v.provider directly as the presetId, but legacy { provider, model } payloads contain old provider names that won't match the new preset id format. Implement a mapping or translation function to convert the legacy provider value to the corresponding new preset id before passing it to getPreset().
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
1 participant
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.
This pull request was created by @kiro-agent on behalf of @inhaq 👻
Comment with /kiro fix to address specific feedback or /kiro all to address everything.
Learn about Kiro Web
The three optional follow-ons from the deep-dive, making the Tier 2/3 agent-runner capabilities reachable end-to-end. Stacked on #20 (base =
pi-agent-tier3); retarget down the stack as the parents merge.1. Steering over HTTP (server
/nudge+ threading)onSteerthroughcreateRoles→RunnerActor/RunnerCritic, which pass it asreq.steeringon every runner call, so a runner's livesteer(text)handle bubbles up.sessionforwards it via the existing options spread.POST /api/runs/:id/nudge({text}) to inject guidance into the in-flight agent:202on success,400(no text),409(nothing steerable — e.g. a CLI/HTTP backend), behind the same bearer-token guard as the rest of/api. The handle is cleared onsettle.2. Desktop live tool feed (
runner_activity)runner_activityevents (which arrive over the existing store-event SSE channel) as a concise per-tool feed in the engine log — you can watch the actor read/edit files in real time.3. Desktop native-agent catalog (
kind: "agent")settings.tsgains agent providers (Anthropic / OpenAI / Google) that emit{ kind: "agent", options: { provider, apiKeyEnv } }runner profiles with valid pi-ai model ids; they edit files directly.main.tsnote/label logic updated so agent providers show their API-key affordance and the "edits files directly" note correctly (previously onlyhttp/cliwere handled).4. Desktop nudge control
api.nudgeRun+ a Nudge input/button in the run view (Enter to send) post to the new endpoint, surfacing409when the active backend isn't steerable.Testing
400no-text,409non-steerable/unknown,401no-token.RunnerActorregisters the runner's live steer handle viaonSteer.tsc --noEmitclean; desktoptsc --noEmitclean.Notes
eventtype (data.type = "runner_activity"), so no server transport change was needed.Summary by CodeRabbit
New Features
Improvements