feat(agents): add provider-neutral orchestration - #5632
Open
megascan wants to merge 13 commits into
Open
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughNative agent orchestration is added across contracts, catalog and rule storage, prompt compilation, durable runs, MCP and WebSocket APIs, provider compatibility, web and mobile settings, chat selection, thread state, persistence, and documentation. ChangesNative agent platform
Estimated code review effort: 5 (Critical) | ~120 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
vouch:unvouched
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Reviewed the new Effect services under apps/server/src/agents/**, their MCP/WS call sites, and the layer wiring. Findings are limited to Effect service conventions in the new code: standalone *Shape interfaces instead of inline Context.Service interfaces, error classes whose only payload is a free-form detail (with the underlying failure discarded rather than kept as cause), and a duplicated agent-services layer in the WebSocket route.
Posted via Macroscope — Effect Service Conventions
megascan
force-pushed
the
agent/native-agent-orchestration
branch
from
August 7, 2026 18:53
86d41ba to
091b57d
Compare
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 19
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (14)
apps/server/src/orchestration/Layers/ProjectionPipeline.ts-813-816 (1)
813-816: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUpdate the thread projection timestamp.
Line 815 changes
agentProfilebut leavesupdatedAtunchanged. Clients can miss this state transition when they order or reconcile threads byupdatedAt, especially if no later session event is emitted. SetupdatedAt: event.occurredAtin this upsert.🤖 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 `@apps/server/src/orchestration/Layers/ProjectionPipeline.ts` around lines 813 - 816, Update the projectionThreadRepository.upsert call in the projection event handler to set updatedAt to event.occurredAt alongside agentProfile, preserving the existing row fields and ensuring the thread timestamp reflects this state transition.docs/internals/glossary.md-95-97 (1)
95-97: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUse standard adverb placement.
Change “applies always” to “always applies.”
🤖 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 `@docs/internals/glossary.md` around lines 95 - 97, Update the glossary’s Rule definition to change the wording from “applies always” to “always applies,” preserving the rest of the definition and its reference unchanged.Source: Linters/SAST tools
apps/web/src/components/settings/AgentsSettings.logic.ts-128-132 (1)
128-132: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReject empty and blank input in
parseInteger.
Number("")andNumber(" ")return0, andNumber.isInteger(0)is true. If the user clears "Maximum runs" or "Maximum concurrency", the document is built with0. The schema then rejects the document with a decode error instead of the field-specific message.🛡️ Proposed fix
function parseInteger(value: string, label: string): number { + if (value.trim().length === 0) throw new Error(`${label} is required.`); const parsed = Number(value); if (!Number.isInteger(parsed)) throw new Error(`${label} must be a whole number.`); return parsed; }🤖 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 `@apps/web/src/components/settings/AgentsSettings.logic.ts` around lines 128 - 132, Update parseInteger to explicitly reject empty or whitespace-only value before converting it with Number, so cleared “Maximum runs” and “Maximum concurrency” fields produce the existing label-specific whole-number error instead of being interpreted as zero.apps/mobile/src/features/settings/SettingsAgentsRouteScreen.tsx-718-730 (1)
718-730: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winReflect the off state in the "Always apply" toggle.
The toggle keeps
bg-primaryfor both states, so the off state looks active. The profile toggle at lines 597-609 switches the background. Also add anaccessibilityLabel, because the row label is a sibling element.🎨 Proposed fix
<Pressable accessibilityRole="switch" + accessibilityLabel="Always apply" accessibilityState={{ checked: props.draft.alwaysApply }} onPress={() => props.onChange("alwaysApply", !props.draft.alwaysApply)} - className="rounded-full bg-primary px-3 py-1" + className={`rounded-full px-3 py-1 ${props.draft.alwaysApply ? "bg-primary" : "bg-subtle-strong"}`} > - <Text className="text-sm font-t3-bold text-primary-foreground"> + <Text + className={`text-sm font-t3-bold ${props.draft.alwaysApply ? "text-primary-foreground" : "text-foreground-muted"}`} + > {props.draft.alwaysApply ? "On" : "Off"} </Text> </Pressable>🤖 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 `@apps/mobile/src/features/settings/SettingsAgentsRouteScreen.tsx` around lines 718 - 730, Update the Always apply toggle in the surrounding settings component to use the inactive background styling when props.draft.alwaysApply is false, matching the profile toggle’s state-dependent styling. Add an accessibilityLabel to the Pressable so the control is identified independently of its sibling text label.apps/mobile/src/features/settings/SettingsAgentsRouteScreen.tsx-457-476 (1)
457-476: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAdd loading and error states to the rules list.
The profile list handles
catalog.isPendingandcatalog.error. The rules list does not. During the first load the user sees "No rules yet." If the catalog request fails, the rules section shows the same empty message and hides the failure.🩹 Proposed fix
<View className="overflow-hidden rounded-2xl bg-subtle"> - {rules.length === 0 ? ( + {catalog.isPending && catalog.data === null ? ( + <Text className="p-4 text-sm text-foreground-muted">Loading rules…</Text> + ) : null} + {catalog.error ? ( + <Text accessibilityRole="alert" className="p-4 text-sm text-danger"> + {catalog.error} + </Text> + ) : null} + {!catalog.isPending && !catalog.error && rules.length === 0 ? ( <Text className="p-4 text-sm text-foreground-muted"> No rules yet. Create one to apply reusable instructions by path. </Text> ) : null}🤖 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 `@apps/mobile/src/features/settings/SettingsAgentsRouteScreen.tsx` around lines 457 - 476, Update the rules list rendering around rules.length and rules.map to handle the catalog loading and error states consistently with the profile list: show a loading state while catalog.isPending, show the catalog error when catalog.error is present, and only show “No rules yet” after a successful load with no rules. Preserve the existing RuleRow rendering for loaded rules.packages/contracts/src/agents.ts-445-452 (1)
445-452: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRule failures report profile error messages.
The rule error aliases point at
AgentProfileError. A missing or conflicting rule therefore producesAgentProfileNotFoundErrorwith the text "Agent profile '/' was not found." That text reaches the Rules settings UI and MCP clients and names the wrong entity. Add rule-specificAgentRuleNotFoundErrorandAgentRuleRevisionConflictErrorvariants, or parameterize the message with the document kind.🤖 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 `@packages/contracts/src/agents.ts` around lines 445 - 452, Update the rule error definitions near AgentRuleGetError, AgentRuleSaveError, AgentRuleArchiveError, and AgentRuleRestoreError so rule failures no longer alias AgentProfileError. Add rule-specific not-found and revision-conflict variants, or parameterize the shared error with the rule document kind, ensuring exposed messages identify a rule rather than an agent profile.packages/client-runtime/src/state/agents.ts-37-72 (1)
37-72: 🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick winAdd command-level caching after agent mutations.
The web and mobile settings screens call
catalog.refresh()inline, so this library does not prevent the same mutation from showing stale catalog/profile/rule data from anotheratomQuery. Addregistry.refresh(...)toonSuccessin these commands so consumers do not have to duplicate the same invalidation.🤖 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 `@packages/client-runtime/src/state/agents.ts` around lines 37 - 72, Update the agent mutation commands in the command registry to invalidate related catalog, profile, and rule queries through registry.refresh(...) in each command’s onSuccess handler. Apply this to saveProfile, archiveProfile, restoreProfile, saveRule, archiveRule, and restoreRule, using the existing query identifiers and preserving their current scheduler and concurrency configuration.apps/server/src/agents/prompt/RuleMatcher.ts-116-116 (1)
116-116: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
escapeRegexdoes not escape*, so wildcards inside{...}become regex quantifiers.Line 153 escapes each alternation branch with
escapeRegex. The escape set at line 116 omits*. A glob such as{*.ts,*.tsx}therefore compiles to(?:\*?\.ts|...)-style output where*acts as a quantifier on the preceding character instead of matching a path segment. The rule then matches the wrong files, silently and with no diagnostic.The single-character path at line 156 is not affected, because
*and?are handled by earlier branches. Only alternation contents reachescapeRegexas multi-character strings.Either reject
*and?inside alternations with a thrownError, which surfaces as aninvalid-globdiagnostic, or compile each alternative through the same character loop.This also relates to the static analysis hint on line 158. Glob text reaches
new RegExpafter this incomplete escaping. Rule globs are author-supplied and the candidate list is bounded to the context files, so the practical backtracking risk is low, but the escaping gap should still be closed.Also applies to: 146-154
🤖 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 `@apps/server/src/agents/prompt/RuleMatcher.ts` at line 116, The escapeRegex function must escape `*` so wildcard characters inside alternation branches cannot become regex quantifiers. Update its character class while preserving the existing alternation compilation flow in the matcher around the branch handling that calls `escapeRegex`.Source: Linters/SAST tools
apps/server/src/agents/prompt/RuleMatcher.ts-238-259 (1)
238-259: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
contentBytesdoes not measurecontent.The loop accumulates only
rule.bodybytes. The emittedcontentalso contains a<!-- t3-agent-rule: scope/id -->\nheader for each non-empty rule and a\n\njoiner between chunks. The returnedcontentBytesis therefore always lower than the byte length ofcontent, and the 64 KiB cap does not bound what the prompt actually carries.The loop also charges bytes for rules whose body is empty, which are then skipped at line 252 and contribute nothing to
content.Measure the chunk that is appended.
🐛 Proposed fix
for (const rule of matched.rules) { - const bodyBytes = textEncoder.encode(rule.body).byteLength; - const nextBytes = contentBytes + bodyBytes; + if (rule.body.length === 0) continue; + const chunk = `<!-- t3-agent-rule: ${rule.scope}/${rule.id} -->\n${rule.body}`; + const separatorBytes = chunks.length === 0 ? 0 : 2; + const nextBytes = contentBytes + separatorBytes + textEncoder.encode(chunk).byteLength; if (nextBytes > maxBytes) { throw new AgentRuleContentOverflowError({ limitBytes: maxBytes, actualBytes: nextBytes, ruleId: rule.id, scope: rule.scope, }); } contentBytes = nextBytes; - if (rule.body.length > 0) { - chunks.push(`<!-- t3-agent-rule: ${rule.scope}/${rule.id} -->\n${rule.body}`); - } + chunks.push(chunk); }Note that the test at
apps/server/src/agents/prompt/prompt.test.tsline 94 usesmaxBytes = 4with a 5-byte body, so it still overflows after this change.🤖 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 `@apps/server/src/agents/prompt/RuleMatcher.ts` around lines 238 - 259, Update the content assembly loop around matched.rules so byte accounting measures each emitted chunk, including its rule header and newline, and skips empty bodies before charging bytes. Use the encoded byte length of the exact chunk appended to chunks, accumulate that value for contentBytes, and retain the overflow check against maxBytes so the existing 5-byte body test still throws.apps/server/src/ws.ts-1111-1127 (1)
1111-1127: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReturn catalog diagnostics in
agentsCatalog.
agentCatalog.list()is a success-only effect that collects malformed profile and rule entries inAgentCatalogSnapshot.diagnostics.agentsCatalogcurrently returns onlyprofilesandrules, so a user cannot see why a catalog entry is missing.🤖 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 `@apps/server/src/ws.ts` around lines 1111 - 1127, Update the WS_METHODS.agentsCatalog handler to include catalog.diagnostics in its returned object alongside the filtered profiles and rules, preserving the existing includeArchived filtering behavior.apps/server/src/agents/run/AgentRunReactor.ts-47-53 (1)
47-53: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winLog when the terminal hook is skipped.
If
getProfileSnapshotreturnsNone, or the workspace root cannot be resolved, Line 52 returns without any signal. The configuredafterResultandonErrorhooks then never run, and the operator sees nothing.putProfileSnapshotshould have persisted the snapshot at launch, so a missing snapshot indicates a real defect upstream. Emit a warning withrun.idandrun.profile.revision.🤖 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 `@apps/server/src/agents/run/AgentRunReactor.ts` around lines 47 - 53, Update the early-return guard in the hook execution flow to emit a warning when profile is null or workspaceRoot is null, including run.id and run.profile.revision in the log; preserve the existing return behavior and do not alter hook execution for valid values.apps/server/src/agents/run/AgentRunDeadlineReactor.ts-26-33 (1)
26-33: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winGuard against an unparsable timestamp, and correct the comment.
Two problems exist in this segment.
Date.parsereturnsNaNfor a malformed timestamp.AgentRun.requestedAtandAgentRun.startedAtare plainstringfields, so a corrupt persisted value producesNaN.isDeadlineExpiredthen always returnsfalse, andschedulecomputesMath.max(0, NaN - nowMillis), which isNaN, and passes it toDuration.millis. The run then never reaches its wall-time budget.The comment states the budget starts when a run is requested. The code prefers
run.startedAt, so a run that stays queued receives a later deadline. Align the comment with the code.🐛 Proposed guard
-/** The wall-time budget starts when a run is requested until it finishes. */ -export const deadlineAtMillis = (run: AgentRun): number => { - const origin = Date.parse(run.startedAt ?? run.requestedAt); - return origin + run.budget.maxWallTimeMinutes * 60_000; -}; +/** + * The wall-time budget starts when a run starts, and falls back to the + * request time while the run is still queued. + */ +export const deadlineAtMillis = (run: AgentRun): number => { + const parsed = Date.parse(run.startedAt ?? run.requestedAt); + const origin = Number.isNaN(parsed) ? Date.parse(run.requestedAt) : parsed; + return (Number.isNaN(origin) ? 0 : origin) + run.budget.maxWallTimeMinutes * 60_000; +};🤖 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 `@apps/server/src/agents/run/AgentRunDeadlineReactor.ts` around lines 26 - 33, Update deadlineAtMillis to handle an unparsable run.startedAt or run.requestedAt without returning NaN, preserving a valid deadline calculation for deadline expiration and scheduling; use the existing AgentRun timestamp context and choose an appropriate safe fallback for invalid persisted timestamps. Correct the comment above deadlineAtMillis to state that the wall-time budget starts from the timestamp selected by the implementation, including the startedAt preference.apps/server/src/persistence/Migrations/040_ProjectionThreadsAgentProfile.ts-4-16 (1)
4-16: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winAdd a migration test for
agent_profile_json.
ProjectionThreadRepositoryreads and writesagent_profile_jsonthroughagentProfileinupsert,getById, andlistByProjectId, but040_ProjectionThreadsAgentProfilehas no test cover. Add a migration test that runs up to migration 040 and asserts the column exists, following the existing migration test pattern.🤖 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 `@apps/server/src/persistence/Migrations/040_ProjectionThreadsAgentProfile.ts` around lines 4 - 16, Add a migration test following the existing migration test pattern that runs migrations through 040_ProjectionThreadsAgentProfile and verifies projection_threads contains the agent_profile_json column. Cover the schema assertion only; do not alter ProjectionThreadRepository or migration behavior.apps/server/src/orchestration/Layers/ProviderCommandReactor.ts-774-798 (1)
774-798: 🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick winMove
getCapabilitiesinto the profile branch.
resolvedPrompt.profile !== nullis the only consumer ofrequestedCapabilities, so fetching it on every turn start adds an unneeded provider lookup. Calling it beforeensureSessionForThreadalso runs capabilities lookup after unknown-instance failures, which can return provider capabilities errors instead of the descriptive unknown-instance errors fromgetInstanceInfo.♻️ Proposed refactor
- const requestedModelSelection = - input.modelSelection ?? threadModelSelections.get(input.threadId) ?? thread.modelSelection; - const requestedCapabilities = yield* providerService.getCapabilities( - requestedModelSelection.instanceId, - ); - if (resolvedPrompt.profile !== null) { - const profile = resolvedPrompt.profile; + const requestedModelSelection = + input.modelSelection ?? threadModelSelections.get(input.threadId) ?? thread.modelSelection; + if (resolvedPrompt.profile !== null) { + const profile = resolvedPrompt.profile; + const requestedCapabilities = yield* providerService.getCapabilities( + requestedModelSelection.instanceId, + );🤖 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 `@apps/server/src/orchestration/Layers/ProviderCommandReactor.ts` around lines 774 - 798, Move the providerService.getCapabilities call into the resolvedPrompt.profile !== null branch, immediately before resolveAgentRuntimeCompatibility, and keep requestedModelSelection available there. Avoid fetching capabilities when no profile is present, while preserving ensureSessionForThread/getInstanceInfo validation ordering so unknown instances produce their existing descriptive errors first.
🤖 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 `@apps/mobile/src/features/settings/agentProfile.logic.ts`:
- Around line 51-56: Extract a shared numeric parsing helper that rejects blank
or whitespace-only input before conversion, then apply range validation at each
caller. In apps/mobile/src/features/settings/agentProfile.logic.ts lines 51-56,
update integer so maxRuns and maxWallTimeMinutes reject blank values; in
apps/mobile/src/features/settings/agentRule.logic.ts lines 44-50, make
parseInteger reuse the shared helper instead of duplicating conversion and
integer checks. In apps/mobile/src/features/settings/agentProfile.logic.test.ts
lines 11-29, add negative cases asserting blank budget input and blank priority
cause buildAgentProfileDocument and buildAgentRuleDocument to throw.
- Around line 40-43: Update the runtimeMode default in draftFromProfile so a new
profile uses the least-permissive intended default, “auto,” instead of
“full-access”; leave the other profile defaults unchanged and preserve explicit
existing runtimeMode values.
In `@apps/mobile/src/features/settings/agentRule.logic.ts`:
- Around line 59-65: Update the mapping logic around the scope and id extraction
to split each colon-delimited target only at its first colon, preserving all
remaining text in id. Keep the environment default for targets without a colon,
then validate the preserved remainder with the existing scope and id checks so
malformed multi-colon targets are rejected rather than truncated.
In `@apps/mobile/src/state/use-thread-composer-state.ts`:
- Around line 110-113: Update the draft persistence and empty-draft handling
around updateComposerDraftSettings and isEmptyDraft so an explicit agentProfile:
null is preserved rather than causing the draft to be deleted. Ensure clearing a
thread-locked profile remains effective for the next turn while retaining
existing removal behavior for genuinely empty drafts.
In `@apps/server/src/agents/AgentCatalog.ts`:
- Around line 702-715: Update validate to reuse the single discovered source
collection returned by list/discovery instead of calling find, getProfile, or
getRule for each entry. Extract the post-find loading logic into
loadProfile(source) and loadRule(source), have getProfile and getRule call these
helpers after find, and have validate resolve each entry’s source from the
already discovered collection before invoking the corresponding loader.
In `@apps/server/src/agents/AgentOrchestrationLive.ts`:
- Around line 979-982: Update the follow-up turn construction in send to load
the pinned profile snapshot via runs.getProfileSnapshot(run.profile.revision),
then derive runtimeMode and interactionMode from that profile using the same
logic as spawn rather than hardcoding "full-access" and "default". Preserve the
profile’s approval requirements for every subsequent turn.
In `@apps/server/src/agents/AgentPromptResolver.ts`:
- Around line 24-45: Reject URI schemes in both normalizeCandidate in
apps/server/src/agents/AgentPromptResolver.ts lines 24-45 and
normalizeWorkspaceRelativePath in apps/server/src/agents/prompt/RuleMatcher.ts
lines 74-98 by adding the same scheme-prefix validation after the drive-letter
check; preserve existing rejection behavior and ensure values such as
https://example.com/x are rejected rather than normalized as relative paths.
Consider sharing the validator to prevent the rules from diverging.
In `@apps/server/src/agents/prompt/PromptCompiler.ts`:
- Around line 180-188: Update compileAgentPrompt and the compileAgentRules
integration to handle AgentRuleContentOverflowError without letting it escape as
a defect. Use isAgentRuleContentOverflowError to convert the overflow into the
established AgentPromptDiagnostic/result path, preserving the declared
AgentPromptCompilation and AgentPromptResolutionError flow for user-facing
handling.
In `@apps/server/src/agents/run/AgentRunDeadlineReactor.ts`:
- Around line 56-67: In apps/server/src/agents/run/AgentRunDeadlineReactor.ts
lines 56-67, update the cancellation handling in the deadline reactor to
distinguish Result failure from an empty successful event list: log the failure
details before returning false, while preserving the existing no-op behavior for
empty events. In apps/server/src/agents/run/AgentRunReactor.ts lines 47-53, add
a warning before the early return that includes run.id, run.profile.revision,
and the specific missing value.
In `@apps/server/src/agents/run/AgentRunReactor.ts`:
- Around line 162-175: Update the event handling flow around handle and
runTerminalHook so terminal hooks execute on forked fibers or through a bounded
per-thread concurrent consumer instead of blocking Stream.runForEach’s single
consumer fiber. Preserve sequential event processing for each thread while
allowing hooks from different runs or threads to proceed independently, and
retain the existing error logging context.
- Around line 87-100: The budget-exhaustion branch in AgentRunReactor must stop
matching the human-readable detail text. Add a structured discriminator such as
reason to AgentRunCommandInvariantError, set it for budget-exhaustion failures,
and update the completion check to compare that discriminator while preserving
the existing dispatch behavior.
In `@apps/server/src/mcp/McpSessionRegistry.ts`:
- Line 131: Update McpSessionRegistry.issue() so the "agents" capability is
granted only when the thread’s attached target profile declares the
corresponding MCP tools, rather than unconditionally for every credential;
preserve "preview" access and ensure delegation uses the profile-specific
capability set.
In `@apps/server/src/orchestration/Layers/ProviderCommandReactor.test.ts`:
- Around line 303-308: Update the thread.turn.start test setup around
resolveAgentPrompt and the dispatch at line 577 to provide a valid agentProfile
with a known profileRef, then assert the resolveAgentPrompt mock receives that
matching profileRef alongside the message. Ensure the test specifically
exercises the pinned-profile path rather than allowing a null profile reference.
- Around line 2917-2926: Replace the local waitFor polling block in the test
with yield* Effect.promise(() => harness.drain()) after the relevant dispatches.
Use harness.drain to wait for both reactor workers before reading the model,
while preserving the subsequent failure-activity assertions.
In `@apps/server/src/persistence/Migrations/039_AgentRuns.ts`:
- Around line 69-72: Update the migration near the existing
idx_projection_agent_runs_lineage definition to add a dedicated index on
projection_agent_runs(root_run_id), using an IF NOT EXISTS guard and a clear
root-run index name. Leave the existing parent_run_id, status, and created_at
index unchanged.
In `@apps/server/src/ws.ts`:
- Around line 2384-2391: Update websocketRpcRouteLayer and the server
route-layer wiring to construct one shared AgentProfileServices.layer per server
instance and provide that same layer to both HTTP routes and WebSocket RPC,
rather than extracting and rebuilding AgentCatalog, AgentProfileStore, and
AgentRuleStore inside websocketRpcRouteLayer. Ensure AgentRuleStore and
AgentProfileStore retain shared mutex and revision state across both transports.
In `@apps/web/src/components/settings/AgentsSettings.tsx`:
- Around line 506-518: Update the scope select in the AgentsSettings profile
form to use the existing-profile rule from RulesSettings: disable it when the
profile is not new by applying the equivalent isNew-based disabled condition,
while preserving the current value and change handling.
In `@apps/web/src/components/settings/RulesSettings.tsx`:
- Around line 149-158: Update the catch block in the rule restore flow to handle
caught values with the same instanceof Error check used near line 132, passing
plain Error instances directly to setError and using failureMessage only for
Cause values. Remove the unsafe Cause.Cause cast while preserving the existing
error-state behavior.
- Around line 108-113: Update save in RulesSettings so buildAgentRuleDocument
receives no baseline when creating a new rule: use the selectedKey/new-rule
state to pass null for new rules, matching the isNew guard in AgentsSettings,
and retain ruleQuery.data?.rule only when editing an existing selected rule.
---
Minor comments:
In `@apps/mobile/src/features/settings/SettingsAgentsRouteScreen.tsx`:
- Around line 718-730: Update the Always apply toggle in the surrounding
settings component to use the inactive background styling when
props.draft.alwaysApply is false, matching the profile toggle’s state-dependent
styling. Add an accessibilityLabel to the Pressable so the control is identified
independently of its sibling text label.
- Around line 457-476: Update the rules list rendering around rules.length and
rules.map to handle the catalog loading and error states consistently with the
profile list: show a loading state while catalog.isPending, show the catalog
error when catalog.error is present, and only show “No rules yet” after a
successful load with no rules. Preserve the existing RuleRow rendering for
loaded rules.
In `@apps/server/src/agents/prompt/RuleMatcher.ts`:
- Line 116: The escapeRegex function must escape `*` so wildcard characters
inside alternation branches cannot become regex quantifiers. Update its
character class while preserving the existing alternation compilation flow in
the matcher around the branch handling that calls `escapeRegex`.
- Around line 238-259: Update the content assembly loop around matched.rules so
byte accounting measures each emitted chunk, including its rule header and
newline, and skips empty bodies before charging bytes. Use the encoded byte
length of the exact chunk appended to chunks, accumulate that value for
contentBytes, and retain the overflow check against maxBytes so the existing
5-byte body test still throws.
In `@apps/server/src/agents/run/AgentRunDeadlineReactor.ts`:
- Around line 26-33: Update deadlineAtMillis to handle an unparsable
run.startedAt or run.requestedAt without returning NaN, preserving a valid
deadline calculation for deadline expiration and scheduling; use the existing
AgentRun timestamp context and choose an appropriate safe fallback for invalid
persisted timestamps. Correct the comment above deadlineAtMillis to state that
the wall-time budget starts from the timestamp selected by the implementation,
including the startedAt preference.
In `@apps/server/src/agents/run/AgentRunReactor.ts`:
- Around line 47-53: Update the early-return guard in the hook execution flow to
emit a warning when profile is null or workspaceRoot is null, including run.id
and run.profile.revision in the log; preserve the existing return behavior and
do not alter hook execution for valid values.
In `@apps/server/src/orchestration/Layers/ProjectionPipeline.ts`:
- Around line 813-816: Update the projectionThreadRepository.upsert call in the
projection event handler to set updatedAt to event.occurredAt alongside
agentProfile, preserving the existing row fields and ensuring the thread
timestamp reflects this state transition.
In `@apps/server/src/orchestration/Layers/ProviderCommandReactor.ts`:
- Around line 774-798: Move the providerService.getCapabilities call into the
resolvedPrompt.profile !== null branch, immediately before
resolveAgentRuntimeCompatibility, and keep requestedModelSelection available
there. Avoid fetching capabilities when no profile is present, while preserving
ensureSessionForThread/getInstanceInfo validation ordering so unknown instances
produce their existing descriptive errors first.
In `@apps/server/src/persistence/Migrations/040_ProjectionThreadsAgentProfile.ts`:
- Around line 4-16: Add a migration test following the existing migration test
pattern that runs migrations through 040_ProjectionThreadsAgentProfile and
verifies projection_threads contains the agent_profile_json column. Cover the
schema assertion only; do not alter ProjectionThreadRepository or migration
behavior.
In `@apps/server/src/ws.ts`:
- Around line 1111-1127: Update the WS_METHODS.agentsCatalog handler to include
catalog.diagnostics in its returned object alongside the filtered profiles and
rules, preserving the existing includeArchived filtering behavior.
In `@apps/web/src/components/settings/AgentsSettings.logic.ts`:
- Around line 128-132: Update parseInteger to explicitly reject empty or
whitespace-only value before converting it with Number, so cleared “Maximum
runs” and “Maximum concurrency” fields produce the existing label-specific
whole-number error instead of being interpreted as zero.
In `@docs/internals/glossary.md`:
- Around line 95-97: Update the glossary’s Rule definition to change the wording
from “applies always” to “always applies,” preserving the rest of the definition
and its reference unchanged.
In `@packages/client-runtime/src/state/agents.ts`:
- Around line 37-72: Update the agent mutation commands in the command registry
to invalidate related catalog, profile, and rule queries through
registry.refresh(...) in each command’s onSuccess handler. Apply this to
saveProfile, archiveProfile, restoreProfile, saveRule, archiveRule, and
restoreRule, using the existing query identifiers and preserving their current
scheduler and concurrency configuration.
In `@packages/contracts/src/agents.ts`:
- Around line 445-452: Update the rule error definitions near AgentRuleGetError,
AgentRuleSaveError, AgentRuleArchiveError, and AgentRuleRestoreError so rule
failures no longer alias AgentProfileError. Add rule-specific not-found and
revision-conflict variants, or parameterize the shared error with the rule
document kind, ensuring exposed messages identify a rule rather than an agent
profile.
---
Nitpick comments:
In `@apps/mobile/src/state/thread-outbox.test.ts`:
- Around line 131-149: Extend the test around resolveQueuedThreadSettings to
cover both remaining agentProfile branches: assert that an explicit
agentProfile: null clears the thread profile, and assert that omitting the
agentProfile property preserves the existing thread profile. Keep the current
queued-profile round-trip assertion unchanged so all three hasOwnProperty-based
behaviors are verified.
In `@apps/mobile/src/state/use-thread-composer-state.test.ts`:
- Around line 1-4: Rename the test file from use-thread-composer-state.test.ts
to agentProfileSelection.test.ts so its filename matches the
resolveAgentProfileSelection module under test.
In `@apps/server/src/agents/AgentCatalog.test.ts`:
- Around line 33-298: Add a focused test in the AgentCatalog suite that calls
the public validate method with one valid profile and one Markdown document
lacking frontmatter, then assert validation aggregates the expected
missing-frontmatter diagnostic alongside the valid document results. Exercise
both discovery/list diagnostics and per-document loading through validate,
without using arbitrary timeouts.
In `@apps/server/src/agents/AgentCatalog.ts`:
- Around line 193-200: Introduce a scope-neutral AgentLocator (or
AgentRuleLocator) alias in packages/contracts and use it for rule references,
including getRule and RuleFrontmatter.profiles where applicable, while retaining
AgentProfileLocator for profile references. Rename decodeProfileLocator at the
rule-decoding call sites around lines 493 and 825 to reflect that it decodes
rule identifiers, and update all related imports and usages.
In `@apps/server/src/agents/AgentHookRunner.ts`:
- Around line 113-127: Update the POSIX invocation in the shell-hook runner to
use /bin/sh with only the -c argument, removing the login-shell behavior while
preserving the existing command, working directory, timeout, and output
handling.
- Around line 15-45: Extract the identical field definitions from
AgentHookBlockedError and AgentHookExecutionError into a shared schema field-set
constant, then reuse it when declaring both TaggedErrorClass schemas. Keep both
error tags and their existing message getters distinct and unchanged.
In `@apps/server/src/agents/AgentOrchestrationLive.ts`:
- Around line 699-709: In the isolated-worktree branch of the prepareThread
setup, capture the already-validated context.thread.branch in a local constant
immediately after its guard, then use that constant for refName and baseRefName
instead of non-null assertions. Preserve the existing branch validation and
worktree creation behavior.
In `@apps/server/src/agents/AgentProfileStore.test.ts`:
- Around line 172-173: Update the test around projectFile in AgentProfileStore
tests to parse t3.json as JSON and inspect its decoded agents array. Assert that
exactly one agent has id "project-reviewer", replacing the raw project-reviewer
substring count while preserving the existing file-read flow.
- Around line 93-104: The compare-and-swap failure paths are untested. In
apps/server/src/agents/AgentProfileStore.test.ts:93-104, add a second save using
the already-consumed saved.revision and assert
AgentProfileStoreRevisionConflictError; in
apps/server/src/agents/AgentRuleStore.test.ts:62-75, add the equivalent
stale-revision save and assert the rule store’s typed revision-conflict error.
Also add, in either test file, coverage for supplying expectedRevision when the
profile or rule does not exist, asserting the appropriate failure.
In `@apps/server/src/agents/AgentProfileStore.ts`:
- Around line 388-397: Resolve the `documentPath` behavior for new environment
profiles in the surrounding profile creation flow: either honor
`input.profile.sourcePath` consistently, or validate and reject conflicting
values while retaining `defaultPath`. If always using the environment default is
intentional, add a concise comment documenting that rule; preserve
existing-project behavior.
In `@apps/server/src/agents/AgentPromptResolver.test.ts`:
- Around line 87-89: Strengthen the test assertion in the resolver test so it
verifies the compiled prelude appears before the user-supplied marker, using the
ordering of “## T3 runtime” and malicious in resolved.message. Keep the existing
assertions unchanged; do not address duplicate markers here, as that belongs to
PromptCompiler.
In `@apps/server/src/agents/AgentPromptResolver.ts`:
- Around line 183-199: Update the Effect.forEach call that loads snapshot.rules
in resolve to use bounded concurrency greater than one, preserving the existing
resolutionError mapping and result ordering. Do not alter rule lookup behavior;
only configure concurrency to avoid sequential catalog.getRule calls.
- Around line 156-166: Extract the child-run turn command ID construction into a
shared helper, then reuse it in AgentOrchestrationLive when sending
thread.turn.start and in AgentPromptResolver.isCompiledAgentTurn when comparing
commandId. Ensure the helper consistently formats the agent-spawn prefix and run
ID.
In `@apps/server/src/agents/AgentRuleStore.test.ts`:
- Around line 137-138: Update the assertion in the t3.json test to decode
projectFile as JSON, inspect its rules array, and assert exactly one entry whose
id is "project-typescript"; remove the substring-count assertion so the test
validates structured data rather than JSON text layout.
In `@apps/server/src/agents/AgentRuleStore.ts`:
- Around line 365-372: Update the documentPath selection in the relevant
AgentRuleStore function to use Result.isSuccess(current), matching the existing
check earlier in the same function, while preserving the current success and
fallback sourcePath behavior.
In `@apps/server/src/agents/prompt/prompt.test.ts`:
- Around line 72-88: The matching tests around matchAgentRules need coverage for
advanced and malformed glob patterns. Add focused cases for alternation and
character-class patterns, including src/**/*.{ts,tsx} matching src/a.tsx and
{*.ts,*.tsx} matching the appropriate files, plus an unclosed src/[ts pattern
that yields an invalid-glob diagnostic and never matches.
In `@apps/server/src/agents/prompt/PromptCompiler.ts`:
- Around line 83-84: Remove the redundant compatibility aliases from
PromptCompiler and RuleMatcher: delete portablePromptEnvelope, compilePrompt,
matchRules, compileRules, and RuleContentOverflowError exports or properties,
retaining only the canonical names and updating index re-exports and internal
references accordingly.
- Around line 144-148: Document in renderPortablePrompt that envelope.task is
intentionally preserved verbatim and may contain duplicate prompt markers or
section headings, so future consumers must not treat them as trust signals;
update the existing AgentPromptResolver test expectation only if needed to
preserve this documented behavior.
In `@apps/server/src/agents/run/AgentRun.test.ts`:
- Around line 165-292: Update the test title for “enforces inherited depth,
run-count, concurrency, and token budgets” to also identify the estimated-cost
budget assertion, so all five covered budget dimensions are visible when the
test fails.
- Around line 294-405: Add tests in the AgentRun transition/decision coverage
for both missing branches: advance the command occurredAt beyond the configured
maxWallTimeMinutes and assert the relevant succeed or follow-up decision is
rejected, then request a child run with maxRuns or maxTotalTokens greater than
its parent and assert budgetDoesNotExpand rejects it. Reuse the existing
request, start, transition, decide, and fixture helpers and preserve current
assertions.
In `@apps/server/src/agents/run/AgentRun.ts`:
- Around line 365-475: Update the second event switch in evolve to add a default
branch that passes the event to a never assertion, making any newly added
AgentRunEvent variant a compile-time error while preserving all existing cases.
In `@apps/server/src/agents/run/AgentRunDeadlineReactor.test.ts`:
- Around line 90-165: Add focused coverage for the make scheduling flow: use a
fake change stream and TestClock to verify layer startup recovers an active run
from listActive and schedules expiration, then emits a completed-status change
and verifies cancelScheduled removes the timer. Exercise the behavior
deterministically without arbitrary timeouts, anchoring the test around make,
listActive recovery, change-event handling, and TestClock.
- Around line 67-85: Replace the trailing `as AgentRunRepository["Service"]`
cast in `repositoryFor` with a `satisfies AgentRunRepository["Service"]` check
on the object literal, preserving the existing inferred object type while
ensuring missing or incompatible service members fail typechecking.
In `@apps/server/src/agents/run/AgentRunReactor.ts`:
- Around line 22-26: Import the exported AgentRun type from ./AgentRun.ts and
use it directly for the run parameter in both hookWorkspace and the other helper
around lines 41–45. Remove the duplicated
NonNullable<Effect.Success<ReturnType<typeof repository.get>>…> type derivation
while preserving the existing helper behavior.
In `@apps/server/src/agents/run/AgentRunRepository.test.ts`:
- Around line 66-206: Extend the AgentRunRepository test suite with focused
coverage for putProfileSnapshot and getProfileSnapshot, including the persisted
profile data used by recovery or hook execution. Add assertions for
getByChildThread and listActive, including expected child-thread lookup and
active-run filtering. Keep the tests isolated within
testLayer("AgentRunRepository", ...) and use the existing migration, repository,
and fixture helpers.
In `@apps/server/src/agents/run/AgentRunRepository.ts`:
- Around line 159-163: The unfiltered SQL branch in the event retrieval method
should not remain available. Make the where/filter parameter required and remove
the fallback query that selects all events, updating the method signature and
callers as needed while preserving the existing filtered queries.
- Around line 218-236: Update the AgentRunRepository projection and migration
039 so result_json is populated from the run’s result data rather than a
constant null, and consumedEstimatedCostUsd is stored alongside consumedTokens.
Ensure the INSERT column/value lists and conflict-update clause consistently
include both fields, using the existing serialization pattern for result data
and the AgentRun property for cost.
In `@apps/server/src/mcp/McpHttpServer.ts`:
- Around line 217-227: Rename ToolkitRegistrations to a preview-specific name
that reflects its use by PreviewToolkitRegistrationLive. Wrap
AgentToolkitHandlersLive and AgentToolkit with
McpToolkit.makeMcpToolkitRegistration using the agents capability, then expose
AgentToolkitRegistrationLive from that registration so it follows the same
capability-aware path as the preview toolkits.
In `@apps/server/src/orchestration/agentProfile.test.ts`:
- Around line 17-30: Update the event helper so it is generic over a specific
OrchestrationEvent member, deriving the type and payload parameters from that
member instead of accepting an arbitrary type and unknown payload. Construct the
shared envelope with the generic event type and retain only any necessary cast
for fields whose union narrowing requires it, ensuring each event call validates
its payload against the selected event type.
- Around line 61-72: Add a second turn-start test case alongside the existing
null-agentProfile case, using a non-null agentProfile in
thread.turn-start-requested and asserting that the projected thread preserves
that exact value. Keep the existing null case unchanged so both clearing and
applying event values are verified.
In `@apps/server/src/orchestration/projector.ts`:
- Around line 547-563: Update the "thread.turn-start-requested" handler so that
when payload.agentProfile is undefined it returns nextBase unchanged, avoiding
the updateThread call; retain the existing agentProfile patch through
updateThread when the value is present.
In `@apps/server/src/persistence/Migrations/039_AgentRuns.test.ts`:
- Around line 48-59: Replace the name-based index assertions in the migration
test with behavioral checks: insert duplicate agent_run_events rows sharing the
same agent_run_id and revision, and duplicate projection_agent_runs rows sharing
child_thread_id, asserting both inserts fail. Preserve any setup required by the
schema and ensure the event constraint test covers the duplicate-revision
behavior used by AgentRunRepository.dispatch.
In `@apps/server/src/persistence/Migrations/039_AgentRuns.ts`:
- Around line 74-82: Remove the redundant CREATE INDEX statements for
idx_agent_run_events_run_revision and idx_projection_agent_runs_child_thread
from the migration, relying on the existing UNIQUE constraints. Update the
corresponding assertions in the migration test to no longer expect either index
name.
In `@apps/server/src/provider/AgentRuntimeCompatibility.test.ts`:
- Around line 29-46: Add coverage in AgentRuntimeCompatibility tests for the
delegation path in resolveAgentRuntimeCompatibility, ensuring the portable
fixture’s mcpServerInjection capability does not mask it. Add assertions for
both mcp-server-injection-unsupported and token-accounting-unsupported while
preserving the existing unsupported-issue expectations.
In `@apps/server/src/ws.ts`:
- Around line 470-482: Replace the derived ref type in mapAgentCatalogError with
AgentProfileLocator, and add AgentProfileLocator to the existing
`@t3tools/contracts` import. Preserve the current error mapping behavior and ref
values.
In `@apps/web/src/components/chat/ChatComposer.tsx`:
- Around line 680-682: Update the selectedAgentProfile useState declaration in
ChatComposer to remove the explicit AgentProfileRef | null generic parameter and
rely on the lazy initializer’s inferred type, preserving the existing initial
value and state behavior.
In `@apps/web/src/components/settings/AgentsSettings.logic.test.ts`:
- Around line 24-44: Update the revision assertion in the “preserves a revision
and parses structured policy fields” test to compare document.revision with the
exact baseline revision, "a".repeat(64), instead of only validating its
hexadecimal format. Keep the existing structured-field assertions unchanged.
In `@apps/web/src/components/settings/AgentsSettings.logic.ts`:
- Around line 177-204: In the configuration-building logic, compute each
optional numeric value once before constructing the result object, then reuse it
for both the undefined check and assigned property. Update the handling around
sharedWriteConcurrency, maxTotalTokens, and maxEstimatedCostUsd while preserving
omission of undefined fields and existing parse behavior.
In `@apps/web/src/components/settings/RulesSettings.logic.ts`:
- Around line 74-89: Extract the shared archived/scope/name/id comparator into
one generic catalog sort helper, then re-export that helper under both
sortAgentRules and sortAgentProfiles. Update the existing sortAgentRules
implementation and the corresponding sortAgentProfiles implementation to use the
shared symbol while preserving their current signatures and ordering.
In `@apps/web/src/components/settings/settingsSearch.ts`:
- Around line 171-175: Add a separate search entry titled “Rules” alongside the
existing “Agents” entry in the settings search configuration, using the same
/settings/agents route so RulesSettingsPanel is discoverable when users search
for “rules”.
In `@apps/web/src/routeTree.gen.ts`:
- Around line 100-104: Remove the `as any` cast from the
`SettingsAgentsRouteImport.update` call in the generated route definition by
regenerating it with typed output. If the generator cannot avoid the cast, add a
narrowly scoped lint exception for this generated route file and document the
generated-code justification.
In `@packages/contracts/src/agents.ts`:
- Around line 351-382: Introduce or reuse a dedicated AgentRuleId brand from
agentRefs.ts, then update AgentRuleSummary.id and all rule operation
identifiers, including AgentRuleGetInput.id, AgentRuleArchiveInput.id, and the
id carried by AgentRuleDocument used in AgentRuleSaveInput, to use AgentRuleId
consistently instead of AgentProfileId or AgentSlug.
- Around line 674-746: Remove the compatibility alias export blocks for
AgentMcpStart/Get/Submit, AgentMcpAgent*, and McpAgentRun* so each codec has one
canonical exported name. Update any call sites referencing those aliases to use
the corresponding canonical
AgentMcpList/Spawn/Status/Wait/Result/Send/Cancel/Integrate symbols.
🪄 Autofix
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: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 121fbec2-7b16-4a78-9116-a9cc89a98d48
📒 Files selected for processing (121)
apps/mobile/src/Stack.tsxapps/mobile/src/features/settings/SettingsAgentsRouteScreen.tsxapps/mobile/src/features/settings/SettingsRouteScreen.tsxapps/mobile/src/features/settings/agentProfile.logic.test.tsapps/mobile/src/features/settings/agentProfile.logic.tsapps/mobile/src/features/settings/agentRule.logic.test.tsapps/mobile/src/features/settings/agentRule.logic.tsapps/mobile/src/features/settings/components/settings-sheet-targets.tsapps/mobile/src/features/threads/NewTaskDraftScreen.tsxapps/mobile/src/features/threads/ThreadComposer.tsxapps/mobile/src/features/threads/ThreadDetailScreen.tsxapps/mobile/src/features/threads/ThreadRouteScreen.tsxapps/mobile/src/features/threads/new-task-flow-provider.tsxapps/mobile/src/features/threads/use-project-actions.tsapps/mobile/src/lib/projectThreadStartTurn.tsapps/mobile/src/state/agentProfileSelection.tsapps/mobile/src/state/agents.tsapps/mobile/src/state/thread-outbox-model.tsapps/mobile/src/state/thread-outbox.test.tsapps/mobile/src/state/use-composer-drafts.tsapps/mobile/src/state/use-thread-composer-state.test.tsapps/mobile/src/state/use-thread-composer-state.tsapps/mobile/src/state/use-thread-outbox-drain.tsapps/server/integration/OrchestrationEngineHarness.integration.tsapps/server/src/agents/AgentCatalog.test.tsapps/server/src/agents/AgentCatalog.tsapps/server/src/agents/AgentHookRunner.test.tsapps/server/src/agents/AgentHookRunner.tsapps/server/src/agents/AgentOrchestration.tsapps/server/src/agents/AgentOrchestrationLive.test.tsapps/server/src/agents/AgentOrchestrationLive.tsapps/server/src/agents/AgentProfileServices.tsapps/server/src/agents/AgentProfileStore.test.tsapps/server/src/agents/AgentProfileStore.tsapps/server/src/agents/AgentPromptResolver.test.tsapps/server/src/agents/AgentPromptResolver.tsapps/server/src/agents/AgentRuleStore.test.tsapps/server/src/agents/AgentRuleStore.tsapps/server/src/agents/prompt/PromptCompiler.tsapps/server/src/agents/prompt/RuleMatcher.tsapps/server/src/agents/prompt/index.tsapps/server/src/agents/prompt/prompt.test.tsapps/server/src/agents/run/AgentRun.test.tsapps/server/src/agents/run/AgentRun.tsapps/server/src/agents/run/AgentRunDeadlineReactor.test.tsapps/server/src/agents/run/AgentRunDeadlineReactor.tsapps/server/src/agents/run/AgentRunReactor.tsapps/server/src/agents/run/AgentRunRepository.test.tsapps/server/src/agents/run/AgentRunRepository.tsapps/server/src/auth/RpcAuthorization.tsapps/server/src/mcp/McpHttpServer.test.tsapps/server/src/mcp/McpHttpServer.tsapps/server/src/mcp/McpInvocationContext.test.tsapps/server/src/mcp/McpInvocationContext.tsapps/server/src/mcp/McpSessionRegistry.test.tsapps/server/src/mcp/McpSessionRegistry.tsapps/server/src/mcp/McpToolkit.tsapps/server/src/mcp/toolkits/agents/handlers.tsapps/server/src/mcp/toolkits/agents/tools.tsapps/server/src/orchestration/Layers/ProjectionPipeline.tsapps/server/src/orchestration/Layers/ProjectionSnapshotQuery.tsapps/server/src/orchestration/Layers/ProviderCommandReactor.test.tsapps/server/src/orchestration/Layers/ProviderCommandReactor.tsapps/server/src/orchestration/agentProfile.test.tsapps/server/src/orchestration/decider.tsapps/server/src/orchestration/projector.tsapps/server/src/persistence/Layers/ProjectionThreads.tsapps/server/src/persistence/Migrations.tsapps/server/src/persistence/Migrations/039_AgentRuns.test.tsapps/server/src/persistence/Migrations/039_AgentRuns.tsapps/server/src/persistence/Migrations/040_ProjectionThreadsAgentProfile.tsapps/server/src/persistence/Services/ProjectionThreads.tsapps/server/src/provider/AgentRuntimeCompatibility.test.tsapps/server/src/provider/AgentRuntimeCompatibility.tsapps/server/src/provider/Layers/ClaudeAdapter.tsapps/server/src/provider/Layers/CodexAdapter.tsapps/server/src/provider/Layers/CursorAdapter.tsapps/server/src/provider/Layers/GrokAdapter.tsapps/server/src/provider/Layers/OpenCodeAdapter.tsapps/server/src/provider/Services/ProviderAdapter.tsapps/server/src/server.test.tsapps/server/src/server.tsapps/server/src/ws.tsapps/web/src/components/ChatView.logic.test.tsapps/web/src/components/ChatView.logic.tsapps/web/src/components/ChatView.tsxapps/web/src/components/chat/AgentProfilePicker.logic.tsapps/web/src/components/chat/AgentProfilePicker.test.tsapps/web/src/components/chat/AgentProfilePicker.tsxapps/web/src/components/chat/ChatComposer.tsxapps/web/src/components/settings/AgentsSettings.logic.test.tsapps/web/src/components/settings/AgentsSettings.logic.tsapps/web/src/components/settings/AgentsSettings.test.tsxapps/web/src/components/settings/AgentsSettings.tsxapps/web/src/components/settings/RulesSettings.logic.test.tsapps/web/src/components/settings/RulesSettings.logic.tsapps/web/src/components/settings/RulesSettings.test.tsxapps/web/src/components/settings/RulesSettings.tsxapps/web/src/components/settings/SettingsSidebarNav.tsxapps/web/src/components/settings/settingsSearch.tsapps/web/src/routeTree.gen.tsapps/web/src/routes/settings.agents.tsxapps/web/src/state/agents.tsdocs/internals/agents.mddocs/internals/glossary.mddocs/user/agents.mdpackages/client-runtime/package.jsonpackages/client-runtime/src/state/agents.tspackages/contracts/src/agentRefs.test.tspackages/contracts/src/agentRefs.tspackages/contracts/src/agents.test.tspackages/contracts/src/agents.tspackages/contracts/src/index.tspackages/contracts/src/orchestration.tspackages/contracts/src/providerRuntime.tspackages/contracts/src/rpc.tspackages/contracts/src/t3ProjectFile.test.tspackages/contracts/src/t3ProjectFile.tspackages/shared/package.jsonpackages/shared/src/agentRuleGlobs.test.tspackages/shared/src/agentRuleGlobs.ts
megascan
force-pushed
the
agent/native-agent-orchestration
branch
from
August 7, 2026 19:19
091b57d to
4195a0b
Compare
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
megascan
force-pushed
the
agent/native-agent-orchestration
branch
from
August 7, 2026 19:42
4195a0b to
2ec33ac
Compare
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
Contributor
There was a problem hiding this comment.
Follow-up on the earlier findings: the standalone *Shape interfaces, the AgentRuleStore message getters, and the duplicated AgentProfileServices.layer in ws.ts are all resolved, and the hook/prompt/orchestration errors now carry structural attributes plus a real cause. One retained issue: several pure validation failures now manufacture an Error (or default one) purely to satisfy a cause field that is required, which the conventions call out explicitly. Three inline notes below.
Posted via Macroscope — Effect Service Conventions
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
apps/server/src/agents/prompt/RuleMatcher.ts (1)
21-21: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove redundant return type annotations.
TypeScript infers these return types from the implementations. Keep annotations where they define required parameter or public data shapes.
As per coding guidelines, “Prefer inferred types over explicit annotations and do not use
any.”Also applies to: 33-33, 74-74, 101-101, 117-117, 120-120, 163-166, 173-176, 184-184
🤖 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 `@apps/server/src/agents/prompt/RuleMatcher.ts` at line 21, Remove redundant explicit return type annotations from the identified getters and methods in RuleMatcher, including get message and the additional referenced locations, while preserving annotations that define required parameter or public data shapes. Keep the implementations and inferred return behavior unchanged, and do not introduce any.Source: Coding guidelines
apps/server/src/agents/AgentOrchestrationLive.ts (1)
822-831: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winReplace
Option.getOrThrowwith a typed failure.Line 830 converts a missing run into an unhandled defect. Every other failure in
spawnmaps intoAgentProfileInvalidError. An MCP caller receives a typed error in all other paths and an untyped defect here.♻️ Proposed refactor
const run = yield* runs.get(runId).pipe( Effect.mapError((cause) => invalid("Could not reload the Agent run.", { operation: "run-reload", cause, runId, }), ), - Effect.map(Option.getOrThrow), + Effect.flatMap( + Option.match({ + onNone: () => + Effect.fail( + invalid("The Agent run disappeared after it started.", { + operation: "run-reload", + runId, + }), + ), + onSome: Effect.succeed, + }), + ), );🤖 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 `@apps/server/src/agents/AgentOrchestrationLive.ts` around lines 822 - 831, Update the run reload flow in spawn around runs.get and Option.getOrThrow so a missing run produces the same typed AgentProfileInvalidError path as other failures instead of an unhandled defect. Map the empty Option to invalid with the existing run-reload operation context and runId, while preserving the current successful run value and runs.get error mapping.
🤖 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 `@apps/mobile/src/features/settings/agentSettings.logic.ts`:
- Around line 1-5: Update parseRequiredNumber to parse the trimmed input once,
reject results that are not finite—including NaN—alongside the existing
required-value validation, and remove its explicit number return type so
TypeScript infers it.
In `@apps/server/src/agents/AgentOrchestrationLive.ts`:
- Around line 693-718: Update failSpawn to clean up isolated Git worktrees
before deleting the child thread when target.workspace.mode is
"isolated-worktree". Track successful createWorktree completion in prepareThread
and only invoke the worktree-removal operation from failSpawn when that creation
succeeded, preserving existing failure dispatch behavior.
- Around line 584-607: Update the agent-run.request handling around the visible
compileAgentPrompt flow to apply the same lineage token and estimated-cost
budget checks already used for follow-up handling. Validate the requested child
run before it is spawned, reject over-budget requests through the existing error
path, and preserve the current depth, run-count, and concurrency checks.
- Around line 52-58: Update the AgentProfileInvalidError construction to omit
the cause property when context?.cause is undefined, using the same
conditional-spread pattern as profileId and runId; preserve the existing cause
value whenever one is provided.
In `@apps/server/src/agents/prompt/RuleMatcher.ts`:
- Around line 241-259: Update the rule-content accumulation in the loop over
matched.rules to measure the serialized output rather than only rule.body:
construct each non-empty rule’s header/body chunk, include its "\n\n" separator
as emitted by chunks.join("\n\n"), and check the resulting byte count against
maxBytes before appending. Preserve empty-rule behavior and report the
serialized size in AgentRuleContentOverflowError.
- Around line 120-160: Replace the backtracking RegExp construction in globRegex
with a linear-time glob-matching implementation, ensuring patterns containing
repeated or overlapping wildcards such as **a cannot cause exponential work when
tested against a non-matching path. Preserve the existing glob semantics for *,
**, ?, character classes, and alternations, and update callers to use the safe
matcher instead of expression.test.
---
Nitpick comments:
In `@apps/server/src/agents/AgentOrchestrationLive.ts`:
- Around line 822-831: Update the run reload flow in spawn around runs.get and
Option.getOrThrow so a missing run produces the same typed
AgentProfileInvalidError path as other failures instead of an unhandled defect.
Map the empty Option to invalid with the existing run-reload operation context
and runId, while preserving the current successful run value and runs.get error
mapping.
In `@apps/server/src/agents/prompt/RuleMatcher.ts`:
- Line 21: Remove redundant explicit return type annotations from the identified
getters and methods in RuleMatcher, including get message and the additional
referenced locations, while preserving annotations that define required
parameter or public data shapes. Keep the implementations and inferred return
behavior unchanged, and do not introduce any.
🪄 Autofix
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: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 5daa31b4-9fa3-4d8e-96b2-d0b2d7a434ce
📒 Files selected for processing (31)
apps/mobile/src/features/settings/agentProfile.logic.test.tsapps/mobile/src/features/settings/agentProfile.logic.tsapps/mobile/src/features/settings/agentRule.logic.test.tsapps/mobile/src/features/settings/agentRule.logic.tsapps/mobile/src/features/settings/agentSettings.logic.tsapps/mobile/src/state/use-composer-drafts.test.tsapps/server/src/agents/AgentCatalog.test.tsapps/server/src/agents/AgentCatalog.tsapps/server/src/agents/AgentOrchestrationLive.test.tsapps/server/src/agents/AgentOrchestrationLive.tsapps/server/src/agents/AgentPromptResolver.test.tsapps/server/src/agents/AgentPromptResolver.tsapps/server/src/agents/prompt/RuleMatcher.tsapps/server/src/agents/prompt/prompt.test.tsapps/server/src/agents/run/AgentRun.test.tsapps/server/src/agents/run/AgentRun.tsapps/server/src/agents/run/AgentRunDeadlineReactor.tsapps/server/src/agents/run/AgentRunReactor.tsapps/server/src/mcp/McpSessionRegistry.tsapps/server/src/orchestration/Layers/ProviderCommandReactor.test.tsapps/server/src/persistence/Migrations/039_AgentRuns.test.tsapps/server/src/persistence/Migrations/039_AgentRuns.tsapps/server/src/ws.tsapps/web/src/components/settings/AgentsSettings.logic.test.tsapps/web/src/components/settings/AgentsSettings.logic.tsapps/web/src/components/settings/AgentsSettings.test.tsxapps/web/src/components/settings/AgentsSettings.tsxapps/web/src/components/settings/RulesSettings.logic.test.tsapps/web/src/components/settings/RulesSettings.logic.tsapps/web/src/components/settings/RulesSettings.tsxdocs/user/agents.md
🚧 Files skipped from review as they are similar to previous changes (24)
- apps/mobile/src/features/settings/agentProfile.logic.test.ts
- apps/web/src/components/settings/AgentsSettings.logic.test.ts
- apps/server/src/persistence/Migrations/039_AgentRuns.test.ts
- apps/web/src/components/settings/RulesSettings.logic.test.ts
- apps/server/src/agents/AgentPromptResolver.test.ts
- apps/server/src/agents/prompt/prompt.test.ts
- apps/mobile/src/features/settings/agentRule.logic.test.ts
- apps/web/src/components/settings/AgentsSettings.logic.ts
- apps/server/src/mcp/McpSessionRegistry.ts
- apps/server/src/agents/run/AgentRunDeadlineReactor.ts
- apps/server/src/ws.ts
- docs/user/agents.md
- apps/web/src/components/settings/RulesSettings.tsx
- apps/server/src/orchestration/Layers/ProviderCommandReactor.test.ts
- apps/mobile/src/features/settings/agentProfile.logic.ts
- apps/web/src/components/settings/AgentsSettings.tsx
- apps/mobile/src/features/settings/agentRule.logic.ts
- apps/server/src/agents/AgentCatalog.test.ts
- apps/server/src/persistence/Migrations/039_AgentRuns.ts
- apps/web/src/components/settings/RulesSettings.logic.ts
- apps/server/src/agents/run/AgentRun.test.ts
- apps/server/src/agents/run/AgentRun.ts
- apps/server/src/agents/AgentPromptResolver.ts
- apps/server/src/agents/run/AgentRunReactor.ts
Contributor
There was a problem hiding this comment.
One finding on error attribute safety in the new Agent orchestration code. Earlier rounds' findings (standalone *Shape interfaces, missing message getters, invented cause values, duplicated AgentProfileServices layer) look addressed.
Posted via Macroscope — Effect Service Conventions
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/mobile/src/features/settings/SettingsAgentsRouteScreen.tsx (1)
257-295: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winAdd failure handling to the archive and restore handlers.
saveRuleDocumentandsavewrap their awaited command intry/catch/finallyand show an error message.archiveRestoreRuleandarchiveRestoreusetry/finallyonly. Ifcommand(...)rejects instead of returning a failure result, the rejection escapes through() => void archiveRestoreRule(), the user sees no message, and an unhandled promise rejection is produced.Add a
catchbranch that sets the corresponding error state.🛠️ Proposed fix for `archiveRestoreRule`
setRuleNotice(selectedRuleSummary.archivedAt ? "Rule restored." : "Rule archived."); catalog.refresh(); + } catch (caught) { + setRuleError(caught instanceof Error ? caught.message : "The rule could not be updated."); } finally { ruleCommandInFlight.current = false; setRuleCommandPending(false); }Also applies to: 349-387
🤖 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 `@apps/mobile/src/features/settings/SettingsAgentsRouteScreen.tsx` around lines 257 - 295, Update archiveRestoreRule and the corresponding archiveRestore handler to add catch branches around their awaited command calls, setting the appropriate rule error state when a rejection occurs. Preserve the existing early returns, success handling, and finally blocks so command-in-flight state is always cleared.
🧹 Nitpick comments (2)
apps/mobile/src/features/settings/SettingsAgentsRouteScreen.tsx (1)
43-44: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winShare
diagnosticLabelinstead of copying it.The same helper exists in
apps/web/src/components/settings/AgentsSettings.tsxandapps/web/src/components/settings/RulesSettings.tsx. Three copies of one formatting rule will drift. Move the helper into shared client code and import it in all three screens.As per coding guidelines, "shared logic belongs in
packages/client-runtimewhen appropriate."🤖 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 `@apps/mobile/src/features/settings/SettingsAgentsRouteScreen.tsx` around lines 43 - 44, Move the diagnosticLabel helper into shared client code under packages/client-runtime, then import and reuse it in SettingsAgentsRouteScreen, AgentsSettings, and RulesSettings. Remove each local duplicate while preserving the existing diagnostic formatting behavior.Source: Coding guidelines
apps/server/src/orchestration/Layers/ProviderCommandReactor.test.ts (1)
672-678: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the user-visible failure for the rejected profile.
The test verifies that no prompt resolution, session start, or turn send occurs. It does not verify what the user sees. Add a read-model assertion after
harness.drain()that the thread records the incompatibility failure. That protects against a silent-drop regression where the turn is rejected without any activity.As per coding guidelines, "Backend behavior changes must include focused tests for that behavior."
🤖 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 `@apps/server/src/orchestration/Layers/ProviderCommandReactor.test.ts` around lines 672 - 678, Extend the test after harness.drain() to read the thread’s activity/read model and assert it records the user-visible incompatibility failure for the rejected profile. Keep the existing no-op assertions for resolveAgentPrompt, startSession, and sendTurn, and use the test harness’s established read-model accessors and failure representation.Source: Coding guidelines
🤖 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.
Outside diff comments:
In `@apps/mobile/src/features/settings/SettingsAgentsRouteScreen.tsx`:
- Around line 257-295: Update archiveRestoreRule and the corresponding
archiveRestore handler to add catch branches around their awaited command calls,
setting the appropriate rule error state when a rejection occurs. Preserve the
existing early returns, success handling, and finally blocks so
command-in-flight state is always cleared.
---
Nitpick comments:
In `@apps/mobile/src/features/settings/SettingsAgentsRouteScreen.tsx`:
- Around line 43-44: Move the diagnosticLabel helper into shared client code
under packages/client-runtime, then import and reuse it in
SettingsAgentsRouteScreen, AgentsSettings, and RulesSettings. Remove each local
duplicate while preserving the existing diagnostic formatting behavior.
In `@apps/server/src/orchestration/Layers/ProviderCommandReactor.test.ts`:
- Around line 672-678: Extend the test after harness.drain() to read the
thread’s activity/read model and assert it records the user-visible
incompatibility failure for the rejected profile. Keep the existing no-op
assertions for resolveAgentPrompt, startSession, and sendTurn, and use the test
harness’s established read-model accessors and failure representation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 7370eb5b-bfc2-4cbe-9684-93259c1edb44
📒 Files selected for processing (24)
apps/mobile/src/features/settings/SettingsAgentsRouteScreen.tsxapps/server/integration/OrchestrationEngineHarness.integration.tsapps/server/src/agents/AgentCatalog.tsapps/server/src/agents/AgentHookRunner.test.tsapps/server/src/agents/AgentHookRunner.tsapps/server/src/agents/AgentOrchestrationLive.test.tsapps/server/src/agents/AgentOrchestrationLive.tsapps/server/src/agents/AgentPromptResolver.test.tsapps/server/src/agents/AgentPromptResolver.tsapps/server/src/agents/prompt/RuleMatcher.tsapps/server/src/agents/prompt/prompt.test.tsapps/server/src/agents/run/AgentRun.test.tsapps/server/src/agents/run/AgentRun.tsapps/server/src/agents/run/AgentRunDeadlineReactor.test.tsapps/server/src/agents/run/AgentRunDeadlineReactor.tsapps/server/src/agents/run/AgentRunReactor.test.tsapps/server/src/agents/run/AgentRunReactor.tsapps/server/src/orchestration/Layers/ProviderCommandReactor.test.tsapps/server/src/orchestration/Layers/ProviderCommandReactor.tsapps/server/src/ws.tsapps/web/src/components/settings/AgentsSettings.tsxapps/web/src/components/settings/RulesSettings.tsxpackages/contracts/src/agents.test.tspackages/contracts/src/agents.ts
🚧 Files skipped from review as they are similar to previous changes (14)
- apps/server/integration/OrchestrationEngineHarness.integration.ts
- apps/server/src/agents/prompt/prompt.test.ts
- apps/server/src/agents/AgentHookRunner.test.ts
- apps/server/src/ws.ts
- apps/server/src/agents/run/AgentRun.ts
- apps/server/src/agents/AgentHookRunner.ts
- apps/web/src/components/settings/AgentsSettings.tsx
- apps/server/src/agents/run/AgentRunDeadlineReactor.ts
- apps/server/src/agents/prompt/RuleMatcher.ts
- apps/server/src/agents/run/AgentRun.test.ts
- apps/server/src/agents/AgentPromptResolver.ts
- apps/web/src/components/settings/RulesSettings.tsx
- apps/server/src/orchestration/Layers/ProviderCommandReactor.ts
- packages/contracts/src/agents.ts
megascan
force-pushed
the
agent/native-agent-orchestration
branch
from
August 7, 2026 21:06
0dd2d15 to
e8bc73b
Compare
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/server/src/provider/Layers/OpenCodeAdapter.ts (1)
1714-1719: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winRebuild the OpenCode adapter when provider settings change.
OpenCodeDriver.createcallsmakeOpenCodeAdapteronce, whilemakeManagedServerProvideronly updates the snapshot/settings stream. IfserverUrlchanges,startSessioncan target the new URL whilecapabilities.agentRuntimestill reports the URL captured at driver creation.🤖 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 `@apps/server/src/provider/Layers/OpenCodeAdapter.ts` around lines 1714 - 1719, Update the OpenCode adapter lifecycle around makeOpenCodeAdapter and makeManagedServerProvider so the adapter is recreated whenever provider settings, especially serverUrl, change rather than only updating the snapshot/settings stream. Ensure startSession and capabilities.agentRuntime both use the current settings and remain consistent after a URL change.
🧹 Nitpick comments (20)
apps/server/src/agents/prompt/RuleMatcher.ts (1)
69-99: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDocument that
normalizeWorkspaceRelativePaththrows.The function is exported and throws
AgentRulePathErroron four separate branches. The doc comment describes the normalization but does not state the failure mode. An external caller that omits atry/catchwill surface an uncaught error.Add the throw contract to the comment.
🤖 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 `@apps/server/src/agents/prompt/RuleMatcher.ts` around lines 69 - 99, Update the doc comment for normalizeWorkspaceRelativePath to explicitly state that it throws AgentRulePathError when the supplied path is invalid, while preserving the existing normalization description and implementation.apps/server/src/agents/AgentProjectFileCoordinator.ts (2)
35-38: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winDocument that
workspaceRootmust already be canonical.The lock key is the raw string. Two spellings of the same directory, for example a symlink path and its
realPath, map to two different semaphores and give no mutual exclusion.AgentProfileStore.writeProjectReferenceresolvesrealPathbefore it calls this function, but the precondition is not stated anywhere.State the precondition on the service method, or normalize the key inside
lockFor.♻️ Proposed change
export class AgentProjectFileCoordinator extends Context.Service< AgentProjectFileCoordinator, { + /** + * Serialize `effect` against other holders of the same workspace lock. + * Callers must pass a canonicalized root (for example the result of + * `FileSystem.realPath`); the lock key is compared as a plain string. + */ readonly withWorkspaceLock: <A, E, R>(🤖 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 `@apps/server/src/agents/AgentProjectFileCoordinator.ts` around lines 35 - 38, Document on the withWorkspaceLock service method that workspaceRoot must already be canonical (realPath-resolved) before use, preserving the existing lockFor keying behavior; alternatively, normalize workspaceRoot inside lockFor so equivalent directory spellings share the same semaphore.
17-33: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value
locksgrows without eviction.
lockForinserts oneSemaphoreper distinctworkspaceRootstring and never removes it. The server is long-lived, and the PR adds isolated worktrees that can produce many distinct roots over a session. Each entry is small, so this is not urgent, but the map has no upper bound.Consider keying by a bounded LRU, or removing a lock when it has no waiters.
🤖 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 `@apps/server/src/agents/AgentProjectFileCoordinator.ts` around lines 17 - 33, Update AgentProjectFileCoordinator.make and its lockFor helper so locks are evicted instead of retained indefinitely for every distinct workspaceRoot. Use a bounded LRU or remove entries once their Semaphore has no waiters, while preserving serialization for concurrent operations sharing the same workspaceRoot.apps/server/src/agents/run/AgentRunReactor.ts (1)
213-258: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winLog the discarded
onErrorhook failures and the dropped usage decode.Two failures are currently silent:
- Lines 224, 239, and 250 use
Effect.ignoreonrunTerminalHook(run, "onError"). Keeping the original failure as the run failure is correct, but an operator gets no signal when theonErrorhook itself fails or when its prerequisites are unavailable.- Line 213 uses
decodeUsage, which returns anOption. A payload that does not satisfyRuntimeTaskUsageyieldsNone, and the run is recorded with no usage. Token and cost budgets are then under-counted with no trace.Replace
Effect.ignorewith a log-and-continue, and log a warning when the usage decode returnsNone.♻️ Sketch
- yield* runTerminalHook(run, "onError").pipe(Effect.ignore); + yield* runTerminalHook(run, "onError").pipe( + Effect.catchCause((cause) => + Effect.logWarning("Agent onError hook failed", { runId: run.id, cause }), + ), + );🤖 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 `@apps/server/src/agents/run/AgentRunReactor.ts` around lines 213 - 258, Update the turn-completion, turn-aborted, and runtime-error paths in the AgentRunReactor to log failures from runTerminalHook(run, "onError") while still continuing and preserving the original run failure; replace Effect.ignore with the repository’s established log-and-continue pattern. After decodeUsage, emit a warning when the result is None, then retain the existing optional usage dispatch behavior.apps/server/src/agents/AgentCatalog.ts (1)
592-676: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExtract the repeated
AgentCatalogDocumentErrormapper.The same
Effect.mapErrorblock that builds anAgentCatalogDocumentErrorwithcode: "invalid-document"appears six times acrossprofileSummary,ruleSummary,profileDocument, andruleDocument. A single helper reduces the repetition and keeps the error shape consistent.♻️ Sketch: one shared mapper
+ const invalidDocument = (kind: CatalogEntryKind, source: Source) => (cause: unknown) => + new AgentCatalogDocumentError({ + kind, + scope: source.ref.scope, + id: source.ref.id, + sourcePath: source.sourcePath, + code: "invalid-document", + cause, + });Each loader then uses
Effect.mapError(invalidDocument("profile", source)).🤖 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 `@apps/server/src/agents/AgentCatalog.ts` around lines 592 - 676, Extract the repeated AgentCatalogDocumentError construction into a shared invalidDocument mapper helper that accepts the document kind and source, preserves the existing scope, id, sourcePath, code, and cause fields, and returns the mapped error. Replace the duplicated Effect.mapError blocks in profileSummary, ruleSummary, profileDocument, and ruleDocument with this helper.apps/server/src/agents/AgentOrchestrationLive.test.ts (2)
446-460: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDo not point the failure test at the repository working directory.
The test passes
process.cwd()andpath.join(process.cwd(), "apps")as worktree paths. The result then depends on the working directory that the test runner selects and on the presence of anappsdirectory below it.GitFailureLayerstubsProcessRunner, so the paths only need to exist for any filesystem checks. UsemakeTempDirectoryScoped, as the other integration tests in this file do. That removes the dependency on the runner working directory and keeps the test away from the real repository.🤖 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 `@apps/server/src/agents/AgentOrchestrationLive.test.ts` around lines 446 - 460, The isolated-worktree failure test currently depends on the repository working directory and its apps directory. Update the test around “does not expose Git command output in isolated-worktree failure details” to create a scoped temporary directory with makeTempDirectoryScoped, derive both worktree paths from it, and retain the existing GitFailureLayer and assertions.
143-164: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winStrengthen the
minimumBudgetsassertion.Every field of
effectiveParentBudgetis smaller than the matching field ofchildProfileBudget. The expected result is the parent budget unchanged. A function that ignores the first argument and returns the second passes this test. Make one child field smaller than the parent field, so the assertion proves a per-field minimum.♻️ Proposed change
const childProfileBudget = { maxRuns: 8, maxConcurrency: 4, maxDepth: 4, - maxWallTimeMinutes: 30, + maxWallTimeMinutes: 2, maxTotalTokens: 100_000, maxEstimatedCostUsd: 10, }; @@ NodeAssert.deepEqual(minimumBudgets(childProfileBudget, effectiveParentBudget), { ...effectiveParentBudget, + maxWallTimeMinutes: 2, });🤖 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 `@apps/server/src/agents/AgentOrchestrationLive.test.ts` around lines 143 - 164, Strengthen the test for minimumBudgets by making one field in childProfileBudget smaller than the corresponding effectiveParentBudget field, while leaving the other fields larger. Update the expected result so that field uses the child value and the remaining fields retain the parent values, verifying per-field minimum behavior.apps/server/src/agents/AgentRuleStore.test.ts (1)
139-140: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the reference count by parsing t3.json.
The test name states that one checked-in project rule reference is written. The assertion counts raw substring occurrences, which equals 2 only because the id and the path both contain
project-typescript. Parse the file and assert the length of the rules array instead. The same pattern exists inapps/server/src/agents/AgentProfileStore.test.ts.🤖 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 `@apps/server/src/agents/AgentRuleStore.test.ts` around lines 139 - 140, Update the test assertion after reading t3.json to parse the JSON and assert the rules array length is one, rather than counting raw “project-typescript” substring occurrences. Apply the same approach in the corresponding AgentProfileStore test pattern.apps/server/src/agents/prompt/prompt.test.ts (2)
216-247: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReplace the manual try/catch capture with a throwing assertion.
Both tests declare a mutable
errorvariable, wrap the call intry/catch, and then check the captured value.assert.throwsaccepts a predicate and expresses the same contract in one statement. It also fails clearly when the call does not throw, instead of testingundefinedagainst the type guard.♻️ Proposed change
- let error: unknown; - try { - compileAgentRules({ rules: [rule] }, 4); - } catch (caught) { - error = caught; - } - assert.isTrue(isAgentRuleContentOverflowError(error)); - assert.equal((error as { limitBytes?: number }).limitBytes, 4); + assert.throws( + () => compileAgentRules({ rules: [rule] }, 4), + (caught: unknown) => + isAgentRuleContentOverflowError(caught) && caught.limitBytes === 4, + );🤖 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 `@apps/server/src/agents/prompt/prompt.test.ts` around lines 216 - 247, Replace the manual try/catch error capture in both tests with assert.throws calls around compileAgentRules, using a predicate that validates isAgentRuleContentOverflowError and the expected limitBytes value for the first test. Keep the existing successful compilation assertions and byte-count behavior unchanged.
202-214: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueState how this test detects a backtracking regression.
The pattern uses 48 overlapping deep-star groups against a 400-character path. If the matcher regressed to exponential backtracking, this test would not fail on an assertion. It would hang until the runner timeout expires. Add a comment that records this intent, so a later reader does not shrink the input and remove the protection.
🤖 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 `@apps/server/src/agents/prompt/prompt.test.ts` around lines 202 - 214, Add a concise comment above the test case describing that its intentionally large overlapping-wildcard input guards against exponential backtracking, which would cause the test to hang or time out rather than fail an assertion. Keep the existing pattern and path sizes unchanged.apps/server/src/agents/AgentProfileStore.test.ts (1)
211-212: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the reference count by parsing t3.json.
The test name states that the
t3.jsonreference stays singular. The assertion counts raw substring occurrences, which equals 2 only because the id and the path both containproject-reviewer. A format change, such as dropping the id or renaming the file, breaks this test without a behavior change. Parse the file and assert the length of theagentsarray.♻️ Proposed change
- const projectFile = yield* fileSystem.readFileString(path.join(workspace, "t3.json")); - assert.equal((projectFile.match(/project-reviewer/g) ?? []).length, 2); + const projectFile = JSON.parse( + yield* fileSystem.readFileString(path.join(workspace, "t3.json")), + ) as { readonly agents?: ReadonlyArray<{ readonly id: string }> }; + assert.deepEqual( + (projectFile.agents ?? []).map((entry) => entry.id), + ["project-reviewer"], + );🤖 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 `@apps/server/src/agents/AgentProfileStore.test.ts` around lines 211 - 212, Update the t3.json assertion in the relevant test to parse projectFile as JSON and assert the length of its agents array is 1, rather than counting raw “project-reviewer” substring occurrences. Keep the test focused on the singular agent reference behavior.apps/server/src/agents/AgentCatalog.test.ts (2)
353-354: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExplain the expected project-load count.
assert.equal(yield* Ref.get(projectLoads), 2)states the contract that the test name describes, but the value 2 is unexplained. State why onevalidatecall loads the project file twice, for example once for profiles and once for rules. Without that, a future change that makes the count 1 or 3 gives no signal about which behavior regressed.🤖 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 `@apps/server/src/agents/AgentCatalog.test.ts` around lines 353 - 354, Clarify the expected project-load count in the test around validation by documenting why one validate call increments projectLoads twice, specifically identifying the separate profile and rules loads. Keep the assertion at 2 and place the explanation adjacent to the assertion so changes to either load path are distinguishable.
358-438: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExtract the shared 101-entry profile fixture.
Both tests build an identical 101-element profile array. The literal is 15 lines long and repeats every field. Extract one
makeProfiles(count)helper and call it from both tests. This keeps the two bound assertions focused on the bound they verify.🤖 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 `@apps/server/src/agents/AgentCatalog.test.ts` around lines 358 - 438, Extract the duplicated 101-entry profile construction from the two tests into a shared makeProfiles(count) helper near the test cases, preserving the existing AgentProfileSummary fields and generated IDs. Replace each inline profiles Array.from block with makeProfiles(101), leaving the assertions and rule/diagnostic fixtures unchanged.apps/web/src/components/settings/AgentsSettings.logic.ts (2)
233-248: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueShare one sort helper with the rules editor.
sortAgentProfilesandsortAgentRulesinapps/web/src/components/settings/RulesSettings.logic.tsimplement the same ordering: active before archived, environment before project, then name, then id. The generic constraints are identical. Extract one helper and call it from both modules.🤖 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 `@apps/web/src/components/settings/AgentsSettings.logic.ts` around lines 233 - 248, Extract the shared ordering logic from sortAgentProfiles into a reusable sort helper with the same generic constraints, then update both sortAgentProfiles and sortAgentRules to call it. Preserve the ordering of active before archived, environment before project, followed by name and id, and remove the duplicated implementation from the settings modules.
176-207: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winParse each optional field once.
parseOptionalIntegerandparseOptionalNumberrun twice for every optional field: once in the spread condition and once in the value. The label string is also repeated at each call site. A future edit can change one copy and not the other, which then produces a mismatched error message. Compute each value once and spread it conditionally.♻️ Proposed change
+ const sharedWriteConcurrency = parseOptionalInteger( + draft.sharedWriteConcurrency, + "Shared write concurrency", + ); + const maxTotalTokens = parseOptionalInteger(draft.maxTotalTokens, "Maximum total tokens"); + const maxEstimatedCostUsd = parseOptionalNumber( + draft.maxEstimatedCostUsd, + "Maximum estimated cost", + ); const document = { @@ workspace: { mode: draft.workspaceMode, access: draft.workspaceAccess, - ...(parseOptionalInteger(draft.sharedWriteConcurrency, "Shared write concurrency") === - undefined - ? {} - : { - sharedWriteConcurrency: parseOptionalInteger( - draft.sharedWriteConcurrency, - "Shared write concurrency", - ), - }), + ...(sharedWriteConcurrency === undefined ? {} : { sharedWriteConcurrency }), }, @@ - ...(parseOptionalInteger(draft.maxTotalTokens, "Maximum total tokens") === undefined - ? {} - : { maxTotalTokens: parseOptionalInteger(draft.maxTotalTokens, "Maximum total tokens") }), - ...(parseOptionalNumber(draft.maxEstimatedCostUsd, "Maximum estimated cost") === undefined - ? {} - : { - maxEstimatedCostUsd: parseOptionalNumber( - draft.maxEstimatedCostUsd, - "Maximum estimated cost", - ), - }), + ...(maxTotalTokens === undefined ? {} : { maxTotalTokens }), + ...(maxEstimatedCostUsd === undefined ? {} : { maxEstimatedCostUsd }),🤖 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 `@apps/web/src/components/settings/AgentsSettings.logic.ts` around lines 176 - 207, Update the configuration-building logic around the workspace and budgets fields to parse each optional value only once, including sharedWriteConcurrency, maxTotalTokens, and maxEstimatedCostUsd. Store each parsed result using the existing labels, then conditionally spread the corresponding property only when its value is defined, ensuring validation and error messages cannot diverge.apps/web/src/components/settings/RulesSettings.test.tsx (2)
1-26: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover the archive control that the test name claims.
The test name mentions "reversible lifecycle controls". The rendered editor is created with
isNewset, soRuleEditordoes not render the Archive or Restore button. The assertions only checkSave. Add a case withisNew={false}andarchivedboth false and true, and assert theArchiveandRestorelabels. This matches the guideline that every way in has a corresponding way out and a way to see the state.As per coding guidelines: "For stateful features, provide reverse transitions and visibility: every way in must have the corresponding way out and a way to see the state".
🤖 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 `@apps/web/src/components/settings/RulesSettings.test.tsx` around lines 1 - 26, Expand the “renders file targeting and reversible lifecycle controls” test to render RuleEditor with isNew={false} in both archived={false} and archived={true} states, asserting the corresponding “Archive” and “Restore” labels while preserving the existing field and Save assertions.Source: Coding guidelines
44-46: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDo not depend on the rendered attribute order.
These regexes require
disabled=""to appear beforearia-labelin the same tag. The order comes from the prop order inside theInputandTextareawrappers inapps/web/src/components/settings/RulesSettings.tsx. If a wrapper spreads props differently, or a new prop is added, the assertions fail while the disabled behavior stays correct. Match the element by itsaria-labelfirst, then assert that the same tag containsdisabled.♻️ Proposed change
- expect(markup).toMatch(/<input[^>]*disabled=""[^>]*aria-label="Rule name"/); - expect(markup).toMatch(/<textarea[^>]*disabled=""[^>]*aria-label="Rule file globs"/); - expect(markup).toMatch(/<textarea[^>]*disabled=""[^>]*aria-label="Rule instructions"/); + const disabledTag = (tag: string, label: string) => + new RegExp(`<${tag}(?=[^>]*aria-label="${label}")(?=[^>]*disabled="")[^>]*>`); + expect(markup).toMatch(disabledTag("input", "Rule name")); + expect(markup).toMatch(disabledTag("textarea", "Rule file globs")); + expect(markup).toMatch(disabledTag("textarea", "Rule instructions"));🤖 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 `@apps/web/src/components/settings/RulesSettings.test.tsx` around lines 44 - 46, Update the markup assertions in the RulesSettings tests to locate each input or textarea by its aria-label first, then verify that the same element contains disabled, without relying on attribute ordering.apps/server/src/agents/run/AgentRunDeadlineReactor.test.ts (1)
76-97: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid the
ascasts on the repository and provider fakes.
repositoryForbuilds a full object literal, then casts it withas AgentRunRepository["Service"]. The cast hides any drift between the fake and the real interface. If a method is added or a signature changes, the compiler stays silent and the test keeps passing against a stale contract. Type the literal directly withsatisfiesor an explicit return type, and remove the cast.providerForusesas unknown as ProviderServiceShape, which is broader still; a partial helper with onePick<ProviderServiceShape, "interruptTurn">cast keeps the surface narrow.As per coding guidelines: "Prefer inferred types over explicit annotations and do not use
any."🤖 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 `@apps/server/src/agents/run/AgentRunDeadlineReactor.test.ts` around lines 76 - 97, Update repositoryFor and providerFor to avoid broad as casts: make the repository object satisfy AgentRunRepository["Service"] directly so interface drift is checked, and narrow the provider fake to Pick<ProviderServiceShape, "interruptTurn"> rather than casting through unknown. Preserve the existing helper behavior while relying on inferred types where possible.Source: Coding guidelines
apps/mobile/src/features/settings/agentRule.logic.test.ts (1)
48-66: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the parsed target instead of only a throw.
The test name states that the full target after the first colon is preserved. The second assertion only checks that
buildAgentRuleDocumentthrows. It does not show which value was parsed, and it passes for any error, including an unrelated one. Assert the thrown message, or build a valid document and assertrule.profiles.♻️ Proposed change
expect(() => buildAgentRuleDocument( { ...draftFromRule(), id: "target", name: "Target", profiles: "environment:reviewer:truncated", }, null, ), - ).toThrow(); + ).toThrow("reviewer:truncated");🤖 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 `@apps/mobile/src/features/settings/agentRule.logic.test.ts` around lines 48 - 66, Update the second assertion in the buildAgentRuleDocument test to verify the parsed target value after the first colon, rather than only asserting that an error is thrown. Use a valid input or assert the specific result/error message so the test confirms profiles preserves “reviewer:truncated” and cannot pass due to an unrelated failure.apps/web/src/components/settings/RulesSettings.logic.ts (1)
62-77: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low valueDrop the invented revision value from new-document builders.
AgentRuleStore.saveandAgentProfileStore.savecompare onlyexpectedRevision;baseline === nullalready prevents a conflict and causes a “new” error when an existing document is found. Returningrevision: "a".repeat(64)makes new documents carry a valid revision that can collide later. Use a shared sentinel or omit the field in bothbuildAgentRuleDocumentandbuildAgentProfileDocument.🤖 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 `@apps/web/src/components/settings/RulesSettings.logic.ts` around lines 62 - 77, Update buildAgentRuleDocument in apps/web/src/components/settings/RulesSettings.logic.ts (lines 62-77) and buildAgentProfileDocument in apps/web/src/components/settings/AgentsSettings.logic.ts (lines 158-164) to stop assigning "a".repeat(64) as the revision for new documents. Omit revision or use the shared sentinel consistently in both builders, while preserving baseline revisions for existing documents.
🤖 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 `@apps/server/src/agents/run/AgentRunReactor.ts`:
- Around line 104-131: Ensure the successful-run flow around
completeSuccessfulRun records a durable typed receipt when input.afterResult
completes, before dispatching command, and consults that receipt on retries to
skip re-executing the hook. Use the existing event-sourced receipt pattern and
preserve the current failure dispatch behavior for hook and budget-exhaustion
failures.
- Around line 23-27: Bound TERMINAL_EVENT_RETRY_SCHEDULE with a finite retry or
elapsed-time limit before it is consumed by Effect.catchCause, while preserving
its existing exponential backoff and 30-second maximum delay. Ensure permanently
failing provider events exhaust this budget and are skipped so the sequential
runForEach fiber can continue before the scheduled stop.
In `@apps/web/src/components/settings/RulesSettings.logic.ts`:
- Around line 53-61: Update the rule-save logic around decodeAgentRuleDocument
to catch decode failures and rethrow a readable error identifying the invalid
field, while preserving existing validation messages. Add an explicit priority
range check after the integer check so values outside -100 through 100 are
rejected before schema decoding. Apply the same readable decode-error handling
and priority-range validation to the profile builder in AgentsSettings.logic.ts.
---
Outside diff comments:
In `@apps/server/src/provider/Layers/OpenCodeAdapter.ts`:
- Around line 1714-1719: Update the OpenCode adapter lifecycle around
makeOpenCodeAdapter and makeManagedServerProvider so the adapter is recreated
whenever provider settings, especially serverUrl, change rather than only
updating the snapshot/settings stream. Ensure startSession and
capabilities.agentRuntime both use the current settings and remain consistent
after a URL change.
---
Nitpick comments:
In `@apps/mobile/src/features/settings/agentRule.logic.test.ts`:
- Around line 48-66: Update the second assertion in the buildAgentRuleDocument
test to verify the parsed target value after the first colon, rather than only
asserting that an error is thrown. Use a valid input or assert the specific
result/error message so the test confirms profiles preserves
“reviewer:truncated” and cannot pass due to an unrelated failure.
In `@apps/server/src/agents/AgentCatalog.test.ts`:
- Around line 353-354: Clarify the expected project-load count in the test
around validation by documenting why one validate call increments projectLoads
twice, specifically identifying the separate profile and rules loads. Keep the
assertion at 2 and place the explanation adjacent to the assertion so changes to
either load path are distinguishable.
- Around line 358-438: Extract the duplicated 101-entry profile construction
from the two tests into a shared makeProfiles(count) helper near the test cases,
preserving the existing AgentProfileSummary fields and generated IDs. Replace
each inline profiles Array.from block with makeProfiles(101), leaving the
assertions and rule/diagnostic fixtures unchanged.
In `@apps/server/src/agents/AgentCatalog.ts`:
- Around line 592-676: Extract the repeated AgentCatalogDocumentError
construction into a shared invalidDocument mapper helper that accepts the
document kind and source, preserves the existing scope, id, sourcePath, code,
and cause fields, and returns the mapped error. Replace the duplicated
Effect.mapError blocks in profileSummary, ruleSummary, profileDocument, and
ruleDocument with this helper.
In `@apps/server/src/agents/AgentOrchestrationLive.test.ts`:
- Around line 446-460: The isolated-worktree failure test currently depends on
the repository working directory and its apps directory. Update the test around
“does not expose Git command output in isolated-worktree failure details” to
create a scoped temporary directory with makeTempDirectoryScoped, derive both
worktree paths from it, and retain the existing GitFailureLayer and assertions.
- Around line 143-164: Strengthen the test for minimumBudgets by making one
field in childProfileBudget smaller than the corresponding effectiveParentBudget
field, while leaving the other fields larger. Update the expected result so that
field uses the child value and the remaining fields retain the parent values,
verifying per-field minimum behavior.
In `@apps/server/src/agents/AgentProfileStore.test.ts`:
- Around line 211-212: Update the t3.json assertion in the relevant test to
parse projectFile as JSON and assert the length of its agents array is 1, rather
than counting raw “project-reviewer” substring occurrences. Keep the test
focused on the singular agent reference behavior.
In `@apps/server/src/agents/AgentProjectFileCoordinator.ts`:
- Around line 35-38: Document on the withWorkspaceLock service method that
workspaceRoot must already be canonical (realPath-resolved) before use,
preserving the existing lockFor keying behavior; alternatively, normalize
workspaceRoot inside lockFor so equivalent directory spellings share the same
semaphore.
- Around line 17-33: Update AgentProjectFileCoordinator.make and its lockFor
helper so locks are evicted instead of retained indefinitely for every distinct
workspaceRoot. Use a bounded LRU or remove entries once their Semaphore has no
waiters, while preserving serialization for concurrent operations sharing the
same workspaceRoot.
In `@apps/server/src/agents/AgentRuleStore.test.ts`:
- Around line 139-140: Update the test assertion after reading t3.json to parse
the JSON and assert the rules array length is one, rather than counting raw
“project-typescript” substring occurrences. Apply the same approach in the
corresponding AgentProfileStore test pattern.
In `@apps/server/src/agents/prompt/prompt.test.ts`:
- Around line 216-247: Replace the manual try/catch error capture in both tests
with assert.throws calls around compileAgentRules, using a predicate that
validates isAgentRuleContentOverflowError and the expected limitBytes value for
the first test. Keep the existing successful compilation assertions and
byte-count behavior unchanged.
- Around line 202-214: Add a concise comment above the test case describing that
its intentionally large overlapping-wildcard input guards against exponential
backtracking, which would cause the test to hang or time out rather than fail an
assertion. Keep the existing pattern and path sizes unchanged.
In `@apps/server/src/agents/prompt/RuleMatcher.ts`:
- Around line 69-99: Update the doc comment for normalizeWorkspaceRelativePath
to explicitly state that it throws AgentRulePathError when the supplied path is
invalid, while preserving the existing normalization description and
implementation.
In `@apps/server/src/agents/run/AgentRunDeadlineReactor.test.ts`:
- Around line 76-97: Update repositoryFor and providerFor to avoid broad as
casts: make the repository object satisfy AgentRunRepository["Service"] directly
so interface drift is checked, and narrow the provider fake to
Pick<ProviderServiceShape, "interruptTurn"> rather than casting through unknown.
Preserve the existing helper behavior while relying on inferred types where
possible.
In `@apps/server/src/agents/run/AgentRunReactor.ts`:
- Around line 213-258: Update the turn-completion, turn-aborted, and
runtime-error paths in the AgentRunReactor to log failures from
runTerminalHook(run, "onError") while still continuing and preserving the
original run failure; replace Effect.ignore with the repository’s established
log-and-continue pattern. After decodeUsage, emit a warning when the result is
None, then retain the existing optional usage dispatch behavior.
In `@apps/web/src/components/settings/AgentsSettings.logic.ts`:
- Around line 233-248: Extract the shared ordering logic from sortAgentProfiles
into a reusable sort helper with the same generic constraints, then update both
sortAgentProfiles and sortAgentRules to call it. Preserve the ordering of active
before archived, environment before project, followed by name and id, and remove
the duplicated implementation from the settings modules.
- Around line 176-207: Update the configuration-building logic around the
workspace and budgets fields to parse each optional value only once, including
sharedWriteConcurrency, maxTotalTokens, and maxEstimatedCostUsd. Store each
parsed result using the existing labels, then conditionally spread the
corresponding property only when its value is defined, ensuring validation and
error messages cannot diverge.
In `@apps/web/src/components/settings/RulesSettings.logic.ts`:
- Around line 62-77: Update buildAgentRuleDocument in
apps/web/src/components/settings/RulesSettings.logic.ts (lines 62-77) and
buildAgentProfileDocument in
apps/web/src/components/settings/AgentsSettings.logic.ts (lines 158-164) to stop
assigning "a".repeat(64) as the revision for new documents. Omit revision or use
the shared sentinel consistently in both builders, while preserving baseline
revisions for existing documents.
In `@apps/web/src/components/settings/RulesSettings.test.tsx`:
- Around line 1-26: Expand the “renders file targeting and reversible lifecycle
controls” test to render RuleEditor with isNew={false} in both archived={false}
and archived={true} states, asserting the corresponding “Archive” and “Restore”
labels while preserving the existing field and Save assertions.
- Around line 44-46: Update the markup assertions in the RulesSettings tests to
locate each input or textarea by its aria-label first, then verify that the same
element contains disabled, without relying on attribute ordering.
🪄 Autofix
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: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f8de1563-10e9-45fe-b5bc-748b6a40b4d9
📒 Files selected for processing (52)
apps/mobile/src/features/settings/SettingsAgentsRouteScreen.tsxapps/mobile/src/features/settings/agentProfile.logic.test.tsapps/mobile/src/features/settings/agentProfile.logic.tsapps/mobile/src/features/settings/agentRule.logic.test.tsapps/mobile/src/features/settings/agentRule.logic.tsapps/mobile/src/features/settings/agentSettings.logic.test.tsapps/mobile/src/features/settings/agentSettings.logic.tsapps/mobile/src/features/threads/NewTaskDraftScreen.tsxapps/mobile/src/features/threads/ThreadComposer.tsxapps/server/src/agents/AgentCatalog.test.tsapps/server/src/agents/AgentCatalog.tsapps/server/src/agents/AgentOrchestrationLive.test.tsapps/server/src/agents/AgentOrchestrationLive.tsapps/server/src/agents/AgentProfileServices.tsapps/server/src/agents/AgentProfileStore.test.tsapps/server/src/agents/AgentProfileStore.tsapps/server/src/agents/AgentProjectFileCoordinator.tsapps/server/src/agents/AgentRuleStore.test.tsapps/server/src/agents/AgentRuleStore.tsapps/server/src/agents/AgentStoreErrorMapping.test.tsapps/server/src/agents/AgentStoreErrorMapping.tsapps/server/src/agents/AgentWorkspaceRoot.test.tsapps/server/src/agents/AgentWorkspaceRoot.tsapps/server/src/agents/prompt/RuleMatcher.tsapps/server/src/agents/prompt/prompt.test.tsapps/server/src/agents/run/AgentRun.test.tsapps/server/src/agents/run/AgentRun.tsapps/server/src/agents/run/AgentRunDeadlineReactor.test.tsapps/server/src/agents/run/AgentRunDeadlineReactor.tsapps/server/src/agents/run/AgentRunReactor.test.tsapps/server/src/agents/run/AgentRunReactor.tsapps/server/src/provider/Layers/CursorAdapter.test.tsapps/server/src/provider/Layers/CursorAdapter.tsapps/server/src/provider/Layers/GrokAdapter.test.tsapps/server/src/provider/Layers/GrokAdapter.tsapps/server/src/provider/Layers/OpenCodeAdapter.test.tsapps/server/src/provider/Layers/OpenCodeAdapter.tsapps/server/src/ws.tsapps/web/src/components/chat/AgentProfilePicker.logic.tsapps/web/src/components/chat/AgentProfilePicker.test.tsapps/web/src/components/chat/AgentProfilePicker.tsxapps/web/src/components/chat/ChatComposer.logic.test.tsapps/web/src/components/chat/ChatComposer.logic.tsapps/web/src/components/chat/ChatComposer.tsxapps/web/src/components/settings/AgentsSettings.logic.test.tsapps/web/src/components/settings/AgentsSettings.logic.tsapps/web/src/components/settings/AgentsSettings.tsxapps/web/src/components/settings/RulesSettings.logic.test.tsapps/web/src/components/settings/RulesSettings.logic.tsapps/web/src/components/settings/RulesSettings.test.tsxapps/web/src/components/settings/RulesSettings.tsxpackages/contracts/src/agents.ts
🚧 Files skipped from review as they are similar to previous changes (21)
- apps/server/src/provider/Layers/CursorAdapter.ts
- apps/mobile/src/features/threads/NewTaskDraftScreen.tsx
- apps/server/src/agents/AgentProfileServices.ts
- apps/server/src/provider/Layers/GrokAdapter.ts
- apps/web/src/components/settings/RulesSettings.logic.test.ts
- apps/mobile/src/features/settings/agentProfile.logic.ts
- apps/mobile/src/features/settings/agentSettings.logic.ts
- apps/mobile/src/features/settings/agentProfile.logic.test.ts
- apps/server/src/agents/run/AgentRun.test.ts
- apps/web/src/components/chat/AgentProfilePicker.test.ts
- apps/server/src/agents/run/AgentRunDeadlineReactor.ts
- apps/web/src/components/settings/AgentsSettings.tsx
- apps/mobile/src/features/threads/ThreadComposer.tsx
- apps/server/src/agents/run/AgentRun.ts
- packages/contracts/src/agents.ts
- apps/mobile/src/features/settings/SettingsAgentsRouteScreen.tsx
- apps/web/src/components/settings/RulesSettings.tsx
- apps/server/src/ws.ts
- apps/web/src/components/chat/ChatComposer.tsx
- apps/server/src/agents/AgentRuleStore.ts
- apps/server/src/agents/AgentOrchestrationLive.ts
Author
|
@coderabbitai review |
|
Author
|
@coderabbitai review |
|
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@apps/mobile/src/features/settings/SettingsAgentsRouteScreen.tsx`:
- Around line 569-574: Update the rules catalog rendering near the rules.length
check in SettingsAgentsRouteScreen to handle loading and error states before
showing the empty-catalog message. Mirror the existing profile catalog guards
around lines 509-521, using the rules catalog’s loading and error state symbols,
and preserve “No rules yet” only for a successfully loaded empty result.
- Around line 839-851: Update the Always apply Pressable to use an off-state
background when props.draft.alwaysApply is false, while preserving the primary
background when enabled. Add an accessibilityLabel identifying it as the “Always
apply” switch, matching the accessible labeling used by the nearby Show in chat
Agent picker switch.
- Around line 287-320: Add a catch handler to archiveRestoreRule, and likewise
to archiveRestore, so rejected command promises set an appropriate user-facing
rule error while preserving the existing context-key guard. Keep the finally
blocks responsible for clearing ruleCommandInFlight and ruleCommandPending,
ensuring void archiveRestoreRule() and the corresponding archiveRestore call do
not produce unhandled rejections.
In `@apps/web/src/components/settings/RulesSettings.tsx`:
- Around line 127-182: In the RulesSettings mutation flow, add shared isMutating
state used by both save and archiveRestore, set it before either command starts,
and clear it in a finally block so it resets on success or failure. Pass
isMutating to RuleEditor and disable both Save and Archive/Restore actions while
it is true, preventing concurrent submissions with stale expectedRevision
values.
🪄 Autofix
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: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 778bef4d-4da2-4296-a024-f1fdc2fefd7c
📒 Files selected for processing (52)
apps/mobile/src/features/settings/SettingsAgentsRouteScreen.tsxapps/mobile/src/features/settings/agentProfile.logic.test.tsapps/mobile/src/features/settings/agentProfile.logic.tsapps/mobile/src/features/settings/agentRule.logic.test.tsapps/mobile/src/features/settings/agentRule.logic.tsapps/mobile/src/features/settings/agentSettings.logic.test.tsapps/mobile/src/features/settings/agentSettings.logic.tsapps/mobile/src/features/threads/NewTaskDraftScreen.tsxapps/mobile/src/features/threads/ThreadComposer.tsxapps/server/src/agents/AgentCatalog.test.tsapps/server/src/agents/AgentCatalog.tsapps/server/src/agents/AgentOrchestrationLive.test.tsapps/server/src/agents/AgentOrchestrationLive.tsapps/server/src/agents/AgentProfileServices.tsapps/server/src/agents/AgentProfileStore.test.tsapps/server/src/agents/AgentProfileStore.tsapps/server/src/agents/AgentProjectFileCoordinator.tsapps/server/src/agents/AgentRuleStore.test.tsapps/server/src/agents/AgentRuleStore.tsapps/server/src/agents/AgentStoreErrorMapping.test.tsapps/server/src/agents/AgentStoreErrorMapping.tsapps/server/src/agents/AgentWorkspaceRoot.test.tsapps/server/src/agents/AgentWorkspaceRoot.tsapps/server/src/agents/prompt/RuleMatcher.tsapps/server/src/agents/prompt/prompt.test.tsapps/server/src/agents/run/AgentRun.test.tsapps/server/src/agents/run/AgentRun.tsapps/server/src/agents/run/AgentRunDeadlineReactor.test.tsapps/server/src/agents/run/AgentRunDeadlineReactor.tsapps/server/src/agents/run/AgentRunReactor.test.tsapps/server/src/agents/run/AgentRunReactor.tsapps/server/src/provider/Layers/CursorAdapter.test.tsapps/server/src/provider/Layers/CursorAdapter.tsapps/server/src/provider/Layers/GrokAdapter.test.tsapps/server/src/provider/Layers/GrokAdapter.tsapps/server/src/provider/Layers/OpenCodeAdapter.test.tsapps/server/src/provider/Layers/OpenCodeAdapter.tsapps/server/src/ws.tsapps/web/src/components/chat/AgentProfilePicker.logic.tsapps/web/src/components/chat/AgentProfilePicker.test.tsapps/web/src/components/chat/AgentProfilePicker.tsxapps/web/src/components/chat/ChatComposer.logic.test.tsapps/web/src/components/chat/ChatComposer.logic.tsapps/web/src/components/chat/ChatComposer.tsxapps/web/src/components/settings/AgentsSettings.logic.test.tsapps/web/src/components/settings/AgentsSettings.logic.tsapps/web/src/components/settings/AgentsSettings.tsxapps/web/src/components/settings/RulesSettings.logic.test.tsapps/web/src/components/settings/RulesSettings.logic.tsapps/web/src/components/settings/RulesSettings.test.tsxapps/web/src/components/settings/RulesSettings.tsxpackages/contracts/src/agents.ts
🚧 Files skipped from review as they are similar to previous changes (41)
- apps/server/src/agents/AgentWorkspaceRoot.test.ts
- apps/server/src/agents/AgentStoreErrorMapping.test.ts
- apps/server/src/provider/Layers/CursorAdapter.test.ts
- apps/server/src/provider/Layers/OpenCodeAdapter.test.ts
- apps/server/src/provider/Layers/CursorAdapter.ts
- apps/mobile/src/features/settings/agentRule.logic.test.ts
- apps/web/src/components/chat/ChatComposer.logic.test.ts
- apps/server/src/agents/run/AgentRunDeadlineReactor.test.ts
- apps/server/src/agents/AgentWorkspaceRoot.ts
- apps/server/src/agents/AgentProfileStore.test.ts
- apps/server/src/provider/Layers/GrokAdapter.test.ts
- apps/server/src/agents/run/AgentRun.test.ts
- apps/mobile/src/features/threads/NewTaskDraftScreen.tsx
- apps/server/src/agents/AgentCatalog.test.ts
- apps/mobile/src/features/settings/agentRule.logic.ts
- apps/server/src/agents/AgentStoreErrorMapping.ts
- apps/web/src/components/settings/RulesSettings.test.tsx
- apps/server/src/provider/Layers/GrokAdapter.ts
- apps/server/src/agents/AgentProjectFileCoordinator.ts
- apps/server/src/agents/prompt/prompt.test.ts
- apps/mobile/src/features/threads/ThreadComposer.tsx
- apps/server/src/provider/Layers/OpenCodeAdapter.ts
- apps/web/src/components/chat/AgentProfilePicker.logic.ts
- apps/server/src/agents/AgentProfileStore.ts
- apps/server/src/ws.ts
- apps/web/src/components/settings/RulesSettings.logic.ts
- apps/server/src/agents/prompt/RuleMatcher.ts
- apps/server/src/agents/AgentRuleStore.ts
- apps/server/src/agents/run/AgentRunDeadlineReactor.ts
- apps/mobile/src/features/settings/agentProfile.logic.ts
- apps/server/src/agents/AgentRuleStore.test.ts
- apps/web/src/components/chat/AgentProfilePicker.test.ts
- apps/web/src/components/chat/ChatComposer.tsx
- apps/server/src/agents/run/AgentRun.ts
- apps/web/src/components/settings/AgentsSettings.tsx
- apps/server/src/agents/AgentProfileServices.ts
- packages/contracts/src/agents.ts
- apps/web/src/components/chat/AgentProfilePicker.tsx
- apps/server/src/agents/AgentOrchestrationLive.test.ts
- apps/server/src/agents/run/AgentRunReactor.ts
- apps/server/src/agents/AgentOrchestrationLive.ts
megascan
marked this pull request as ready for review
August 7, 2026 22:15
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want fixes drafted automatically? Bugbot Autofix can create code changes for findings. A team admin can enable Autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit c2bc3bf. Configure here.
Contributor
ApprovabilityVerdict: Needs human review Diff is too large for automated approval analysis. A human reviewer should evaluate this PR. You can customize Macroscope's approvability policy. Learn more. |
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.
What Changed This PR adds a provider-neutral Agents system to T3 Code. It is one end-to-end feature, but it is intentionally not a small change: the implementation crosses contracts, persistence, server orchestration, MCP, all five provider adapters, web/desktop, mobile, tests, migrations, and documentation. The diff is 136 files, 17,408 additions, and 84 deletions. The implementation is ready for human review after automated review hardening. Suggested labels:
### After: first-party profile and Rule management The selected specialist is marked delegation only; the host identifier is redacted from this public evidence image. 
### After: searchable picker beside the model picker Search filters the direct-chat profiles immediately. The delegation-only Terra specialist is intentionally absent while remaining available to orchestration through 
### After: direct Agent selection is visible beside the model 
### After: file-aware Rules use the same settings language 
Short picker interaction recording Evidence is published on a separate fork branch so binary review artifacts do not enter this product diff. ## Verification - Original feature matrix over 28 focused files: 185/185 tests passed. - First post-review regression matrix over 21 focused files: 116/116 tests passed. - CodeRabbit response matrix over every modified test file: 105/105 tests passed. - Third review-hardening matrix over every newly modified test file: 94/94 tests passed. - Latest reviewer-response matrix across mobile, catalog, stores, Rules, prompt matching, orchestration, budgets, and terminal hooks: 45/45 tests passed. - Recovery-hardening runtime matrix across orchestration, deadlines, event retry, persistence compensation, Rules, and shared project-file coordination: 57/57 tests passed. - Latest web/mobile selection and compare-and-swap matrix: 29/29 tests passed. - Truthful provider-capability regressions for OpenCode, Cursor, and Grok: 3/3 tests passed. - Latest event-ordering, single-execution hook retry, async settings context, and active draft picker matrix: 43/43 tests passed. - Cross-surface readable settings validation matrix: 29/29 tests passed.
enhancement,documentation,size:XXL,needs-triage,preview:web, andcodex. The fork author does not have upstream triage permission, so GitHub rejected applying them directly. ### Agent profiles and Rules - Adds typed Agent profile and Rule contracts, including environment/project scope, content-addressed profile revisions, model selection, instructions, runtime/workspace/tool policy, hooks, delegation allowlists, provider requirements, and bounded budgets. - Stores environment profiles under T3-owned state and project profiles as explicitt3.jsonreferences. Project documents are contained to the canonical project root; the catalog does not recursively scan repositories. - Saves Markdown/YAML documents with compare-and-swap revision checks, atomic replacement, archive/restore, duplicate diagnostics, and backward-compatible decoding. - Adds deterministic Rules that can always apply, match workspace-relative file globs, or attach explicitly to profiles. - AddschatSelectable(defaulttruefor existing documents). Turning it off makes a specialist “delegation only”: it disappears from new top-level chat choices but remains available toagent_listand orchestration. This is discovery policy, not authorization. ### Durable orchestration - Models child work as ordinary T3 threads with a pinned profile revision, not provider-native subagents. This preserves T3 history, remote access, and multi-device visibility. - Adds an event-sourcedAgentRundomain with append-only events, transactional projection updates, immutable profile snapshots, parent-thread/lineage queries, revision-based waiting, deadlines, usage, results, follow-ups, cancellation, and integration state. - Adds migrations 39/40 for Agent run storage and the optional pinned profile on thread projections. These numbers were rebased onto current upstream migration 38. - Enforces hard lineage ceilings: depth 4, concurrency 8, 32 runs, and 120 minutes. Child budgets can reduce but cannot exceed the parent budget. - Adds a race-free wait path: subscribers attach before the durable revision read, so agents can wait for progress without sleep loops or polling. ### T3-owned Agent tools Adds a provider-portable MCP toolkit: -agent_list-agent_spawn-agent_status-agent_wait-agent_result-agent_send-agent_cancel-agent_integrateThe toolkit is scoped from the invoking T3 thread/project. Launch is asynchronous and returns a durable run id; result reads are bounded and paginated. ### Provider-neutral boundary - Adds explicit Agent runtime capability declarations to Codex, Claude, Cursor, Grok, and OpenCode adapters. - Keeps provider-specific complexity at the adapter boundary. Catalog, orchestration, persistence, clients, and MCP handlers do not switch on provider kind. - Uses deny-by-default compatibility checks. A spawn is rejected when the selected adapter cannot honestly enforce a requested guarantee, such as exact native tool restriction, system-level instruction delivery, or requested usage accounting. - Future providers join the same path by declaring capabilities and supporting the existing T3 MCP injection boundary; they do not require a new Agent implementation. ### Shared and isolated workspaces - Shared runs use the invoking workspace and obey profile write-concurrency policy. - Isolated runs use a separate Git worktree and require explicit integration. - Integration verifies canonical paths and Git common-directory identity, refuses dirty targets and untracked child files, generates a bounded tracked binary patch, preflights it withgit apply --check --3way, then applies it. Conflicts remain visible instead of being guessed through. ### Web, desktop, mobile, and remote behavior - Adds Settings → Agents with environment/project profile management, archive/restore, policy editing, Rules management, and the direct-chat visibility switch. - Adds a searchable Agent picker beside the web/desktop model picker. A currently pinned profile remains visible in its existing thread even if later marked delegation-only. - Adds native mobile settings plus Agent selection in new-task and existing-thread composer flows. Mobile uses the platform menu rather than a text-search popover. - Extends typed WebSocket RPCs and shared client runtime state, so local, LAN, relay, and tunnel clients talk to the host-owned catalog and durable runs rather than reading local files. - Adds user documentation, contributor architecture documentation, and glossary entries. ## Why ACP gives T3 a clean provider transport, but ACP providers do not inherently know they are running inside T3 and cannot reliably orchestrate one another. Provider-native subagent features also differ in naming, policy, lifetime, and availability. That makes a workflow such as “chat with an inexpensive coordinator, delegate architecture to one model, implementation to another, and review to a third” provider-specific and fragile. This design puts orchestration in the layer that actually has the necessary context: T3. It reuses T3 threads, WebSocket contracts, persistence, MCP injection, and provider adapters. The visible Agent is a reusable T3 policy profile; every child remains inspectable as normal T3 work. The result works with current providers without baking OpenGrok/GrokBuild behavior into the core, while giving future providers one explicit compatibility contract. ## UI Changes ### Before: no Agent settings surfaceagent_list.1062149af; Cursor Bugbot found no new issues; all 108/108 review threads are resolved. - Latest-main broad changed-test matrix: 318/320 tests passed. The two failures are unchanged upstream Windows-only limitations: a hard-coded/logsseparator assertion and a symlink test blocked by localEPERM; all other 198 tests outside that server seam passed. - Contracts, shared runtime, server, web, and mobile typechecks: passed. - Whole-PR targeted lint and formatting: passed with zero diagnostics. - Web production build: passed. - Server bundle build: passed. -git diff --check: passed. - Rebased onto currentupstream/main; the feature branch is thirteen commits ahead and zero commits behind. Integrated browser coverage used an isolated.t3development environment and exercised: 1. creating multiple environment Agent profiles and a file-aware Rule; 2. preserving brace globs such assrc/**/*.{ts,tsx}across save and reload; 3. searching and selecting a chat-selectable Agent directly beside the model picker; 4. confirming a delegation-only specialist is excluded from direct-chat search; 5. applying a profile's preferred model when available and falling back safely when unavailable; 6. archiving and restoring both a profile and a Rule without destructive deletion; 7. retaining an archived-but-pinned profile label on its existing durable thread; 8. opening a fresh draft with Choose agent instead of leaking the previous route's selection; 9. preserving the earlier real Grok 4.5 turn and selected profile across watcher restarts. Focused backend coverage additionally exercises prompt injection and pinned-profile continuity, child-run lifecycle and result retrieval, shared orchestration, isolated-worktree integration/refusal, compare-and-swap persistence, deadline handling, and provider capability rejection. ## Honest Scope and Known Limitations - During an early contributor-machine startup, migration 40 was applied to the developer's live T3 home before startup failed. The process was stopped, no feature records were intentionally written there, and every subsequent runtime/UI pass used an isolated.t3home. This affected local contributor state only, not repository or production data. - This is a large architectural feature PR, despite being one product concern. It does not meet the repository preference for small contributions; that checklist item is intentionally left unchecked. - Agents are T3-managed child threads, not a wrapper around provider-native subagent APIs. This is deliberate, but it means provider-native team/agent UIs are not surfaced here. -chatSelectableis not an access-control boundary. Delegation policy and provider compatibility remain the execution gates. - Web/desktop has text search in the picker; mobile currently uses the native platform menu and does not provide text search. - Token and monetary budgets can only be enforced where the adapter reports the required usage. Profiles that require unsupported accounting are rejected rather than approximately enforced. - Isolated integration intentionally rejects untracked child files and dirty targets. It does not invent staging/merge decisions on the user's behalf. - Profile/Rule files are environment-local or repository-referenced. There is no cross-environment profile marketplace, cloud sync, or import/export workflow in this PR. - Catalog RPC responses intentionally cap profiles, Rules, and diagnostics at 100 each to bound WebSocket payloads. Oversized catalogs remain loadable and show an explicit truncation diagnostic, but entries after the first 100 are not selectable in this version. - The integrated UI pass covered web/desktop behavior in the real client. Mobile received focused unit coverage and a full TypeScript check, but was not exercised on a physical device or simulator in this pass. - I did not run a production-scale concurrency soak, relay/tunnel latency test, or accessibility screen-reader audit. The implementation bounds payloads and avoids broadcast profile bodies, but maintainers should treat performance and accessibility review as explicit draft-review work. - The server typecheck emits existing non-failing Effect style suggestions; there are no type errors. - On this Windows host, the full changed-test matrix retains two unrelated upstream test limitations: one assertion hard-codes a POSIX path separator, and one symlink-security test requires privileges not available to the current process. Both failing lines predate this branch and are unchanged by the PR. - The new provider capability assertions pass directly. Running the complete Cursor/Grok adapter files on this Windows host additionally hits their existing Unix .sh mock-wrapper EFTYPE limitation, and the complete OpenCode file hits the same privileged-symlink EPERM limitation noted above; Linux CI remains the authoritative full-adapter run. - Automated review raised terminal-hook cross-run concurrency as performance debt. Hooks intentionally remain inline in this PR becauseafterResultis part of the durable success/failure decision and provider events must remain ordered per thread. A safe improvement needs a keyed bounded scheduler with explicit drain/shutdown semantics; raw forking would make correctness worse. - Agent MCP toolkit groups are exposed to every provider session, but this is not the authorization boundary: spawn requires a selected profile and exact delegation allowlist, while lifecycle operations enforce project, thread, lineage, and run ownership.t3McpCapabilitiesremains a compatibility declaration rather than an ACL. ## Checklist - [ ] This PR is small and focused — it is one focused feature, but the end-to-end implementation is intentionally large. - [x] I explained what changed and why. - [x] I included before/after screenshots for the UI changes. - [x] I included a short video for the picker interaction. Implemented with GPT-5.6 Sol through the Codex harness in T3 Code.Note
Add provider-neutral agent orchestration with profile/rule management UI and MCP toolkit
AgentRunevent-sourced state machine,AgentRunRepository, deadline reactor, andAgentOrchestrationservice interface, persisted via migrations 39–40 (newprojection_agent_runs,agent_run_events, andagent_profile_snapshotstables).AgentProfileStoreandAgentRuleStorewith compare-and-swap semantics, Markdown+YAML frontmatter serialization, per-workspace semaphore locking, and revision conflict errors.AgentCatalogto discover and lazily load profiles and rules from the filesystem, andAgentPromptResolverto compile prompts (with hook execution, rule matching, and context file extraction) before forwarding to the provider.resolveAgentRuntimeCompatibilityto validate provider adapter capabilities against agent profile requirements; incompatible combinations fail early with a structured error inProviderCommandReactor.agentsCatalog,agentsGetProfile/SaveProfile/ArchiveProfile/RestoreProfile,agentsGetRule/SaveRule/ArchiveRule/RestoreRule) and anAgentToolkitMCP server withagent_list/spawn/status/wait/result/send/cancel/integratetools./settings/agents) and mobile (SettingsAgentssheet screen), and integrates anAgentProfilePickerinto the chat composer on both platforms.agentProfilereference through orchestration events, projection, and queued outbox messages; thethread.turn-start-requestedevent can clear the pinned profile.Macroscope summarized 1062149.
Summary by CodeRabbit
Summary by CodeRabbit
Note
Medium Risk
Changes affect turn-start payloads, remote agent document persistence, and new server catalog/hook execution paths; mistakes could alter which instructions/models run or mishandle revision conflicts, though mobile is mostly UI/state over existing RPC contracts.
Overview
Mobile adds a first-party Settings → Agents flow and threads agent profile selection through new-task and existing-thread composers, drafts, and the outbox.
The new Settings Agents screen lets users pick environment/project context, browse profile and rule catalogs (with load/save/archive via remote commands), and edit compact drafts backed by
agentProfile.logic/agentRule.logic(revision checks, schema validation, chat-selectable filtering). Navigation is registered asSettingsAgentswith a Configuration row on the main settings screen.Composer UX adds an Agent control pill next to model/options. It loads the catalog through
useAgentProfileCatalog, lists only chat-selectable profiles (while keeping a pinned delegation-only profile visible), applies a profile’s default model when chosen, and passesagentProfilethroughresolveAgentProfileSelectioninto thread creation,thread.turn.start, queued messages, and persisted composer drafts (including explicit “No agent”).Server (in this diff) introduces
AgentCatalog(Markdown discovery, bounded RPC lists, projectt3.jsonreferences),AgentHookRunner(workspace-contained context/shell hooks with timeouts), andAgentOrchestrationservice surface plus orchestration helpers/tests; the integration harness stubsAgentPromptResolverwhere profiles are unused.Reviewed by Cursor Bugbot for commit 1062149. Bugbot is set up for automated code reviews on this repo. Configure here.