Repository navigation
feat: thread parentAgent through session, task, and chat hooks - #1
Merged
Merged
Conversation
Co-authored-by: opencode-agent[bot] <opencode-agent[bot]@users.noreply.github.com>
…o#30761) Co-authored-by: Shoubhit Dash <shoubhit2005@gmail.com>
Co-authored-by: LukeParkerDev <10430890+Hona@users.noreply.github.com>
Re-applies the parent-agent threading from forked commits eb815ac and e64832d against the current upstream/dev shape: - SessionV1.User gains optional parentAgent field - TaskTool.runTask passes parentAgent: ctx.agent into ops.prompt so spawned subagent sessions know their parent - LLMRequestPrep.prepare forwards input.user.parentAgent into the chat.params and chat.headers plugin trigger inputs (matches the widened hook input types already on this branch) - Adds parent-agent plugin test
- Add parentAgent: Schema.optional(Schema.String) to SessionPrompt.PromptInput and ShellInput so callers (TaskTool.runTask, ops.prompt, shell) can pass it. - Persist input.parentAgent on the SessionV1.User built by createUserMessage and shellImpl, so llm/request.ts and plugin hooks read the real value instead of undefined. - Rewrite parent-agent.test.ts against current shapes: use Schema.decodeUnknownSync (effect/Schema has no .zod) and SessionV1.User (MessageV2.User no longer exported).
|
Thanks for your contribution! This PR doesn't have a linked issue. All PRs must reference an existing issue. Please:
See CONTRIBUTING.md for details. |
|
Important Review skippedToo many files! This PR contains 296 files, which is 146 over the limit of 150. To get a review, narrow the scope: ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (4)
📒 Files selected for processing (296)
You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request migrates the desktop application from Tauri to Electron, updates Nix and package configurations, introduces locale-specific translation glossaries, and integrates an AWS-based data lake for ingestion and monitoring. It also refactors several SolidJS components in the web application to improve dialogs, layout, and state management, alongside reorganizing Playwright E2E tests. The review feedback highlights critical SolidJS and Solid-Query issues, including using createMutation instead of useMutation to prevent runtime import errors, ensuring proper reactive dependency tracking in createResource by passing the active server as a source, adding optional chaining to avoid potential TypeErrors on uninitialized store data, and fixing a state persistence bug in the provider auto-selection logic.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
|
||
| setForm("saving", true) | ||
| const saveMutation = useMutation(() => ({ | ||
| mutationFn: async (result: NonNullable<ReturnType<typeof validate>>) => { |
There was a problem hiding this comment.
Use createMutation instead of useMutation to match the correct export from @tanstack/solid-query.
const saveMutation = createMutation(() => ({
| import { useDialog } from "@opencode-ai/ui/context/dialog" | ||
| import { Dialog } from "@opencode-ai/ui/dialog" | ||
| import { TextField } from "@opencode-ai/ui/text-field" | ||
| import { useMutation } from "@tanstack/solid-query" |
There was a problem hiding this comment.
In @tanstack/solid-query (v5), the standard and exported hook is createMutation, not useMutation. Using useMutation will result in a runtime import error.
Suggested change
| import { useMutation } from "@tanstack/solid-query" | |
| import { createMutation } from "@tanstack/solid-query" |
| return | ||
| } | ||
| const saveMutation = useMutation(() => ({ | ||
| mutationFn: async () => { |
There was a problem hiding this comment.
Use createMutation instead of useMutation to match the correct export from @tanstack/solid-query.
const saveMutation = createMutation(() => ({
Comment on lines
+84
to
+85
| ? input.t("provider.custom.error.required") | ||
| : seenModels.has(id) |
There was a problem hiding this comment.
This is validating the model ID, but it uses the translation key for the provider ID (provider.custom.error.providerID.required). It should use a generic required key or a model-specific one to avoid displaying an incorrect error message to the user.
const idError = !id
? input.t("provider.custom.error.required")| return res.data?.[props.provider] ?? fallback() | ||
| }, | ||
| ) | ||
| const loading = createMemo(() => auth.loading && !serverSync.data.provider_auth[props.provider]) |
There was a problem hiding this comment.
Use optional chaining here to prevent a potential TypeError if provider_auth is undefined.
Suggested change
| const loading = createMemo(() => auth.loading && !serverSync.data.provider_auth[props.provider]) | |
| const loading = createMemo(() => auth.loading && !serverSync.data.provider_auth?.[props.provider]) |
| }, | ||
| ) | ||
| const loading = createMemo(() => auth.loading && !serverSync.data.provider_auth[props.provider]) | ||
| const methods = createMemo(() => auth.latest ?? serverSync.data.provider_auth[props.provider] ?? fallback()) |
There was a problem hiding this comment.
Use optional chaining here to prevent a potential TypeError if provider_auth is undefined.
Suggested change
| const methods = createMemo(() => auth.latest ?? serverSync.data.provider_auth[props.provider] ?? fallback()) | |
| const methods = createMemo(() => auth.latest ?? serverSync.data.provider_auth?.[props.provider] ?? fallback()) |
Comment on lines
+323
to
331
| let auto = false | ||
| createEffect(() => { | ||
| if (auto) return | ||
| if (loading()) return | ||
| if (methods().length === 1) { | ||
| selectMethod(0) | ||
| auto = true | ||
| void selectMethod(0) | ||
| } | ||
| }) |
There was a problem hiding this comment.
Since auto is a local boolean variable, it will persist across runs of the effect for the lifetime of the component. However, if props.provider changes while the component is kept mounted, auto will remain true, preventing the auto-selection from running for the new provider. We should track the selected provider in auto instead of a simple boolean to support dynamic provider changes.
Suggested change
| let auto = false | |
| createEffect(() => { | |
| if (auto) return | |
| if (loading()) return | |
| if (methods().length === 1) { | |
| selectMethod(0) | |
| auto = true | |
| void selectMethod(0) | |
| } | |
| }) | |
| let auto = "" | |
| createEffect(() => { | |
| if (auto === props.provider) return | |
| if (loading()) return | |
| if (methods().length === 1) { | |
| auto = props.provider | |
| void selectMethod(0) | |
| } | |
| }) |
| let at = 0 | ||
| const dirs = [project.worktree, ...(project.sandboxes ?? [])] | ||
| for (const directory of dirs) { | ||
| const sessions = sync.child(directory, { bootstrap: false })[0].session |
There was a problem hiding this comment.
Use optional chaining and a fallback array here to prevent a potential TypeError if the child store is not yet initialized or doesn't have the session property.
Suggested change
| const sessions = sync.child(directory, { bootstrap: false })[0].session | |
| const sessions = sync.child(directory, { bootstrap: false })[0]?.session ?? [] |
Comment on lines
+272
to
+289
| const [startupHealthCheck, healthCheckActions] = createResource(() => | ||
| props.disableHealthCheck | ||
| ? true | ||
| : Effect.gen(function* () { | ||
| if (!server.current) return true | ||
| const { http, type } = server.current | ||
|
|
||
| while (true) { | ||
| const res = yield* Effect.promise(() => checkServerHealth(http)) | ||
| if (res.healthy) return true | ||
| if (checkMode() === "background" || type === "http") return false | ||
| } | ||
| }).pipe( | ||
| Effect.timeoutOrElse({ duration: "10 seconds", orElse: () => Effect.succeed(false) }), | ||
| Effect.ensuring(Effect.sync(() => setCheckMode("background"))), | ||
| Effect.runPromise, | ||
| ), | ||
| ) |
There was a problem hiding this comment.
In SolidJS, createResource only tracks reactive dependencies accessed in its first argument (the source function). Since server.current is accessed inside the fetcher function itself, changes to the active server will not automatically trigger a new health check. We should pass the active server as the source to make the resource fully reactive.
const [startupHealthCheck, healthCheckActions] = createResource(
() => ({ server: server.current, disable: props.disableHealthCheck }),
({ server, disable }) =>
disable
? true
: Effect.gen(function* () {
if (!server) return true
const { http, type } = server
while (true) {
const res = yield* Effect.promise(() => checkServerHealth(http))
if (res.healthy) return true
if (checkMode() === "background" || type === "http") return false
}
}).pipe(
Effect.timeoutOrElse({ duration: "10 seconds", orElse: () => Effect.succeed(false) }),
Effect.ensuring(Effect.sync(() => setCheckMode("background"))),
Effect.runPromise,
),
)
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds parentAgent context propagation across sessions, tasks, and chat hooks.
Changes