Conversation
…d of skill invocations. (anomalyco#17053)
…lyco#16974) Co-authored-by: wangxinxin <xinxin.wang@pharmbrain.com>
|
This PR doesn't fully meet our contributing guidelines and PR template. What needs to be fixed:
Please edit this PR description to address the above within 2 hours, or it will be automatically closed. If you believe this was flagged incorrectly, please let a maintainer know. |
|
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. |
📝 WalkthroughWalkthroughA comprehensive feature release adding workspace pinning with persistence, project grouping and sub-project management, session modal with deep linking, native Git repository cloning, model favorites and quick select, rich markdown copy support, and enhanced file reference parsing across the application. Changes
Sequence DiagramssequenceDiagram
participant User as User
participant DialogOpenProject as DialogOpenProject
participant Platform as Platform API
participant Tauri as Tauri Backend
participant System as System (git)
participant Filesystem as Filesystem
User->>DialogOpenProject: Select git mode, enter URL
DialogOpenProject->>DialogOpenProject: Validate Git URL
activate DialogOpenProject
DialogOpenProject->>Platform: cloneGitRepository(url, targetDir)
activate Platform
Platform->>Tauri: clone_git_repository command
activate Tauri
Tauri->>Tauri: Resolve target directory (default or provided)
Tauri->>System: git clone url directory
activate System
System->>Filesystem: Create repository
Filesystem-->>System: Repository created
deactivate System
System-->>Tauri: Success, directory path
Tauri-->>Platform: Return cloned directory path
deactivate Tauri
Platform-->>DialogOpenProject: Return directory
deactivate Platform
DialogOpenProject->>DialogOpenProject: List files, emit onSelect
DialogOpenProject-->>User: Open project with new directory
deactivate DialogOpenProject
sequenceDiagram
participant Tauri as Desktop App (Tauri)
participant SessionModal as Session Modal (Vite)
participant SDK as OpenCode SDK
participant Server as Server API
Tauri->>SessionModal: Launch iframe/window
activate SessionModal
SessionModal->>Tauri: awaitInitialization()
activate Tauri
Tauri->>Server: Get server URL, auth
Server-->>Tauri: Server ready data
Tauri-->>SessionModal: URL, credentials, source
deactivate Tauri
SessionModal->>SDK: Initialize client(baseUrl, auth)
activate SDK
SDK->>Server: List projects, sessions
Server-->>SDK: Projects and sessions
SDK-->>SessionModal: Data
deactivate SDK
SessionModal->>SessionModal: Sort by priority, filter "needs attention"
User->>SessionModal: Click session
SessionModal-->>SessionModal: Build opencode://open-session deep link
SessionModal->>Tauri: Navigate to deep link
activate Tauri
Tauri->>Tauri: Parse deep link, navigate to session
Tauri-->>User: Open session in main window
deactivate Tauri
deactivate SessionModal
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
✨ Finishing Touches
🧪 Generate unit tests (beta)
|
…te' into clean/all-work # Conflicts: # packages/app/src/i18n/en.ts # packages/app/src/pages/layout.tsx # packages/ui/src/components/message-part.tsx
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
🟠 Major comments (27)
packages/opencode/src/cli/cmd/tui/component/prompt/autocomplete.tsx-340-341 (1)
340-341:⚠️ Potential issue | 🟠 MajorFix scoped-agent insertion too, not just rendering.
This avoids the doubled
@in the menu, but selecting a scoped agent still inserts@@scope/namebecauseonSelectpassesagent.nameintoinsertPart(), andinsertPart()always prepends another@. Please normalize the inserted value the same way as the display text.Suggested fix
.map( (agent): AutocompleteOption => ({ // Avoid double @ for scoped package agents (e.g., `@openpets/coder/pr-review`) display: agent.name.startsWith("@") ? agent.name : "@" + agent.name, onSelect: () => { - insertPart(agent.name, { + insertPart(agent.name.startsWith("@") ? agent.name.slice(1) : agent.name, { type: "agent", name: agent.name, source: { start: 0, end: 0, value: "", }, }) }, }), )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/opencode/src/cli/cmd/tui/component/prompt/autocomplete.tsx` around lines 340 - 341, The selected scoped-agent value is being double-@ inserted because onSelect passes agent.name into insertPart() while insertPart() always prepends "@"; update the onSelect handler (where agent objects are used in the autocomplete list) to normalize the value the same way as the display logic: compute selectedValue = agent.name.startsWith("@") ? agent.name : "@" + agent.name and pass selectedValue into insertPart(), so both rendering and insertion use identical normalization for agent.name.packages/util/src/session-transcript.ts-45-45 (1)
45-45:⚠️ Potential issue | 🟠 MajorAvoid exporting raw reasoning parts by default.
This branch serializes
part.type === "reasoning"verbatim into the transcript. If reasoning parts are meant to stay hidden anywhere else in the product, this export path bypasses that and leaks internal model traces.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/util/src/session-transcript.ts` at line 45, The current serializer in session-transcript.ts returns reasoning parts verbatim when part.type === "reasoning" (the `_Thinking:_\n\n${part.text}\n\n` branch), which can leak internal model traces; change the behavior to never export raw reasoning text — either skip these parts or replace them with a fixed redaction placeholder like "[internal reasoning hidden]" (preserve other part types as-is), and update the function that builds transcripts to use that placeholder instead of returning part.text for reasoning parts.packages/util/src/session-transcript.ts-48-56 (1)
48-56:⚠️ Potential issue | 🟠 MajorMake tool-state rendering resilient to falsy and non-JSON inputs.
part.state.input ? ...skips valid inputs like0,false, and"", andJSON.stringify(part.state.input, null, 2)can throw forBigIntor circular objects, which would fail the entire transcript export.🛠️ Proposed hardening
+const stringifyForTranscript = (value: unknown) => { + try { + return JSON.stringify(value, null, 2) + } catch { + return String(value) + } +} + const formatPart = (part: Part) => { if (part.type === "text" && part.text && !part.synthetic) return `${part.text}\n\n` if (part.type === "reasoning" && part.text) return `_Thinking:_\n\n${part.text}\n\n` if (part.type !== "tool" || !part.tool || !part.state) return "" - const input = part.state.input - ? `\n**Input:**\n\`\`\`json\n${JSON.stringify(part.state.input, null, 2)}\n\`\`\`\n` + const input = part.state.input !== undefined + ? `\n**Input:**\n\`\`\`json\n${stringifyForTranscript(part.state.input)}\n\`\`\`\n` : "" const output = - part.state.status === "completed" && part.state.output + part.state.status === "completed" && part.state.output !== undefined ? `\n**Output:**\n\`\`\`\n${part.state.output}\n\`\`\`\n` : "" const error = - part.state.status === "error" && part.state.error ? `\n**Error:**\n\`\`\`\n${part.state.error}\n\`\`\`\n` : "" + part.state.status === "error" && part.state.error !== undefined + ? `\n**Error:**\n\`\`\`\n${part.state.error}\n\`\`\`\n` + : "" return `**Tool: ${part.tool}**\n${input}${output}${error}\n` }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/util/src/session-transcript.ts` around lines 48 - 56, The current checks use truthiness and raw JSON.stringify which will skip valid falsy inputs (0, false, "") and can throw on BigInt/circular values; change the guards to test for null/undefined (e.g., part.state.input !== undefined && part.state.input !== null) for input, output and error, and wrap JSON serialization in a safe try/catch: attempt JSON.stringify(part.state.input, null, 2) and on failure fall back to a safe string representation (e.g., String(part.state.input) or util.inspect(part.state.input) / converting BigInt to string); update the template builders (the variables input, output, error near part.state.input/part.state.output/part.state.error) to use these null/undefined checks and the safe-serialize fallback so transcript generation never throws on non-JSON values.packages/opencode/src/config/config.ts-739-744 (1)
739-744:⚠️ Potential issue | 🟠 MajorPrompt-based package agents drop their own
name.
agentNameis computed above, but the parsed config never includes it. Callers that readconfig.agent[key].name—the same shape produced by the local loaders and theagent/package path—will getundefinedfor prompts loaded from npm packages.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/opencode/src/config/config.ts` around lines 739 - 744, The parsed Agent config is missing its name field so package-based prompt agents end up with undefined names; update the config creation in the block that constructs const config: Agent to include a name property using the already-computed agentName (e.g. name: frontmatter.name || agentName) so callers expecting config.agent[key].name get the correct value; modify the object literal that currently sets description, prompt, mode, ...frontmatter to also set name accordingly.packages/opencode/src/config/config.ts-288-288 (1)
288-288:⚠️ Potential issue | 🟠 MajorNormalize the target version in
needsInstall()as well.If
Installation.VERSIONis not valid semver, Line 288 writes*, butneedsInstall()still compares@opencode-ai/pluginagainst the raw version string. That makes the dependency look stale on every load and re-triggers installs indefinitely.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/opencode/src/config/config.ts` at line 288, needsInstall() currently compares the installed '@opencode-ai/plugin' version against raw Installation.VERSION which can be non-semver and causes perpetual reinstalls; update needsInstall() to use the same normalized targetVersion logic as Line 288 (i.e., use "*" when Installation.isLocal() is true, otherwise call pluginVersion(Installation.VERSION)) so the comparison uses the canonical targetVersion value; reference the existing pluginVersion(...) helper and Installation.isLocal()/Installation.VERSION when locating where to change the comparison.packages/opencode/src/config/config.ts-218-220 (1)
218-220:⚠️ Potential issue | 🟠 MajorDon't merge project package agents after the managed override.
This runs after the enterprise-managed config load, so agents from
Instance.worktree/node_modulescan overwrite managed agent fields and break the documented precedence order. Move this scan before Lines 212-216, or re-apply managed config afterward.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/opencode/src/config/config.ts` around lines 218 - 220, The project node_modules agent load currently merges into result.agent after the enterprise-managed override, allowing Instance.worktree package agents (loaded via loadPackageAgents and merged with mergeDeep into result.agent) to overwrite managed settings; move the projectNodeModules scan and the call that sets result.agent = mergeDeep(...) to execute before the managed config application (the code that applies the managed override), or if easier, keep it where it is but re-apply the managed configuration merge after calling loadPackageAgents so the managed config wins; update references to Instance.worktree, loadPackageAgents, mergeDeep, and result.agent accordingly so the precedence is correct.packages/opencode/src/config/config.ts-672-717 (1)
672-717:⚠️ Potential issue | 🟠 MajorMalformed package prompts currently fail the entire config load.
Unlike
loadAgent()andloadCommand(), these paths callConfigMarkdown.parse()without a recovery path. One bad markdown file in a dependency will makeConfig.get()reject for the whole workspace instead of logging and skipping the bad package entry.Also applies to: 720-752
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/opencode/src/config/config.ts` around lines 672 - 717, In loadAgentsFromPackage, surround the call(s) to ConfigMarkdown.parse (used in the PACKAGE_AGENT_GLOB and PACKAGE_PROMPT_GLOB handling) with a try/catch so a malformed markdown file does not throw and abort Config.get; on parse failure catch the error, log a clear message including the package name and the file path (and the error) and then continue to the next file (skip adding that agent to result), mirroring the resilient behavior in loadAgent/loadCommand and ensuring result only gets populated with successfully parsed Agent.safeParse entries (e.g., when constructing and assigning parsed.data to result[prefixedName]).script/check-preload.ts-9-13 (1)
9-13:⚠️ Potential issue | 🟠 MajorDon't regex-parse
bunfig.tomlhere.
bunfig.tomlis TOML, sopreloadentries can legally use single-quoted literal strings and comments. The current regex-based approach has two concrete problems:
- Misses single-quoted entries: A regex for
"([^"]+)"only extracts double-quoted strings. Single-quoted values likepreload = ['./preload.ts']are silently ignored.- Matches commented-out entries: The regex
preload\s*=\s*\[([\s\S]*?)\]can match commented-out lines like# preload = ["./ghost.ts"], reporting files that shouldn't be preloaded.Bun supports TOML as a standard parsed format, so use a real TOML parser instead.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@script/check-preload.ts` around lines 9 - 13, The current code builds preload by regexing the file text (the const text and const preload logic using /preload\s*=\s*\[([\s\S]*?)\]/ and /"([^"]+)"/), which misses single-quoted TOML strings and picks up commented-out lines; replace this with a proper TOML parse: read the file text, parse it with a TOML parser, then extract the preload array from the parsed object (handling both single- and double-quoted strings naturally and ignoring comments), and assign that array to the preload variable instead of using the regex extraction.script/check-preload.ts-7-8 (1)
7-8:⚠️ Potential issue | 🟠 MajorFix the
node_modulesskip predicate.
node_modules/pkg/bunfig.tomlat the repo root never matches"/node_modules/", so dependency bunfigs still get scanned. That can make this check fail on files the script meant to ignore.♻️ Proposed fix
for await (const file of new Bun.Glob("**/bunfig.toml").scan(".")) { - if (file.includes("/node_modules/")) continue + if (file.split(/[\\/]/).includes("node_modules")) continue const text = await Bun.file(file).text()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@script/check-preload.ts` around lines 7 - 8, The skip predicate currently checks file.includes("/node_modules/"), which misses repo-root paths like "node_modules/pkg/bunfig.toml"; update the predicate used in the Bun.Glob scan loop (the file variable in the for-await over new Bun.Glob(...).scan(".")) to detect node_modules as a path segment instead of requiring leading/trailing slashes—for example replace the string check with a path-segment-aware test (e.g. a RegExp like /(^|\/)node_modules\// or an includes("node_modules/") that accounts for both leading and non-leading occurrences) so dependency bunfigs are correctly ignored.packages/app/src/components/dialog-select-session.tsx-169-172 (1)
169-172:⚠️ Potential issue | 🟠 MajorDon't collapse fetch failures into an empty state.
A rejected
session.list()currently becomes[], so the dialog shows “empty” instead of “failed to load”. Becausecachedstays unset andinflightis cleared infinally, the next search/input will silently retry the same failing request.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/app/src/components/dialog-select-session.tsx` around lines 169 - 172, The current catch in dialog-select-session's fetch chain swallows session.list() failures by returning [] and leaving state.cached unset, which hides errors and causes silent retries; instead, in the promise chain for session.list() remove the catch that maps failures to [] and handle errors explicitly: on rejection set a state.error (or state.loadFailed) flag/message and do not populate state.cached, then keep the finally block to clear state.inflight; update any UI consumers to show the “failed to load” state when state.error is set. Ensure you reference the same session.list() call site and state properties (state.inflight, state.cached, add state.error/state.loadFailed) so failures are surfaced rather than collapsed into an empty list.packages/app/src/components/dialog-select-session.tsx-210-212 (1)
210-212:⚠️ Potential issue | 🟠 MajorAdd "directory" to
filterKeysto align with the items filter logic.The
items()function filters ontitle,description, anddirectory(line 185), butfilterKeysonly includes the first two. This causes items matching only on directory to be excluded by the subsequent fuzzy filter inuseFilteredList.items={items} key={(item) => item.id} filterKeys={["title", "description", "directory"]}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/app/src/components/dialog-select-session.tsx` around lines 210 - 212, The filterKeys prop on the list is missing "directory", causing directory-only matches to be excluded; update the component where you pass filterKeys (the prop alongside items and key in dialog-select-session.tsx) to include "directory" (i.e., change filterKeys from ["title","description"] to ["title","description","directory"]) so it lines up with the items() filter logic used earlier and the fuzzy filtering in useFilteredList.packages/app/src/components/dialog-select-session.tsx-101-103 (1)
101-103:⚠️ Potential issue | 🟠 MajorUse
<Show>here;<Match>is inert outside<Switch>.Line 101 wraps the preview branch in
Matchwithout a parentSwitch. In Solid,<Match>is only a marker element that<Switch>inspects to determine which branch to render—it has no conditional logic of its own. The latest assistant text will not render. Use<Show>for standalone conditionals.Suggested fix
-import { createMemo, Match, type Accessor, Switch } from "solid-js" +import { createMemo, Match, Show, type Accessor, Switch } from "solid-js" ... - <Match when={showLatest() && latest()}> - <div class="text-12-regular text-text-weak truncate pr-4 mt-0.5">{latest()}</div> - </Match> + <Show when={showLatest() && latest()}> + {(text) => <div class="text-12-regular text-text-weak truncate pr-4 mt-0.5">{text()}</div>} + </Show>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/app/src/components/dialog-select-session.tsx` around lines 101 - 103, The preview branch uses <Match> outside a <Switch>, so replace the <Match when={showLatest() && latest()}> wrapper with Solid's <Show when={showLatest() && latest()}> and close it with </Show>, keeping the inner div and {latest()} unchanged; also ensure Show is imported from 'solid-js' in the dialog-select-session component so the conditional around showLatest() and latest() actually renders.packages/opencode/src/server/routes/session.ts-270-271 (1)
270-271:⚠️ Potential issue | 🟠 MajorKeep
time_updatedserver-owned.
updatedis now part of the public PATCH payload and is forwarded during archive/unarchive. That lets callers backdate or future-date a session, which skews thetime_updatedordering/filtering used bySession.list()andSession.listGlobal(). Stamp this on the server instead and keep any backfill/import path internal.💡 Suggested change
validator( "json", z.object({ title: z.string().optional(), time: z .object({ archived: z.number().nullable().optional(), - updated: z.number().optional(), }) .optional(), }), ), @@ if (updates.time && "archived" in updates.time) { session = await Session.setArchived({ sessionID, time: updates.time.archived ?? null, - updated: updates.time.updated, + updated: Date.now(), }) }Also applies to: 284-289
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/opencode/src/server/routes/session.ts` around lines 270 - 271, The public session PATCH/schema must not accept client-supplied updated timestamps; remove the client-facing "updated" field from the Zod schemas in session route (the occurrences around the snippet where "archived: z.number().nullable().optional(), updated: z.number().optional()," and the similar block at lines ~284-289) and instead set/overwrite the session's updated/time_updated server-side in the handlers that perform archive/unarchive and general PATCH updates (the code paths that feed into Session.list() and Session.listGlobal()); ensure server code stamps the current time (or appropriate internal value) into the session record before saving so clients cannot backdate/future-date sessions.packages/app/src/components/dialog-select-model.tsx-96-113 (1)
96-113:⚠️ Potential issue | 🟠 MajorGive the favorite toggle an accessible name and state.
This is an icon-only control. Right now assistive tech won't get a reliable label, and it also can't tell whether the model is already favorited.
♿ Suggested fix
<Tooltip value={language.t(favorite(i.provider.id, i.id) ? "dialog.model.unfavorite" : "dialog.model.favorite")} > <IconButton icon="circle-check" variant="ghost" size="small" class="shrink-0" + aria-label={language.t( + favorite(i.provider.id, i.id) ? "dialog.model.unfavorite" : "dialog.model.favorite", + )} + aria-pressed={favorite(i.provider.id, i.id)} classList={{ "opacity-100": favorite(i.provider.id, i.id), "opacity-30": !favorite(i.provider.id, i.id), }} onMouseDown={(event) => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/app/src/components/dialog-select-model.tsx` around lines 96 - 113, The icon-only favorite toggle lacks accessible name and state; update the IconButton used inside the Tooltip to include an accessible label and a toggle state by adding aria-label set to the same localized string you compute (language.t(favorite(...) ? "dialog.model.unfavorite" : "dialog.model.favorite")) and aria-pressed bound to the boolean favorite(i.provider.id, i.id); keep the existing onClick/onMouseDown and Tooltip but ensure IconButton receives aria-label and aria-pressed so assistive tech can read its name and whether it is currently favorited (use the existing favorite(...) and toggleFavorite(...) helpers).packages/desktop/scripts/predev.ts-5-21 (1)
5-21:⚠️ Potential issue | 🟠 MajorAdd explicit bounds checking for Windows and Linux architectures.
target()silently falls back to x64 binaries for any unrecognized architecture. Windows ARM (and any other non-x64 Windows host) will build/copy the wrong sidecar; same applies to non-ARM/non-x64 Linux hosts. Currently supported targets are only:aarch64-apple-darwin,x86_64-apple-darwin,x86_64-pc-windows-msvc,x86_64-unknown-linux-gnu, andaarch64-unknown-linux-gnu.Fail fast for unsupported configurations instead of silently selecting x64:
🛠️ Suggested fix
function target() { const env = Bun.env.TAURI_ENV_TARGET_TRIPLE if (env) return env if (process.platform === "darwin") { return process.arch === "arm64" ? "aarch64-apple-darwin" : "x86_64-apple-darwin" } if (process.platform === "win32") { - return "x86_64-pc-windows-msvc" + if (process.arch === "x64") return "x86_64-pc-windows-msvc" + throw new Error(`Unsupported Windows architecture: ${process.arch}`) } if (process.arch === "arm64") { return "aarch64-unknown-linux-gnu" } - return "x86_64-unknown-linux-gnu" + if (process.arch === "x64") return "x86_64-unknown-linux-gnu" + throw new Error(`Unsupported Linux architecture: ${process.arch}`) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/desktop/scripts/predev.ts` around lines 5 - 21, The target() function currently falls back silently to x86_64 for unknown arches; update target() to explicitly validate architectures for each platform and fail fast on unsupported combos: keep the Bun.env.TAURI_ENV_TARGET_TRIPLE override, then for darwin keep the existing arm64 vs x86_64 branching, for win32 only accept process.arch === "x64" and return "x86_64-pc-windows-msvc" otherwise throw an error (or process.exit) indicating unsupported Windows architecture, and for linux only accept "arm64" (return "aarch64-unknown-linux-gnu") or "x64" (return "x86_64-unknown-linux-gnu") otherwise throw an error indicating unsupported Linux architecture; implement these checks in the existing target() function to avoid silent fallbacks.packages/app/src/pages/session/use-session-commands.tsx-286-301 (1)
286-301:⚠️ Potential issue | 🟠 MajorFix keybind conflicts in sessionCommand and modelCommand definitions.
Two keybinds are assigned twice, causing command shadowing:
mod+shift+arrowdown: assigned to bothmessage.bottom(line 298) andmodel.quick.switch(line 317)mod+shift+arrowup: assigned to bothmessage.top(line 290) andmodel.quick.switch.reverse(line 333)Choose distinct keybinds for these commands to ensure both remain accessible.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/app/src/pages/session/use-session-commands.tsx` around lines 286 - 301, Two commands share the same hotkeys causing shadowing: change the duplicate keybinds so sessionCommand ids "message.top" and "message.bottom" keep their current mod+shift+arrowup/arrowdown while modelCommand ids "model.quick.switch" and "model.quick.switch.reverse" use distinct keys (for example mod+shift+arrowright and mod+shift+arrowleft, or mod+k / mod+shift+k) instead; update the keybind strings in the modelCommand definitions (look for modelCommand with id "model.quick.switch" and "model.quick.switch.reverse") so they no longer collide with sessionCommand ids "message.top" and "message.bottom".packages/desktop/package.json-12-12 (1)
12-12:⚠️ Potential issue | 🟠 MajorRoute dev:session-modal through predev and use cross-platform environment variable handling.
This command calls
tauri devdirectly, skipping the sidecar binary preparation handled bypredev(which builds and copies the opencode CLI binary tosrc-tauri/sidecars). Additionally, theVITE_OPEN_SESSION_SEARCH=1syntax is POSIX-only and won't work on Windows shells.Follow the pattern from the root
dev:desktopcommand: runpredevfirst, thentauri dev. Set the environment variable in a cross-platform way (e.g., via.envfile, vite config, or a wrapper script).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/desktop/package.json` at line 12, Update the "dev:session-modal" npm script so it runs the predev preparation step before invoking tauri dev and does not rely on POSIX-only inline env vars; specifically, change the script that currently named "dev:session-modal" to call the existing predev script (same as "dev:desktop" uses) and then run "tauri dev", and move the VITE_OPEN_SESSION_SEARCH=1 setting into a cross-platform mechanism (for example add it to the project .env, Vite config, or a small wrapper script that sets process.env before starting) so Windows shells are supported and the opencode sidecar is built/copied by predev.packages/desktop/src-tauri/src/lib.rs-615-633 (1)
615-633:⚠️ Potential issue | 🟠 MajorAvoid a live GitHub clone in a unit test.
This test requires outbound network access, GitHub availability, and a system
gitbinary. It will flap in CI and fail for offline contributors. Please switch to a local temporary repo fixture or gate it behind an explicit integration-test opt-in.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/desktop/src-tauri/src/lib.rs` around lines 615 - 633, The test test_clone_with_git_real_repository calls clone_with_git with a real GitHub URL causing network and git-binary dependence; replace it with a local temporary repository fixture or gate it as an opt-in integration test. To fix, create a local repo in temp_dir (e.g., initialize with git2 or run `git init` + create a commit) and then call clone_with_git using a file:// or local path URL to that repo, or alternatively mark test_clone_with_git_real_repository with #[ignore] or move it behind a feature/#[cfg(test_integration)] so it only runs when integration tests are explicitly enabled; ensure you still assert that the target/.git exists and clean up temp files.packages/desktop/src-tauri/src/lib.rs-433-437 (1)
433-437:⚠️ Potential issue | 🟠 MajorThe WSL clone path is unreachable right now.
This branch keys off
get_wsl_config(), butpackages/desktop/src-tauri/src/server.rsstill returnsWslConfig { enabled: false }unconditionally. On Windows, enabling WSL in the UI will still fall through to nativegit clone.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/desktop/src-tauri/src/lib.rs` around lines 433 - 437, The WSL branch never runs because get_wsl_config() always returns WslConfig { enabled: false } in server.rs; update the code that constructs and returns WslConfig in server.rs so it reflects the actual persisted/UI setting (or reads the same config source the UI writes) instead of hardcoding false, so that get_wsl_config() on Windows can return enabled=true when the user enables WSL and allow clone_with_wsl(&url, directory.as_deref()) to execute.packages/app/src/pages/session.tsx-821-826 (1)
821-826:⚠️ Potential issue | 🟠 MajorMake
jumpToTopreveal the cached history first.
createSessionHistoryWindow()only renders the tail of the timeline. This handler just scrolls the current window totop = 0, so the next backfill preserves the viewport and long sessions never actually jump to their beginning.🛠️ Suggested fix
const jumpToTop = () => { const el = scroller if (!el) return autoScroll.pause() - el.scrollTo({ top: 0, behavior: "auto" }) + historyWindow.setTurnStart(0) + requestAnimationFrame(() => { + scroller?.scrollTo({ top: 0, behavior: "auto" }) + }) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/app/src/pages/session.tsx` around lines 821 - 826, The jumpToTop handler currently just scrolls the visible window; first ensure the timeline window is repositioned to include the cached/earliest history before scrolling: call the function that renders the full/history tail (createSessionHistoryWindow) or whatever routine that expands the session history window to the beginning, await or wait for that update to complete, then pause autoScroll and call scroller.scrollTo({ top: 0, behavior: "auto" }); keep the existing guards (if (!scroller) return) and ensure any async call to createSessionHistoryWindow is awaited or its completion observed before performing the scroll.packages/ui/src/components/markdown-copy.ts-42-54 (1)
42-54:⚠️ Potential issue | 🟠 MajorAdd error handling for
clipboard.write()rejections at runtime.Even when
ClipboardItemandclipboard.writeexist, the operation can reject withNotAllowedErrorin non-secure contexts, without user activation, or when blocked by permissions/policy. Currently, such failures cause the copy to fail silently instead of degrading to plain text.Wrap the
write()call in a try-catch block to fall back towriteText()as a final attempt:Suggested fix
export async function writeClipboardPayload(input: { text: string; html?: string }) { const clipboard = navigator.clipboard if (!clipboard) return if (!input.html || typeof ClipboardItem === "undefined" || typeof clipboard.write !== "function") { await clipboard.writeText(input.text) return } - await clipboard.write([ - new ClipboardItem({ - "text/plain": new Blob([input.text], { type: "text/plain" }), - "text/html": new Blob([input.html], { type: "text/html" }), - }), - ]) + try { + await clipboard.write([ + new ClipboardItem({ + "text/plain": new Blob([input.text], { type: "text/plain" }), + "text/html": new Blob([input.html], { type: "text/html" }), + }), + ]) + } catch { + await clipboard.writeText(input.text) + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/markdown-copy.ts` around lines 42 - 54, In writeClipboardPayload, wrap the clipboard.write([...]) call in a try-catch inside the writeClipboardPayload function (when ClipboardItem and clipboard.write are available) and on error call await clipboard.writeText(input.text) as a fallback; ensure you still return after successful write or fallback and reference the existing symbols clipboard, ClipboardItem, clipboard.write, and clipboard.writeText so failures (e.g., NotAllowedError) degrade to plain-text copy instead of failing silently.packages/session-modal/src/main.tsx-212-220 (1)
212-220:⚠️ Potential issue | 🟠 MajorPreserve discovered auth when reloading the server URL.
saveBaseUrl()replaces the whole connection with{ baseUrl, source: "manual" }. In the Tauri flow that drops the initializedusername/password, so pressing Reload once can turn a working authenticated modal into repeated 401s.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/session-modal/src/main.tsx` around lines 212 - 220, saveBaseUrl currently overwrites the whole connection object, dropping initialized auth (username/password/token) and causing 401s; instead, read the current connection state and call setConnection with a merged object that preserves existing auth fields while updating baseUrl and source, and keep the localStorage key "session-modal.base-url" and log call unchanged; update the saveBaseUrl function to merge the existing connection (e.g., spread existing connection) with { baseUrl: value, source: "manual" } before calling setConnection so username/password are retained.packages/app/src/pages/layout.tsx-280-289 (1)
280-289:⚠️ Potential issue | 🟠 MajorDerive
expandedProjectfrom the active route as well.
groupedProjects()hides every subproject unless its parent matchesstore.expandedProject, but the new effects only clean that state—they never populate it fromcurrentDir(). Refreshing or deep-linking directly into a child project can therefore leave the active project hidden in the rail until something callsnavigateToProject().Also applies to: 336-347
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/app/src/pages/layout.tsx` around lines 280 - 289, groupedProjects currently only expands subprojects when store.expandedProject matches a parent, but store.expandedProject is never derived from the route/current directory, so deep-links can hide the active project; update the logic that computes store.expandedProject (or compute a derivedExpanded variable inside createMemo groupedProjects) to also consider currentDir() / the active route: derive the parent workspace key from currentDir() (using workspaceKey(currentDir())) and treat that as the expanded key if store.expandedProject is undefined, so groupedProjects (and the similar block at 336-347) will include the active child project even after refresh without requiring navigateToProject().packages/app/src/pages/layout.tsx-1171-1180 (1)
1171-1180:⚠️ Potential issue | 🟠 MajorHandle rejected unarchive requests in the command action.
This handler drops the promise returned by
unarchiveSession(), so failures surface as unhandled rejections and the user gets no toast or recovery path. It should mirror the error handling used inarchiveSession().🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/app/src/pages/layout.tsx` around lines 1171 - 1180, The unarchive command handler drops the promise from unarchiveSession(), causing unhandled rejections; update the onSelect for id "session.unarchive" to mirror archiveSession() by retrieving const session = selectedSession(), calling unarchiveSession(session, session.time.updated ?? session.time.created) and awaiting it (or attaching .then/.catch), and ensure failures are caught so the same toast/error recovery path used by archiveSession() is invoked (show error toast and handle any rollback or state updates).packages/app/src/components/dialog-open-project.tsx-21-25 (1)
21-25:⚠️ Potential issue | 🟠 MajorRender the locked
pathflow too.
lockMode=truewithmode="path"currently hides both input sections, so the dialog shows a submit button with no way to enter or browse a directory. Since the props expose that combination, the path-only variant is unusable.Also applies to: 152-191, 193-243
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/app/src/components/dialog-open-project.tsx` around lines 21 - 25, The DialogOpenProject component currently hides both input sections when lockMode is true, making the mode="path" variant unusable; update the JSX conditional logic so that when props.mode === "path" you still render the path input flow even if props.lockMode is true (render it as readOnly/disabled or show the locked path value and disable the browse button as appropriate), while preserving the existing locked behavior for the project selection flow; locate the conditional blocks that render the path input and project input inside DialogOpenProject and change the conditions that check props.lockMode to allow the path branch to render when props.mode === "path".packages/session-modal/src/main.tsx-146-187 (1)
146-187:⚠️ Potential issue | 🟠 MajorIsolate per-directory fetch failures.
This aggregate load uses
Promise.allall the way down, so one stale/inaccessible directory can reject the entire resource and flip the whole modal into the error state. These per-directory failures should degrade to[], not take down the rest of the session list.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/session-modal/src/main.tsx` around lines 146 - 187, The per-directory fetch inside the directories.map used to build rows can reject the whole Promise.all; wrap the async callback (the function that builds scoped via client(value.baseUrl, auth, directory) and calls scoped.session.list, scoped.permission.list, scoped.question.list, scoped.session.status) in a try/catch so any error for a single directory is caught, logged, and the callback returns an empty array for that directory ([]) instead of throwing; keep the rest of the mapping logic (rootSession filtering, bySession grouping, computing permissionCount/questionCount/requiresInput/status/busy/priority) unchanged so only the failing directory degrades without flipping the entire modal to an error state.packages/ui/src/components/message-part.tsx-209-218 (1)
209-218:⚠️ Potential issue | 🟠 MajorStrip Windows separators before opening project-relative files.
On Windows,
relativizeProjectPath()returns values like\src\file.ts;replace(/^\//, "")leaves that leading separator intact. The new path click handlers will then pass an invalid relative path toopenFilePath().Minimal fix
- const file = relativizeProjectPath(path, directory).replace(/^\//, "") + const file = relativizeProjectPath(path, directory).replace(/^[\\/]+/, "")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/message-part.tsx` around lines 209 - 218, openProjectFile currently strips only a leading forward slash from the relativized path which leaves a leading backslash on Windows (e.g. "\src\file.ts") and yields an invalid path for openFilePath; update the cleanup on the value returned by relativizeProjectPath in openProjectFile to strip any leading slash or backslash (use a regex that matches both characters, e.g. /^[\\/]+/) so the resulting file string is normalized before calling openFilePath({ path: file }).
🟡 Minor comments (11)
packages/util/src/session-transcript.ts-83-85 (1)
83-85:⚠️ Potential issue | 🟡 MinorDon't leave a dangling separator after the last message.
The current mapping appends
---to every row, so every non-empty transcript ends with a trailing separator. Build the separators between rows instead.✂️ Proposed fix
- const body = rows - .map((row) => `${formatMessage(row.info, row.parts)}---\n`) - .join("\n") - .trimEnd() + const body = rows + .map((row) => formatMessage(row.info, row.parts).trimEnd()) + .join("\n\n---\n\n")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/util/src/session-transcript.ts` around lines 83 - 85, The code building the transcript body currently appends a trailing separator because it maps each row to `${formatMessage(row.info, row.parts)}---\n`; change this so you map rows to just `formatMessage(row.info, row.parts)` and then join the resulting array with the separator (e.g., `'\n---\n'`) to place separators between messages only; update the `body` assignment that uses `rows` and `formatMessage` accordingly and keep a final newline if the original output relied on it.packages/opencode/src/config/config.ts-701-703 (1)
701-703:⚠️ Potential issue | 🟡 MinorWindows path normalization is a no-op here.
replace(/\//g, "/")never rewrites\, so nested package agent IDs keep backslashes on Windows. That will produce mixed-separator names that do not match the slash-delimited format used by the rest of the config loaders.Also applies to: 727-728
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/opencode/src/config/config.ts` around lines 701 - 703, The Windows path normalization is ineffective because replace(/\//g, "/") doesn't convert backslashes; update the normalization of relativePath to produce forward-slash IDs by replacing backslashes with "/" (e.g., replace(/\\+/g, "/") or use path.posix.normalize on a split) when computing agentName and prefixedName (referencing relativePath, agentName, prefixedName and pkg.agentDir); apply the same fix to the other occurrence that mirrors lines 727-728 so all agent IDs use slash-delimited paths consistently.packages/app/src/components/dialog-select-session.tsx-201-201 (1)
201-201:⚠️ Potential issue | 🟡 MinorUpdate the Tailwind v4+ important modifier syntax.
The workspace uses Tailwind v4.1.11. In Tailwind v4+, the important modifier must be placed at the end of the class name. Change
!max-h-[480px]tomax-h-[480px]!on line 201.Suggested fix
- <Dialog class="pt-3 pb-0 !max-h-[480px]" transition> + <Dialog class="pt-3 pb-0 max-h-[480px]!" transition>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/app/src/components/dialog-select-session.tsx` at line 201, Update the Tailwind important modifier syntax in the Dialog JSX: replace the deprecated prefix form "!max-h-[480px]" with the v4+ suffix form "max-h-[480px]!" in the Dialog element inside the dialog-select-session component (the Dialog with class="pt-3 pb-0 !max-h-[480px]" in packages/app/src/components/dialog-select-session.tsx) so the class becomes pt-3 pb-0 max-h-[480px]! while preserving the other classes and the transition prop.packages/desktop/src/menu.ts-72-90 (1)
72-90:⚠️ Potential issue | 🟡 MinorInconsistent i18n: Some menu items use hardcoded English strings.
- Line 73:
"Search All Sessions..."should uset("desktop.menu.file.searchAllSessions")or similar- Line 87:
"Clone Project..."should uset("desktop.menu.file.cloneProject")or similarOther items in the same block correctly use the
t()function (e.g., Lines 78, 83).🌐 Suggested fix for i18n consistency
await MenuItem.new({ - text: "Search All Sessions...", + text: t("desktop.menu.file.searchAllSessions"), accelerator: "Shift+Cmd+P", action: () => trigger("session.search.all"), }),await MenuItem.new({ - text: "Clone Project...", + text: t("desktop.menu.file.cloneProject"), accelerator: "Shift+Cmd+O", action: () => trigger("project.clone"), }),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/desktop/src/menu.ts` around lines 72 - 90, Replace the hardcoded English menu labels in the MenuItem.new entries with i18n calls: change the text for the item that calls trigger("session.search.all") to t("desktop.menu.file.searchAllSessions") and change the text for the item that calls trigger("project.clone") to t("desktop.menu.file.cloneProject") so all menu items use the t() function consistently alongside the existing MenuItem.new and trigger(...) usages.packages/ui/src/components/markdown-copy.test.ts-33-65 (1)
33-65:⚠️ Potential issue | 🟡 MinorRestore mocked globals with their original descriptors in
finally.Both tests capture only the VALUES of
globalThis.navigatorandglobalThis.ClipboardItem(lines 34–35, 69–70) and restore them at the end of the test body (lines 64–65, 90–91). If an assertion fails orwriteClipboardPayload()throws, the restoration code never runs, causing mocks to leak into subsequent tests. Additionally, restoring withObject.defineProperty()andconfigurable: truechanges the original descriptor shape. Capture the full descriptors usingObject.getOwnPropertyDescriptor()and restore them in afinallyblock orafterEachhook.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/markdown-copy.test.ts` around lines 33 - 65, The test currently replaces globalThis.navigator and globalThis.ClipboardItem by saving only their values and restoring them at the end of the test body; change it to capture the full property descriptors via Object.getOwnPropertyDescriptor(globalThis, "navigator") and Object.getOwnPropertyDescriptor(globalThis, "ClipboardItem") before mocking, then perform the mocks, and ensure restoration happens in a finally block (or an afterEach) that uses Object.defineProperty to restore the original descriptors exactly so mocks cannot leak if writeClipboardPayload or assertions throw; reference the test's FakeClipboardItem, writeClipboardPayload, and the globalThis.navigator/globalThis.ClipboardItem mocks when making these changes.packages/app/src/context/server.tsx-255-257 (1)
255-257:⚠️ Potential issue | 🟡 MinorPotential inconsistency between normalized deduplication and exact-match operations.
The deduplication logic uses
normalizeWorktreefor comparison, but the original (non-normalized)directoryis stored. Subsequent operations (close,expand,collapse,move) use exact string matching (x.worktree === directory).This could cause issues if paths are passed inconsistently. For example:
- User opens
/tmp/repo/→ stored as/tmp/repo/- User later tries to close
/tmp/repo(no trailing slash) → exact match failsConsider normalizing the stored value as well:
const target = normalizeWorktree(directory) if (current.find((x) => normalizeWorktree(x.worktree) === target)) return -setStore("projects", key, [{ worktree: directory, expanded: true }, ...current]) +setStore("projects", key, [{ worktree: target, expanded: true }, ...current])Or alternatively, apply normalization in the other operations too for consistency.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/app/src/context/server.tsx` around lines 255 - 257, The dedupe uses normalizeWorktree but you store the raw directory, causing later exact-match ops (close/expand/collapse/move) on x.worktree === directory to fail; change the stored value to the normalized form by passing the normalized target into setStore (i.e., store { worktree: target, expanded: true } instead of { worktree: directory, ... }), or alternatively ensure all operations (close, expand, collapse, move) compare using normalizeWorktree(x.worktree) === normalizeWorktree(directory); update references to normalizeWorktree and the setStore("projects", key, ...) call accordingly so storage and comparisons use the same normalized representation.packages/app/src/pages/session/message-timeline.tsx-1-12 (1)
1-12:⚠️ Potential issue | 🟡 MinorRemove unused
startTransitionimport.
startTransitionis imported on line 10 but never used in the file. Remove it from the import statement.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/app/src/pages/session/message-timeline.tsx` around lines 1 - 12, The import statement in message-timeline.tsx includes an unused symbol startTransition; remove startTransition from the named imports (the import that currently lists For, Index, createEffect, createMemo, createSignal, on, onCleanup, Show, startTransition, type JSX) so the file only imports the actually used symbols.packages/app/src/pages/directory-layout.tsx-27-54 (1)
27-54:⚠️ Potential issue | 🟡 MinorHardcoded error title should use i18n.
The error toast at line 38 uses a hardcoded string
"Open failed"instead of a localized string vialanguage.t(). This is inconsistent with error handling elsewhere in the codebase.🌐 Proposed fix for i18n consistency
Note: You'll need to add
useLanguageto the component and define an appropriate translation key.function DirectoryDataProvider(props: ParentProps<{ directory: string }>) { const navigate = useNavigate() const sync = useSync() const platform = usePlatform() + const language = useLanguage() const slug = createMemo(() => base64Encode(props.directory)) // ... in onOpenFilePath handler: showToast({ variant: "error", - title: "Open failed", + title: language.t("common.requestFailed"), description, })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/app/src/pages/directory-layout.tsx` around lines 27 - 54, Replace the hardcoded error title in the onOpenFilePath handler with a localized string: import and call useLanguage to get language (or use existing language prop), add a translation key (e.g. "errors.open_failed") in your i18n files, and change the showToast title from "Open failed" to language.t("errors.open_failed"); keep the rest of the error handling (description extraction, showToast call, and dispatching the "opencode:open-file-path" event) intact and ensure the new translation key is used where platform.openPath errors are handled.packages/ui/src/components/markdown.tsx-267-279 (1)
267-279:⚠️ Potential issue | 🟡 MinorStrip
file-linkbuttons from copied HTML.This only removes the copy button. If the selection includes a rendered file reference, the clipboard HTML still contains
button.file-linkplus itsdata-file-*attributes. Rich paste targets commonly drop buttons, so the reference disappears, and the hidden file metadata is copied out of the app.🧹 Suggested fix
const wrap = document.createElement("div") wrap.appendChild(range.cloneContents()) for (const item of wrap.querySelectorAll('[data-slot="markdown-copy-button"]')) { item.remove() } + for (const item of wrap.querySelectorAll("button.file-link")) { + const replacement = + item.querySelector("code")?.cloneNode(true) ?? document.createTextNode(item.textContent ?? "") + item.replaceWith(replacement) + } const html = serializeMarkdownClipboardHTML(wrap.innerHTML)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/markdown.tsx` around lines 267 - 279, The clipboard HTML still contains rendered file references and their hidden metadata; update the selection sanitization in the block around selection, wrap, serializeMarkdownClipboardHTML and clip.setData to remove any button.file-link elements (e.g., wrap.querySelectorAll('button.file-link') -> remove or replace with plain text/filename node) and strip any data-file-* attributes from remaining elements (iterate elements with attributes starting with "data-file-" and remove those attributes) before calling serializeMarkdownClipboardHTML and setting clip.setData.packages/app/src/pages/layout/helpers.ts-182-187 (1)
182-187:⚠️ Potential issue | 🟡 MinorDon't hardcode the all-projects label in this helper.
"All projects"is user-visible but fixed to English here, so every non-English locale will still render this group name untranslated. Return a stable id and translate it where the label is rendered instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/app/src/pages/layout/helpers.ts` around lines 182 - 187, The helper currently hardcodes the user-visible label "All projects" in the returned group object (id: "all", label: "All projects"), which prevents localization; change the helper to return a stable id (keep id: "all") and remove the user-visible label property (or set label to undefined) so the UI layer can translate it where rendered; update the component that consumes this helper to map id "all" to a translated string (e.g., via t('allProjects')) instead of relying on the helper for the label.packages/session-modal/src/main.tsx-247-283 (1)
247-283:⚠️ Potential issue | 🟡 MinorThe “Needs attention” panel still renders idle sessions.
The header/count uses
attention(), but the list below iterates oversessions(). That makes non-attention rows appear under a heading that says they need attention.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/session-modal/src/main.tsx` around lines 247 - 283, The list under the "Needs attention" panel is rendering all sessions (sessions()) while the header/count uses attention(), so non-attention rows appear incorrectly; change the UI to iterate and test the attention() array instead of sessions() — update the Show that checks sessions().length > 0 to attention().length > 0 and the For each to use attention() (and any inner uses of item still refer to item.requiresInput/item.busy etc.), so only attention() items are listed and the counts match the rendered rows.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 639bb7a4-9243-487f-847e-c76b022fceff
⛔ Files ignored due to path filters (3)
bun.lockis excluded by!**/*.lockpackages/sdk/js/src/v2/gen/sdk.gen.tsis excluded by!**/gen/**packages/sdk/js/src/v2/gen/types.gen.tsis excluded by!**/gen/**
📒 Files selected for processing (98)
.github/pull_request_template.mdpackage.jsonpackages/app/e2e/actions.tspackages/app/e2e/projects/projects-switch.spec.tspackages/app/e2e/projects/workspaces.spec.tspackages/app/e2e/selectors.tspackages/app/src/app.tsxpackages/app/src/components/dialog-edit-project.tsxpackages/app/src/components/dialog-open-project.helpers.test.tspackages/app/src/components/dialog-open-project.helpers.tspackages/app/src/components/dialog-open-project.tsxpackages/app/src/components/dialog-select-model-unpaid.tsxpackages/app/src/components/dialog-select-model.tsxpackages/app/src/components/dialog-select-session.tsxpackages/app/src/components/dialog-settings.tsxpackages/app/src/components/session/session-header.tsxpackages/app/src/components/settings-general.tsxpackages/app/src/components/settings-keybinds.tsxpackages/app/src/components/settings-models.tsxpackages/app/src/context/command.tsxpackages/app/src/context/global-sync/bootstrap.tspackages/app/src/context/layout.test.tspackages/app/src/context/layout.tsxpackages/app/src/context/local.tsxpackages/app/src/context/models.tsxpackages/app/src/context/platform.tsxpackages/app/src/context/server.test.tspackages/app/src/context/server.tsxpackages/app/src/context/settings.tsxpackages/app/src/i18n/en.tspackages/app/src/index.csspackages/app/src/pages/directory-layout.tsxpackages/app/src/pages/layout.tsxpackages/app/src/pages/layout/deep-links.tspackages/app/src/pages/layout/helpers.test.tspackages/app/src/pages/layout/helpers.tspackages/app/src/pages/layout/sidebar-items.tsxpackages/app/src/pages/layout/sidebar-project.tsxpackages/app/src/pages/layout/sidebar-shell.tsxpackages/app/src/pages/layout/sidebar-workspace.tsxpackages/app/src/pages/session.tsxpackages/app/src/pages/session/helpers.test.tspackages/app/src/pages/session/helpers.tspackages/app/src/pages/session/message-timeline.tsxpackages/app/src/pages/session/terminal-panel.tsxpackages/app/src/pages/session/use-session-commands.tsxpackages/desktop/package.jsonpackages/desktop/scripts/predev.tspackages/desktop/src-tauri/capabilities/default.jsonpackages/desktop/src-tauri/src/constants.rspackages/desktop/src-tauri/src/lib.rspackages/desktop/src-tauri/src/server.rspackages/desktop/src-tauri/tauri.session-modal.conf.jsonpackages/desktop/src/bindings.tspackages/desktop/src/i18n/en.tspackages/desktop/src/index.tsxpackages/desktop/src/menu.tspackages/opencode/script/preload.jspackages/opencode/src/cli/cmd/desktop.tspackages/opencode/src/cli/cmd/tui/component/prompt/autocomplete.tsxpackages/opencode/src/config/config.tspackages/opencode/src/index.tspackages/opencode/src/project/project.tspackages/opencode/src/server/routes/session.tspackages/opencode/src/session/index.tspackages/opencode/src/util/which.tspackages/opencode/test/config/config.test.tspackages/opencode/test/project/project.test.tspackages/script/src/index.tspackages/script/src/version.test.tspackages/script/src/version.tspackages/session-modal/.gitignorepackages/session-modal/README.mdpackages/session-modal/index.htmlpackages/session-modal/package.jsonpackages/session-modal/src/desktop-bindings.tspackages/session-modal/src/main.tsxpackages/session-modal/src/styles.csspackages/session-modal/tsconfig.jsonpackages/session-modal/vite.config.tspackages/ui/src/components/markdown-copy.test.tspackages/ui/src/components/markdown-copy.tspackages/ui/src/components/markdown-file-ref.test.tspackages/ui/src/components/markdown-file-ref.tspackages/ui/src/components/markdown.csspackages/ui/src/components/markdown.tsxpackages/ui/src/components/message-part.csspackages/ui/src/components/message-part.tsxpackages/ui/src/components/session-turn.tsxpackages/ui/src/context/data.tsxpackages/ui/src/context/dialog.tsxpackages/ui/src/hooks/filter-search.test.tspackages/ui/src/hooks/filter-search.tspackages/ui/src/hooks/use-filtered-list.tsxpackages/ui/src/i18n/en.tspackages/util/src/session-transcript.tspackages/web/src/content/docs/keybinds.mdxscript/check-preload.ts
| fn clone_with_git(url: &str, target: &str) -> Result<(), String> { | ||
| tracing::info!(%url, %target, "Running git clone"); | ||
| let output = Command::new("git") | ||
| .args(["clone", "--", url, target]) | ||
| .output() | ||
| .map_err(|e| format!("Failed to run git clone: {e}"))?; | ||
|
|
||
| if output.status.success() { | ||
| tracing::info!(%target, "git clone completed"); | ||
| return Ok(()); | ||
| } | ||
|
|
||
| let stderr = String::from_utf8_lossy(&output.stderr).trim().to_string(); | ||
| if !stderr.is_empty() { | ||
| tracing::warn!(%url, %target, stderr = %stderr, "git clone failed"); | ||
| return Err(stderr); | ||
| } | ||
|
|
||
| tracing::warn!(%url, %target, "git clone failed without stderr"); | ||
| Err("git clone failed".to_string()) |
There was a problem hiding this comment.
Redact clone URLs before logging them.
These logs persist the raw repository URL. If a user clones with embedded credentials or a PAT in the URL, the secret is written to disk.
🔒 Suggested fix
fn clone_with_git(url: &str, target: &str) -> Result<(), String> {
- tracing::info!(%url, %target, "Running git clone");
+ let repo = repo_name(url);
+ tracing::info!(repo = %repo, %target, "Running git clone");
@@
- tracing::warn!(%url, %target, stderr = %stderr, "git clone failed");
+ tracing::warn!(repo = %repo, %target, stderr = %stderr, "git clone failed");
@@
- tracing::info!(%url, ?directory, "clone_git_repository requested");
+ tracing::info!(repo = %repo_name(&url), ?directory, "clone_git_repository requested");Also applies to: 425-431
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/desktop/src-tauri/src/lib.rs` around lines 339 - 358, The logs in
clone_with_git leak raw repository URLs (which may contain credentials);
implement a small helper like redact_url(url: &str) -> String that strips or
masks userinfo (username:password@) and any PATs from the URL, then replace uses
of %url in tracing::info! and tracing::warn! inside clone_with_git with the
redacted value; do the same replacement in the other git helper at the second
location (the function around lines 425-431) so all git-related log calls use
redact_url(url) instead of the raw url.
|
This pull request has been automatically closed because it was not updated to meet our contributing guidelines within the 2-hour window. Feel free to open a new pull request that follows our guidelines. |
6 participants
Summary
devas a squashed tracking branch that is easier to review thanlocal/all-work.anomalyco/opencode#17018on top of the existing stack.Upstream PRs referenced
desktopcommand for deep-linking into desktop app anomalyco/opencode#15479 - feat(opencode): adddesktopcommand for deep-linking into desktop appNotes
clean/all-workremains the clean tracking branch for the combined work.local/all-workPR is still available as an unsquashed reference branch if needed.