This repository was archived by the owner on Jul 20, 2026. It is now read-only.
Conversation
Acknowledges main's squash commit as an ancestor to prevent phantom conflicts on the next dev→main promotion.
chore: back-merge main → dev
…egistry (#364) Three low-complexity board items in one commit: 1. GitHub webhook dedup — track recently-dispatched (owner/repo#number) tuples with a 60s window; skip duplicate issues.opened events. Root cause of Quinn's 4x comment on issue #359. 2. Suppress raw "Skill X completed by Y" echo — the outbound handler was posting the SkillResult stub as a GitHub comment alongside Quinn's own triage analysis. Now filters out single-line "completed by" stubs. 3. Populate blast-v1 registry from agent cards — same card-refresh pattern as hitl-mode-v1 (already shipped). Adds clearAgent() to BlastRegistry, parses capabilities.extensions[blast-v1].params.skills in SkillBrokerPlugin._loadBlastDeclarations(). x-blast-radius metadata stamping now actually fires for agents that opt in. Closes feature-1776307294155-3rqjqdmoi (webhook dedup). Closes feature-1776303349701-g7olxwk09 (blast registry). Co-authored-by: Automaker <automaker@localhost> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Two fixes discovered during end-to-end Quinn testing: 1. A2AExecutor._executeStream now breaks out of the SSE loop when it receives a non-terminal `task` event (submitted / working / input-required). Holding the stream open for agents that don't push incremental events crashes their SSE generator when the sync caller times out (observed in Quinn: GeneratorExit mid-yield, polluting logs with OpenTelemetry context-detach errors on every long task). TaskTracker.pollTask handles completion for these tasks cleanly. 2. SkillDispatcherPlugin picks the push-notification callback base URL based on whether the target agent is docker-internal (single-label hostname) or external (FQDN/IP). Internal agents get WORKSTACEAN_INTERNAL_BASE_URL (default http://workstacean:3000) which resolves on the shared docker network; external agents get WORKSTACEAN_BASE_URL (operator-configured public/Tailscale URL). Before this fix, `ava:8081` only resolved over Tailscale, so all docker-internal agents (Quinn, Jon, Researcher) silently failed to deliver push callbacks. Expose A2AExecutor.url as a public getter for the dispatcher's heuristic. Closes feature-1776309307266-pk9kwzc67 (streaming GeneratorExit). Closes feature-1776309298112-tltlsypow (push callback URL). Co-authored-by: Automaker <automaker@localhost> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
📝 WalkthroughWalkthroughThe changes include version bumps across packages and several feature additions to core components. GitHub plugin adds message filtering and auto-triage deduplication with a 60-second window; A2A executor exposes its configured URL and modifies streaming behavior to break early for non-terminal task states; BlastRegistry gains agent-specific cache clearing; callback routing now detects docker-internal versus external agents; and SkillBrokerPlugin integrates blast declaration loading into its card-discovery flow. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![protoquinn[bot]](https://avatars.githubusercontent.com/in/3282126?s=60&v=4){kind=small}
![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
🧹 Nitpick comments (2)
lib/plugins/github.ts (1)
556-574:recentDispatchesmap grows unbounded — consider periodic cleanup.Entries are added but never removed. Over long-running deployments, this will accumulate one entry per unique issue ever processed. A simple fix is to prune stale entries during the dedup check.
♻️ Proposed fix — prune stale entries inline
private _handleAutoTriage( event: string, payload: Record<string, unknown>, ctx: GitHubEventContext, autoTriage: AutoTriageConfig, bus: EventBus, getToken: (owner: string, repo: string) => Promise<string>, ): void { const dedupKey = `${ctx.owner}/${ctx.repo}#${ctx.number}`; + const now = Date.now(); + + // Prune stale entries to bound memory growth + for (const [key, ts] of this.recentDispatches) { + if (now - ts >= GitHubPlugin.DEDUP_WINDOW_MS) { + this.recentDispatches.delete(key); + } + } + const lastDispatched = this.recentDispatches.get(dedupKey); - if (lastDispatched && Date.now() - lastDispatched < GitHubPlugin.DEDUP_WINDOW_MS) { - console.log(`[github] Auto-triage: skipping duplicate ${dedupKey} (dispatched ${Date.now() - lastDispatched}ms ago)`); + if (lastDispatched && now - lastDispatched < GitHubPlugin.DEDUP_WINDOW_MS) { + console.log(`[github] Auto-triage: skipping duplicate ${dedupKey} (dispatched ${now - lastDispatched}ms ago)`); return; } - this.recentDispatches.set(dedupKey, Date.now()); + this.recentDispatches.set(dedupKey, now);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/plugins/github.ts` around lines 556 - 574, recentDispatches is never pruned so it will grow unbounded; inside _handleAutoTriage where dedupKey is computed and lastDispatched checked against GitHubPlugin.DEDUP_WINDOW_MS, add logic to remove stale map entries: iterate recentDispatches and delete any entries whose timestamp is older than DEDUP_WINDOW_MS (or a multiple like 2x) before setting the current dedupKey timestamp, ensuring the map is compacted periodically during dedupe checks.src/plugins/skill-broker-plugin.ts (1)
329-340: Avoid duplicating BlastRadius literals in Line 329 validation.The local
validRadiiset can drift fromBlastRadius/blast ordering definitions and silently reject newly added valid radii later. Derive allowed values fromblast.tsas a single source of truth.♻️ Proposed refactor
import { defaultBlastRegistry, BLAST_URI, + BLAST_ORDER, type BlastRadius, } from "../executor/extensions/blast.ts"; + +const VALID_BLAST_RADII = new Set<BlastRadius>( + Object.keys(BLAST_ORDER) as BlastRadius[], +); @@ - const validRadii = new Set(["self", "project", "repo", "fleet", "public"]); let declared = 0; @@ - if (typeof entry.radius !== "string" || !validRadii.has(entry.radius)) continue; + if ( + typeof entry.radius !== "string" || + !VALID_BLAST_RADII.has(entry.radius as BlastRadius) + ) continue;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/plugins/skill-broker-plugin.ts` around lines 329 - 340, The code currently hardcodes validRadii as a Set and risks drifting from the actual BlastRadius definitions; replace the local Set with the canonical list exported from blast.ts (e.g., derive allowed values from the BlastRadius enum or the exported radii array) and use that when validating entry.radius inside the loop that calls defaultBlastRegistry.declare (retain the same checks and the call to defaultBlastRegistry.declare with radius: entry.radius as BlastRadius). Ensure you import the correct symbol (BlastRadius or the exported radii list) from blast.ts and use it to build the validation Set so new radii added in blast.ts are accepted automatically.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/plugins/github.ts`:
- Line 331: The if-statement that tests content using the regex matching "Skill
... completed by ..." currently uses \w+ for the agent name and therefore
rejects names with hyphens; update that regex on the line containing the /^Skill
".+" completed by \w+$/i.test(content) check so the final token allows hyphens
(for example replace the \w+ token with a character class that permits word
characters and hyphens or with a non-space token) so agent names like
"proto-agent" are matched and the early return works correctly.
In `@src/executor/executors/a2a-executor.ts`:
- Around line 466-475: The early break in src/executor/executors/a2a-executor.ts
stops SSE consumption for any non-terminal taskState, which can drop handling
when the first state is "input-required"; update the logic so "input-required"
is treated as non-terminal that must keep the stream open (i.e., do not break
when taskState === "input-required"), and align this change with the dispatcher
by adding "input-required" to the tracked states in
src/executor/skill-dispatcher-plugin.ts (the arrays/conditions around the
existing "submitted"/"working" checks). Ensure the SSE break condition only
triggers for terminal states ("completed","failed","canceled","rejected") or
when the dispatcher explicitly tracks the non-terminal state.
---
Nitpick comments:
In `@lib/plugins/github.ts`:
- Around line 556-574: recentDispatches is never pruned so it will grow
unbounded; inside _handleAutoTriage where dedupKey is computed and
lastDispatched checked against GitHubPlugin.DEDUP_WINDOW_MS, add logic to remove
stale map entries: iterate recentDispatches and delete any entries whose
timestamp is older than DEDUP_WINDOW_MS (or a multiple like 2x) before setting
the current dedupKey timestamp, ensuring the map is compacted periodically
during dedupe checks.
In `@src/plugins/skill-broker-plugin.ts`:
- Around line 329-340: The code currently hardcodes validRadii as a Set and
risks drifting from the actual BlastRadius definitions; replace the local Set
with the canonical list exported from blast.ts (e.g., derive allowed values from
the BlastRadius enum or the exported radii array) and use that when validating
entry.radius inside the loop that calls defaultBlastRegistry.declare (retain the
same checks and the call to defaultBlastRegistry.declare with radius:
entry.radius as BlastRadius). Ensure you import the correct symbol (BlastRadius
or the exported radii list) from blast.ts and use it to build the validation Set
so new radii added in blast.ts are accepted automatically.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c4819fc9-4327-4bf1-bf6b-9dce7122c787
📒 Files selected for processing (7)
dashboard/package.jsonlib/plugins/github.tspackage.jsonsrc/executor/executors/a2a-executor.tssrc/executor/extensions/blast.tssrc/executor/skill-dispatcher-plugin.tssrc/plugins/skill-broker-plugin.ts
|
|
||
| const content = String((msg.payload as Record<string, unknown>).content ?? "").trim(); | ||
| if (!content) return; | ||
| if (/^Skill ".+" completed by \w+$/i.test(content)) return; |
There was a problem hiding this comment.
Regex won't match agent names containing hyphens.
The pattern \w+ only matches [a-zA-Z0-9_]. If this.config.name contains a hyphen (e.g., "proto-agent"), the regex fails and the "echo" comment is posted anyway.
🐛 Proposed fix
- if (/^Skill ".+" completed by \w+$/i.test(content)) return;
+ if (/^Skill ".+" completed by [\w-]+$/i.test(content)) return;📝 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.
Suggested change
| if (/^Skill ".+" completed by \w+$/i.test(content)) return; | |
| if (/^Skill ".+" completed by [\w-]+$/i.test(content)) return; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/plugins/github.ts` at line 331, The if-statement that tests content using
the regex matching "Skill ... completed by ..." currently uses \w+ for the agent
name and therefore rejects names with hyphens; update that regex on the line
containing the /^Skill ".+" completed by \w+$/i.test(content) check so the final
token allows hyphens (for example replace the \w+ token with a character class
that permits word characters and hyphens or with a non-space token) so agent
names like "proto-agent" are matched and the early return works correctly.
Comment on lines
+466
to
+475
| // Non-terminal state (submitted / working / input-required) on the | ||
| // initial task event → stop consuming the SSE stream and let | ||
| // TaskTracker drive the task to completion via tasks/get polling. | ||
| // Holding the stream open longer forces slow agents to keep their | ||
| // SSE generator alive for minutes, which crashes them when the | ||
| // sync caller eventually times out and closes the connection. | ||
| if (taskState !== "completed" && taskState !== "failed" | ||
| && taskState !== "canceled" && taskState !== "rejected") { | ||
| break; | ||
| } |
There was a problem hiding this comment.
Non-terminal early break can drop input-required task handling.
At Line [466]-[475], the stream now breaks on all non-terminal task states. But the dispatcher currently tracks only submitted/working (src/executor/skill-dispatcher-plugin.ts, Line [28] and Line [319]). If the first state is input-required (or another non-terminal), it can bypass tracking/polling and be treated as completed too early.
Suggested alignment fix (dispatcher side)
-const WORKING_STATES = new Set(["submitted", "working"]);
+const NON_TERMINAL_STATES = new Set(["submitted", "working", "input-required"]);
...
- && WORKING_STATES.has(taskState)
+ && NON_TERMINAL_STATES.has(taskState)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/executor/executors/a2a-executor.ts` around lines 466 - 475, The early
break in src/executor/executors/a2a-executor.ts stops SSE consumption for any
non-terminal taskState, which can drop handling when the first state is
"input-required"; update the logic so "input-required" is treated as
non-terminal that must keep the stream open (i.e., do not break when taskState
=== "input-required"), and align this change with the dispatcher by adding
"input-required" to the tracked states in
src/executor/skill-dispatcher-plugin.ts (the arrays/conditions around the
existing "submitted"/"working" checks). Ensure the SSE break condition only
triggers for terminal states ("completed","failed","canceled","rejected") or
when the dispatcher explicitly tracks the non-terminal state.
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Summary
Test plan
Summary by CodeRabbit
Bug Fixes
Chores