feat(web): task-centric redesign — enclosed shell, full-page settings, multi-project task tree#82
Merged
Merged
Conversation
…, multi-project task tree
UI redesign:
- Borderless sidebar + top bar shell wrapping an inset surface chat panel (包裹感);
removed the earlier outer window-frame attempt in favor of one continuous shell.
- Settings converted from a centered modal to a full page that mirrors the chat
shell layout (full-height rail + top bar + inset surface content panel).
- Welcome/onboarding (SetupView) rebranded onto orange tokens; unified off-brand
emerald/zinc/hardcoded colors to design tokens across TopBar, ProjectSwitcher,
DiffViewer, ChatInput; added a saturated --color-success status token.
Task architecture — Phase 0 (design: docs/web-task-architecture.md):
- GET /api/workspace exposes git branch + dirty (fixes the hardcoded-null branch
in TopBar).
- GET /api/tasks lists sessions across ALL projects; PATCH /api/tasks/{id} updates
pin/archive/unread/title (SessionMeta gained these additive fields).
- Sidebar rebuilt as a Workspace > Project > Task tree: expandable projects,
per-project counts, pinned-first sorting, unread dots, relative time, and a
per-task context menu (pin/rename/archive/mark-unread/delete).
- submit-pr builtin skill: the agent commits/pushes/opens a PR when triggered by
a user query (no manual git UI).
- ⌘K command palette (cross-project task search + quick actions) and browser
notifications on run-finished / approval-needed.
Deferred (documented): true parallel task execution (Phase 1-2, large server
refactor) and composer $/# entry points (in jcode `/` already = skills, and
cross-conversation linking needs the Phase-1 task model).
Tests: internal/web/tasks_test.go (workspace + cross-project list + metadata),
internal/skills/skills_test.go (submit-pr loads). go vet, go build ./...,
vue-tsc and vite build all pass.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 38 minutes and 36 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. 📝 WalkthroughWalkthroughExtends Task-centric workspace and cross-project task management
Remote SSH workspace connection wizard
CSS variable theming and design system
Desktop app (Tauri)
CI, build infrastructure, and support
Sequence Diagram(s)sequenceDiagram
rect rgba(99, 102, 241, 0.5)
Note over User,API: Ctrl+K CommandPalette task selection
User->>App.vue: Ctrl/Cmd+K keydown
App.vue->>CommandPalette: paletteOpen=true
CommandPalette->>useBranch: (branch state ready)
CommandPalette->>useProjectStore: fetchAllTasks()
useProjectStore->>API: GET /api/tasks
API->>Server: ListAllSessions(all projects)
Server->>SessionStore: load shared index
Server-->>API: TaskItem[] by project
API-->>useProjectStore: populate allTasks
CommandPalette->>User: render palette with tasks
User->>CommandPalette: select task (arrow + Enter)
CommandPalette->>useProjectStore: updateTaskMeta(uuid, {unread:false})
useProjectStore->>API: PATCH /api/tasks/:id with UpdatedAt
API->>Server: UpdateSessionMeta(uuid, mutate)
Server-->>API: updated meta
API-->>useProjectStore: confirm
CommandPalette->>chatStore: resetToWelcomeAfterSwitch(), loadSession(uuid)
CommandPalette->>App.vue: emit('close')
end
rect rgba(16, 185, 129, 0.5)
Note over WebSocket,User: Notification on agent completion
WebSocket->>App.vue: onAgentDone(success/error)
App.vue->>useNotifications: notify(title, message)
useNotifications->>User: Browser Notification (tab hidden, permission granted)
end
rect rgba(168, 85, 247, 0.5)
Note over User,Server: Remote SSH workspace bind flow
User->>RemoteConnectWizard: open with prefill (seamless reconnect)
RemoteConnectWizard->>API: POST /api/remote/connect (key auth)
API->>Server: remote.Connect(SSHOptions)
Server->>Server: remote.DiscoverPwd, register pending
API-->>RemoteConnectWizard: connection_id
RemoteConnectWizard->>API: POST /api/remote/list-dir
API->>Server: remote.ListDirs
Server-->>API: subdirectories
User->>RemoteConnectWizard: select directory, bind
RemoteConnectWizard->>API: POST /api/remote/bind
API->>Server: switchToRemote(executor, path)
Server->>Server: rebuild agent, close PTYs, update pwd/recorder
Server-->>API: bind success with label
API-->>RemoteConnectWizard: complete
RemoteConnectWizard->>projectStore: upsertRemoteProject(label, remote)
RemoteConnectWizard->>chatStore: resetToWelcomeAfterSwitch()
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate 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 |
![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
🧹 Nitpick comments (1)
web/src/components/SetupView.vue (1)
172-172: 💤 Low value
--color-on-primarytoken not defined in tokens.css.The code references
var(--color-on-primary,#fff)with a fallback to white, which prevents breakage. However, for consistency and maintainability, consider adding--color-on-primarytotokens.cssfor both light and dark themes.Also applies to: 366-366
🤖 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 `@web/src/components/SetupView.vue` at line 172, The CSS variable --color-on-primary is referenced in SetupView.vue at lines 172 and 366 with a white fallback, but the variable is not defined in tokens.css. Add --color-on-primary to tokens.css for both light and dark themes to eliminate the reliance on the fallback value and ensure consistent theming across the application. No changes are needed in SetupView.vue itself as the existing code with the fallback will work correctly once the variable is defined in tokens.css.
🤖 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 `@internal/web/server.go`:
- Around line 447-465: The handleWorkspace function is using the server context
(s.ctx) for git command execution instead of the request context, which means
operations won't be cancelled when clients disconnect. Replace both occurrences
of s.ctx in the exec.CommandContext calls (for the branchCmd and statusCmd) with
r.Context() to ensure proper request lifecycle management and cancellation when
the HTTP client disconnects.
---
Nitpick comments:
In `@web/src/components/SetupView.vue`:
- Line 172: The CSS variable --color-on-primary is referenced in SetupView.vue
at lines 172 and 366 with a white fallback, but the variable is not defined in
tokens.css. Add --color-on-primary to tokens.css for both light and dark themes
to eliminate the reliance on the fallback value and ensure consistent theming
across the application. No changes are needed in SetupView.vue itself as the
existing code with the fallback will work correctly once the variable is defined
in tokens.css.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8fb1e853-76fc-4fd0-8357-43c4ba6f1e40
📒 Files selected for processing (20)
docs/web-task-architecture.mdinternal/session/session.gointernal/skills/builtin/submit-pr/SKILL.mdinternal/skills/skills_test.gointernal/web/server.gointernal/web/tasks_test.goweb/src/App.vueweb/src/components/ChatInput.vueweb/src/components/CommandPalette.vueweb/src/components/DiffViewer.vueweb/src/components/ProjectSwitcher.vueweb/src/components/SettingsDialog.vueweb/src/components/SetupView.vueweb/src/components/Sidebar.vueweb/src/components/TopBar.vueweb/src/composables/api.tsweb/src/composables/notifications.tsweb/src/stores/project.tsweb/src/styles/tokens.cssweb/src/types/api.ts
- DeleteSessionByUUID resolves the owning project across all projects so a task deleted from the sidebar tree that is NOT in the active project is actually removed from the index (previously a silent no-op left a ghost entry while the JSONL was still deleted). Only removes the file when the uuid exists in the index, which also prevents a crafted uuid from deleting an arbitrary file. Adds TestDeleteTaskCrossProject. (found by self-review) - handleWorkspace now uses r.Context() instead of s.ctx so the git commands are cancelled when the client disconnects. (CodeRabbit review feedback on #82) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
…polish Add a Tauri 2 desktop app under desktop/ that embeds the jcode Go binary as a sidecar and loads the existing Vue web UI over loopback, with native notifications, menu-bar tray, single-instance focus, window-state memory, and a global toggle shortcut. The same bundle degrades to a plain browser via the feature-detected useDesktop.ts bridge. Includes the supporting work this branch accumulated: - Web SSH remote-connect wizard (internal/remote + /api/remote/*, ssh:// sessions) - Request-scoped web-server origin gate (internal/web/server.go) closing the loopback RCE surface; cors_test.go coverage - Inline workspace/branch pickers, unified borderless titlebar chrome, Codex-style composer, hidden-but-scrollable scrollbars, non-selectable app chrome - Phantom-empty-session fix (gate todo/goal OnUpdate on HasRecording) Docs: docs/desktop.md plus updates across the docs set. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add .github/workflows/ci.yml — a quality gate on push-to-main and PRs: Go build/vet/test + golangci-lint (only-new-issues, so the existing lint debt doesn't block) and web type-check/lint/build. Extend release.yml with a `desktop` matrix job that builds the Tauri bundles on native runners for macOS Intel (x86_64-apple-darwin), macOS Apple Silicon (aarch64-apple-darwin), Windows, and Linux, and attaches the .dmg/.msi/.exe/.deb/.AppImage installers (with sha256) to the GitHub Release alongside the CLI binaries. macOS code-signing + notarization wire through optional APPLE_* secrets (unset -> unsigned build that still runs). A "Set up macOS signing & notarization" step supports both notarization methods and picks one: App Store Connect API key (.p8 base64 secret decoded to disk; recommended) or Apple ID + app-specific password. Required certificates/secrets documented in docs/release.md. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The first CI run failed because gitignored generated artifacts were absent: - Go job: internal/web embeds dist/* via //go:embed, so the module won't compile without `make build-web`. - Web job: vue-tsc can't resolve ./themes.generated until the theme generator runs (go generate ./internal/theme/...). Add the missing steps to each job — the go job now also sets up Node/pnpm to build the frontend, the web job sets up Go to run the theme generator (theme only, no models.dev fetch). Verified locally: theme regen + vue-tsc pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 18
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/src/components/TopBar.vue (1)
33-36:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winKeep the status label priority aligned with the status color.
When
isRunningandwsConnectedare both true, the dot shows the running color but the tooltip says “Connected”. CheckisRunningfirst here too.Proposed fix
const statusLabel = computed(() => { - if (props.wsConnected) return 'Connected' if (props.isRunning) return 'Running' + if (props.wsConnected) return 'Connected' return 'Disconnected' })🤖 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 `@web/src/components/TopBar.vue` around lines 33 - 36, The statusLabel computed property checks wsConnected before isRunning, but the visual status indicator prioritizes the running state. Reorder the conditions in the statusLabel computed property to check isRunning before wsConnected, so that when both are true, the label displays "Running" instead of "Connected", keeping the tooltip consistent with the visual color priority of the status dot.
🟡 Minor comments (3)
desktop/src-tauri/tauri.conf.json-10-10 (1)
10-10:⚠️ Potential issue | 🟡 MinorTauri APIs are actively required; add restrictive CSP for defense-in-depth instead.
withGlobalTauri: trueis necessary because the codebase actively uses Tauri plugins (notifications, dialogs, opener) inweb/src/composables/useDesktop.ts. The capability system already restricts Tauri API access to localhost origins only (default.jsonlimits "remote" tohttp://127.0.0.1:*), and the UI is served from the embedded Go sidecar, providing origin-based hardening.However,
csp: nullreduces defense-in-depth. Consider adding a restrictive CSP aligned with the sidecar-origin model (e.g.,default-src 'self' http://127.0.0.1:*; script-src 'self' 'wasm-unsafe-eval') to harden against renderer-side injection in the localhost-served content.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@desktop/src-tauri/tauri.conf.json` at line 10, The tauri.conf.json file currently has csp set to null, which removes Content Security Policy protection and reduces defense-in-depth security. Keep withGlobalTauri: true as it is necessary for the Tauri plugins used in useDesktop.ts, but replace the csp: null configuration with a restrictive CSP policy that aligns with the sidecar-origin model. The CSP should allow resources from self and localhost on any port (http://127.0.0.1:*) for default sources, and specifically allow wasm-unsafe-eval for scripts to support WebAssembly execution while maintaining security hardening against renderer-side injection attacks on the localhost-served content.web/src/components/SettingsDialog.vue-52-56 (1)
52-56:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winIPv6 aliases can be parsed into an invalid host/port pair.
Using
lastIndexOf(':')to splithost:portbreaks IPv6 forms (multiple colons), which can truncate the host and derive a wrong port inconnectToAlias.💡 Proposed fix
- const colon = host.lastIndexOf(':') - if (colon >= 0) { - port = parseInt(host.slice(colon + 1), 10) || 22 - host = host.slice(0, colon) - } + const bracketed = host.match(/^\[(.+)\](?::(\d+))?$/) + if (bracketed) { + host = bracketed[1] + port = bracketed[2] ? parseInt(bracketed[2], 10) || 22 : 22 + } else if (host.includes(':') && host.indexOf(':') === host.lastIndexOf(':')) { + const colon = host.lastIndexOf(':') + port = parseInt(host.slice(colon + 1), 10) || 22 + host = host.slice(0, colon) + }🤖 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 `@web/src/components/SettingsDialog.vue` around lines 52 - 56, The host:port parsing logic using lastIndexOf(':') incorrectly handles IPv6 addresses which contain multiple colons. In the connectToAlias method where the colon-based splitting occurs, check if the host string begins with an opening bracket '[' to detect IPv6 format. For IPv6 addresses (enclosed in brackets like [::1]:22), extract the port by finding the closing bracket ']' first, then check for a colon and port after it. For non-IPv6 formats (IPv4 or hostnames), keep the existing lastIndexOf(':') logic. This ensures IPv6 addresses are not truncated and ports are correctly parsed.internal/remote/ssh.go-69-89 (1)
69-89:⚠️ Potential issue | 🟡 MinorFix error wrapping on line 124 to use
%winstead of%v.Line 124 uses
%vinstead of%w, breaking error chain unwrapping and violating the coding guideline forinternal/**/*.gofiles: "Usefmt.Errorf("tool_name: %w", err)for wrapped errors in non-tool code." Change to:return nil, fmt.Errorf("ls %s: %w", path, err)🤖 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 `@internal/remote/ssh.go` around lines 69 - 89, Locate the error formatting statement on line 124 that currently uses the %v verb for error wrapping and change it to use %w instead. This applies to the error message that formats a path or filename with the error value. The change ensures proper error chain unwrapping and complies with the guideline for error handling in internal/**/*.go files, which requires using fmt.Errorf with %w for wrapped errors. Replace %v with %w in the error formatting call to enable error chain unwrapping.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.
Inline comments:
In @.github/workflows/ci.yml:
- Line 22: The `actions/checkout@v5` actions on lines 22 and 62 use the default
setting of `persist-credentials: true`, which unnecessarily leaves the GitHub
token in the file system for the job duration. Add the `persist-credentials:
false` parameter explicitly to both checkout action usages to disable credential
persistence and reduce the attack surface for token exfiltration.
- Line 22: Replace all action version references that use major version tags
(such as `@v5`, `@v6`, `@v7`) with pinned commit hashes throughout the workflow file.
Locate each action reference (such as actions/checkout and any other actions
used) on lines 22, 27, 47, 62, and 64, and update them from the format
`action-name@vX` to `action-name@{commit-hash}` where the commit hash is
obtained from the action's latest release page or GitHub API. This ensures the
workflow uses specific, immutable versions rather than moving target tags.
- Around line 74-81: The Lint step in the CI workflow currently only runs oxlint
via `npx oxlint .` but skips the eslint checks that are part of the complete
linting workflow defined in the project. Update the Lint step to change the run
command from `npx oxlint .` to `cd web && pnpm lint` to execute the full linting
workflow that includes both oxlint and eslint checks as configured in the
project's package.json.
In @.github/workflows/release.yml:
- Around line 241-270: The version strings from GitHub Actions context are being
injected directly into bash and Node.js scripts, creating command injection
vulnerabilities. In the get_version step, add validation to ensure the version
matches a semantic version regex pattern before outputting it. In the Sync
bundle version and Build sidecar steps, pass the version as an environment
variable instead of embedding it directly in inline scripts, then modify the
Node.js script that updates tauri.conf.json to read the version from the
environment variable rather than using template literal injection, and ensure
the go build command similarly references the version through environment
variables instead of direct interpolation.
- Around line 155-218: The desktop release job uses tag-pinned action references
such as actions/checkout@v5, actions/setup-go@v6, dtolnay/rust-toolchain@stable,
and swatinem/rust-cache@v2, which creates supply chain security risk. Replace
all tag-pinned action references throughout the desktop job configuration with
their corresponding full commit SHAs to ensure immutable versions. For each
action, look up its latest stable release commit SHA and update the reference
from the tag format (e.g., `@v5`) to the full commit SHA format to harden the
workflow against supply chain attacks.
In `@desktop/src-tauri/src/sidecar.rs`:
- Around line 18-31: Implement nonce-based authentication between the Rust
launcher and the sidecar to prevent port spoofing attacks. In the start()
function, generate an unguessable nonce before calling pick_free_port(), then
ensure this nonce is passed to the sidecar during initialization. Modify the
health check probe (currently at line 144) to require the sidecar echo back the
nonce in its /api/health response before calling w.navigate() to the webview.
This prevents an attacker's spoofed listener from satisfying the health check
and gaining access to privileged Tauri capabilities via the wildcard "remote"
capability.
In `@internal/command/web.go`:
- Around line 398-403: After updating the systemPrompt on line 402 with the
remote environment details (remotePlatform, remotePwd, and envLabel), you must
also update the planPrompt variable in the same way to ensure consistency. Use
the same parameters (remotePlatform, remotePwd, envLabel, and
skillLoader.Descriptions()) that are used for the systemPrompt update so that
when currentPlanMode is true and line 417 rebuilds the prompt context, it
references the correct remote environment instead of stale context.
- Around line 391-419: The remote switch is applying irreversible state changes
to env, approvalState, systemPrompt, pwd, and rec before validating that
createAgent succeeds on line 417. If agent creation fails, the runtime is left
in a partially switched state. Reorganize the code to call createAgent first,
before making any of the state mutations. Only apply the state changes to
env.SetSSH, approvalState.SetWorkpath, systemPrompt assignment, pwd assignment,
rec.Close/NewRecorder, and related updates if and after the agent creation
succeeds, ensuring the runtime remains consistent if the operation fails.
In `@internal/remote/ssh.go`:
- Around line 94-101: The Connect function does not accept context.Context as
its first parameter, preventing callers from threading cancellation during SSH
dial operations. Add context.Context as the first parameter to the Connect
function signature (before SSHOptions), then pass this context through to the
tools.NewSSHExecutor call to enable proper cancellation propagation. This aligns
with the pattern already used by other remote operations like DiscoverPwd and
ListDirs in the same package, and matches the coding guidelines for
network-bound APIs.
In `@internal/web/git.go`:
- Around line 86-94: The branch name in the git checkout command is not
validated before being passed as an argument, allowing branch names that start
with a dash to be interpreted as git options (option injection vulnerability).
Add validation using git check-ref-format --branch command to validate the
branch name before constructing the args slice in the section where you build
the git command arguments. Additionally, consider refactoring the git command
from checkout to use git switch (with the -c flag when req.Create is true) for
better clarity and security, ensuring the branch variable is validated before it
is appended to the args slice.
In `@internal/web/remote.go`:
- Around line 53-57: The remoteConnRegistry class only enforces TTL cleanup
during the add() method, causing expired pending SSH sessions to remain open
indefinitely and leak resources. Add a call to rg.sweepLocked() at the beginning
of the get() method (right after acquiring the lock) and the take() method
(right after acquiring the lock) to ensure expired connections are cleaned up
during these common access paths, preventing resource leaks from accumulated
stale SSHExecutor connections.
- Around line 204-237: The code has a race condition where `get()` retrieves the
connection without claiming ownership, allowing a concurrent cancel request to
close the executor while handleRemoteBind is still using it. Replace the `get()`
call at line 204 with `take()` to atomically claim ownership of the connection
at the start. Since `take()` removes the connection from the registry, add a
check in the error path after the switchToRemote call to handle cleanup if an
error occurs (since ownership was already transferred). Finally, remove the
redundant `take()` call at the end since the connection ownership is already
claimed at the start.
In `@internal/web/server.go`:
- Around line 2963-2967: The CORS preflight configuration in the
isAllowedWebOrigin check is missing the PATCH HTTP method in the
Access-Control-Allow-Methods header, but the API now supports PATCH for task
updates. Add PATCH to the comma-separated list of allowed methods in the
Access-Control-Allow-Methods header line, placing it alongside the existing GET,
POST, PUT, DELETE, and OPTIONS methods.
- Around line 84-86: The switchToRemote callback function signature is missing
context.Context as the first parameter, preventing the handler from properly
threading request context for cancellation. Add context.Context as the first
parameter to the switchToRemote function signature in the server.go file, then
update the call site in the handler (line 217 in remote.go) to pass r.Context()
when invoking the callback. Apply the same change to the other callback
signatures mentioned at lines 146 and 188 to ensure consistent context threading
across all remote binding operations.
In `@Makefile`:
- Around line 75-79: The RUST_TARGET variable may be empty if the rustc command
fails or produces no output, which causes SIDECAR_EXE to have an incorrect value
that won't match Tauri's expected external-bin naming convention. Add a check
immediately after the RUST_TARGET assignment that halts the build with an error
message if RUST_TARGET is empty, ensuring the sidecar name generated by
SIDECAR_EXE is always valid and prevents confusing runtime failures downstream.
- Line 93: The fallback logic in both the desktop dev and build targets (lines
93 and 97) attempts to install packages with pnpm first and falls back to npm if
pnpm is unavailable, but then unconditionally executes pnpm tauri dev or pnpm
tauri build. This causes the targets to fail when pnpm is not installed since
the fallback only covers the install step. Update both targets to ensure the
tauri invocation also has a fallback mechanism, either by using the same package
manager that was successfully used for installation or by applying the
conditional fallback pattern to the tauri command as well.
In `@web/src/components/RemoteConnectWizard.vue`:
- Around line 117-120: The host parsing logic in both locations (lines 117-120
and 178-181) using lastIndexOf(':') incorrectly splits IPv6 addresses since
colons are part of the address format. To fix this, improve the host and port
parsing to properly detect IPv6 addresses (either in bracket notation like
[2001:db8::10]:8080 or plain format) and only treat the last colon as a port
separator for non-IPv6 addresses. For IPv6 addresses in bracket notation,
extract the address from within the brackets and parse the port after the
closing bracket. For plain IPv6 addresses without explicit port notation, assign
the address to form.host without splitting on colons and use the default port
value.
In `@web/src/stores/project.ts`:
- Around line 25-27: The return statement in the RemoteMeta object is setting
host to the full hostPort string, which includes the port number when present
(e.g., "example.com:2222"). This breaks the API payload semantics. Use the colon
variable that was already calculated to extract just the hostname portion from
hostPort before returning it - when colon is >= 0, set host to hostPort.slice(0,
colon), otherwise set host to hostPort. This ensures the host field contains
only the hostname without the port number in the RemoteMeta return object.
---
Outside diff comments:
In `@web/src/components/TopBar.vue`:
- Around line 33-36: The statusLabel computed property checks wsConnected before
isRunning, but the visual status indicator prioritizes the running state.
Reorder the conditions in the statusLabel computed property to check isRunning
before wsConnected, so that when both are true, the label displays "Running"
instead of "Connected", keeping the tooltip consistent with the visual color
priority of the status dot.
---
Minor comments:
In `@desktop/src-tauri/tauri.conf.json`:
- Line 10: The tauri.conf.json file currently has csp set to null, which removes
Content Security Policy protection and reduces defense-in-depth security. Keep
withGlobalTauri: true as it is necessary for the Tauri plugins used in
useDesktop.ts, but replace the csp: null configuration with a restrictive CSP
policy that aligns with the sidecar-origin model. The CSP should allow resources
from self and localhost on any port (http://127.0.0.1:*) for default sources,
and specifically allow wasm-unsafe-eval for scripts to support WebAssembly
execution while maintaining security hardening against renderer-side injection
attacks on the localhost-served content.
In `@internal/remote/ssh.go`:
- Around line 69-89: Locate the error formatting statement on line 124 that
currently uses the %v verb for error wrapping and change it to use %w instead.
This applies to the error message that formats a path or filename with the error
value. The change ensures proper error chain unwrapping and complies with the
guideline for error handling in internal/**/*.go files, which requires using
fmt.Errorf with %w for wrapped errors. Replace %v with %w in the error
formatting call to enable error chain unwrapping.
In `@web/src/components/SettingsDialog.vue`:
- Around line 52-56: The host:port parsing logic using lastIndexOf(':')
incorrectly handles IPv6 addresses which contain multiple colons. In the
connectToAlias method where the colon-based splitting occurs, check if the host
string begins with an opening bracket '[' to detect IPv6 format. For IPv6
addresses (enclosed in brackets like [::1]:22), extract the port by finding the
closing bracket ']' first, then check for a colon and port after it. For
non-IPv6 formats (IPv4 or hostnames), keep the existing lastIndexOf(':') logic.
This ensures IPv6 addresses are not truncated and ports are correctly parsed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 740791a7-e0e6-4772-97b7-cc109c819b94
⛔ Files ignored due to path filters (52)
desktop/pnpm-lock.yamlis excluded by!**/pnpm-lock.yamldesktop/src-tauri/Cargo.lockis excluded by!**/*.lockdesktop/src-tauri/icons/128x128.pngis excluded by!**/*.pngdesktop/src-tauri/icons/128x128@2x.pngis excluded by!**/*.pngdesktop/src-tauri/icons/32x32.pngis excluded by!**/*.pngdesktop/src-tauri/icons/64x64.pngis excluded by!**/*.pngdesktop/src-tauri/icons/Square107x107Logo.pngis excluded by!**/*.pngdesktop/src-tauri/icons/Square142x142Logo.pngis excluded by!**/*.pngdesktop/src-tauri/icons/Square150x150Logo.pngis excluded by!**/*.pngdesktop/src-tauri/icons/Square284x284Logo.pngis excluded by!**/*.pngdesktop/src-tauri/icons/Square30x30Logo.pngis excluded by!**/*.pngdesktop/src-tauri/icons/Square310x310Logo.pngis excluded by!**/*.pngdesktop/src-tauri/icons/Square44x44Logo.pngis excluded by!**/*.pngdesktop/src-tauri/icons/Square71x71Logo.pngis excluded by!**/*.pngdesktop/src-tauri/icons/Square89x89Logo.pngis excluded by!**/*.pngdesktop/src-tauri/icons/StoreLogo.pngis excluded by!**/*.pngdesktop/src-tauri/icons/android/mipmap-hdpi/ic_launcher.pngis excluded by!**/*.pngdesktop/src-tauri/icons/android/mipmap-hdpi/ic_launcher_foreground.pngis excluded by!**/*.pngdesktop/src-tauri/icons/android/mipmap-hdpi/ic_launcher_round.pngis excluded by!**/*.pngdesktop/src-tauri/icons/android/mipmap-mdpi/ic_launcher.pngis excluded by!**/*.pngdesktop/src-tauri/icons/android/mipmap-mdpi/ic_launcher_foreground.pngis excluded by!**/*.pngdesktop/src-tauri/icons/android/mipmap-mdpi/ic_launcher_round.pngis excluded by!**/*.pngdesktop/src-tauri/icons/android/mipmap-xhdpi/ic_launcher.pngis excluded by!**/*.pngdesktop/src-tauri/icons/android/mipmap-xhdpi/ic_launcher_foreground.pngis excluded by!**/*.pngdesktop/src-tauri/icons/android/mipmap-xhdpi/ic_launcher_round.pngis excluded by!**/*.pngdesktop/src-tauri/icons/android/mipmap-xxhdpi/ic_launcher.pngis excluded by!**/*.pngdesktop/src-tauri/icons/android/mipmap-xxhdpi/ic_launcher_foreground.pngis excluded by!**/*.pngdesktop/src-tauri/icons/android/mipmap-xxhdpi/ic_launcher_round.pngis excluded by!**/*.pngdesktop/src-tauri/icons/android/mipmap-xxxhdpi/ic_launcher.pngis excluded by!**/*.pngdesktop/src-tauri/icons/android/mipmap-xxxhdpi/ic_launcher_foreground.pngis excluded by!**/*.pngdesktop/src-tauri/icons/android/mipmap-xxxhdpi/ic_launcher_round.pngis excluded by!**/*.pngdesktop/src-tauri/icons/icon.icois excluded by!**/*.icodesktop/src-tauri/icons/icon.pngis excluded by!**/*.pngdesktop/src-tauri/icons/ios/AppIcon-20x20@1x.pngis excluded by!**/*.pngdesktop/src-tauri/icons/ios/AppIcon-20x20@2x-1.pngis excluded by!**/*.pngdesktop/src-tauri/icons/ios/AppIcon-20x20@2x.pngis excluded by!**/*.pngdesktop/src-tauri/icons/ios/AppIcon-20x20@3x.pngis excluded by!**/*.pngdesktop/src-tauri/icons/ios/AppIcon-29x29@1x.pngis excluded by!**/*.pngdesktop/src-tauri/icons/ios/AppIcon-29x29@2x-1.pngis excluded by!**/*.pngdesktop/src-tauri/icons/ios/AppIcon-29x29@2x.pngis excluded by!**/*.pngdesktop/src-tauri/icons/ios/AppIcon-29x29@3x.pngis excluded by!**/*.pngdesktop/src-tauri/icons/ios/AppIcon-40x40@1x.pngis excluded by!**/*.pngdesktop/src-tauri/icons/ios/AppIcon-40x40@2x-1.pngis excluded by!**/*.pngdesktop/src-tauri/icons/ios/AppIcon-40x40@2x.pngis excluded by!**/*.pngdesktop/src-tauri/icons/ios/AppIcon-40x40@3x.pngis excluded by!**/*.pngdesktop/src-tauri/icons/ios/AppIcon-512@2x.pngis excluded by!**/*.pngdesktop/src-tauri/icons/ios/AppIcon-60x60@2x.pngis excluded by!**/*.pngdesktop/src-tauri/icons/ios/AppIcon-60x60@3x.pngis excluded by!**/*.pngdesktop/src-tauri/icons/ios/AppIcon-76x76@1x.pngis excluded by!**/*.pngdesktop/src-tauri/icons/ios/AppIcon-76x76@2x.pngis excluded by!**/*.pngdesktop/src-tauri/icons/ios/AppIcon-83.5x83.5@2x.pngis excluded by!**/*.pngweb/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (72)
.github/workflows/ci.yml.github/workflows/release.ymlMakefileREADME.mddesktop/.gitignoredesktop/package.jsondesktop/pnpm-workspace.yamldesktop/splash/index.htmldesktop/src-tauri/.gitignoredesktop/src-tauri/Cargo.tomldesktop/src-tauri/build.rsdesktop/src-tauri/capabilities/default.jsondesktop/src-tauri/icons/android/mipmap-anydpi-v26/ic_launcher.xmldesktop/src-tauri/icons/android/values/ic_launcher_background.xmldesktop/src-tauri/icons/icon.icnsdesktop/src-tauri/src/main.rsdesktop/src-tauri/src/sidecar.rsdesktop/src-tauri/src/tray.rsdesktop/src-tauri/tauri.conf.jsondocs/changelog.mddocs/commands.mddocs/configuration.mddocs/desktop.mddocs/goal.mddocs/release.mddocs/themes.mddocs/web-interface.mdinternal/command/ssh.gointernal/command/web.gointernal/remote/ssh.gointernal/remote/ssh_test.gointernal/session/index_test.gointernal/tools/env.gointernal/web/cors_test.gointernal/web/git.gointernal/web/remote.gointernal/web/server.goweb/package.jsonweb/src/App.vueweb/src/components/BranchPicker.vueweb/src/components/ChatInput.vueweb/src/components/ChatMessage.vueweb/src/components/CommandPalette.vueweb/src/components/DiffViewer.vueweb/src/components/FileTreePanel.vueweb/src/components/FileViewer.vueweb/src/components/GoalBanner.vueweb/src/components/MCPPanel.vueweb/src/components/ProjectSwitcher.vueweb/src/components/RemoteConnectWizard.vueweb/src/components/RightPanel.vueweb/src/components/SettingsDialog.vueweb/src/components/SetupView.vueweb/src/components/Sidebar.vueweb/src/components/TaskList.vueweb/src/components/TerminalInstance.vueweb/src/components/TerminalPanel.vueweb/src/components/ToolCallCard.vueweb/src/components/TopBar.vueweb/src/components/WorkspacePicker.vueweb/src/composables/api.tsweb/src/composables/notifications.tsweb/src/composables/useBranch.tsweb/src/composables/useDesktop.tsweb/src/composables/useFolderBrowser.tsweb/src/main.tsweb/src/stores/chat.tsweb/src/stores/project.tsweb/src/style.cssweb/src/styles/animations.cssweb/src/styles/tokens.cssweb/src/types/api.ts
✅ Files skipped from review due to trivial changes (20)
- desktop/.gitignore
- web/src/main.ts
- desktop/src-tauri/icons/android/values/ic_launcher_background.xml
- desktop/package.json
- web/src/composables/useFolderBrowser.ts
- README.md
- desktop/src-tauri/.gitignore
- docs/goal.md
- docs/web-interface.md
- web/src/components/FileTreePanel.vue
- web/src/components/TaskList.vue
- desktop/src-tauri/icons/android/mipmap-anydpi-v26/ic_launcher.xml
- docs/themes.md
- docs/configuration.md
- desktop/splash/index.html
- web/src/components/TerminalPanel.vue
- web/src/components/TerminalInstance.vue
- docs/commands.md
- web/src/components/RightPanel.vue
- web/src/components/ChatMessage.vue
🚧 Files skipped from review as they are similar to previous changes (4)
- web/src/components/DiffViewer.vue
- web/src/components/CommandPalette.vue
- web/src/components/SetupView.vue
- web/src/components/Sidebar.vue
| name: Go (build · vet · test · lint) | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v5 |
There was a problem hiding this comment.
Disable credential persistence on checkout actions.
The actions/checkout@v5 actions default to persist-credentials: true, which leaves the GitHub token in ~/.netrc for the duration of the job. This is unnecessary for read-only CI and increases the surface for token exfiltration. Set persist-credentials: false explicitly.
🔒 Proposed fix: disable credential persistence
- uses: actions/checkout@v5
with:
fetch-depth: 0
+ persist-credentials: falseApply the same change to line 62.
Also applies to: 62-62
🧰 Tools
🪛 zizmor (1.25.2)
[warning] 22-25: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 22-22: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 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 @.github/workflows/ci.yml at line 22, The `actions/checkout@v5` actions on
lines 22 and 62 use the default setting of `persist-credentials: true`, which
unnecessarily leaves the GitHub token in the file system for the job duration.
Add the `persist-credentials: false` parameter explicitly to both checkout
action usages to disable credential persistence and reduce the attack surface
for token exfiltration.
Source: Linters/SAST tools
Pin action versions to commit hashes for supply-chain security.
All action references use major version tags (@v5, @v6, @v7), which point to a moving target. Consider pinning to a commit hash (e.g., actions/checkout@a1b2c3d4e5f6...) to protect against unexpected upstream changes and prevent exploitation if an action's tag is mutated. This is especially important for security-sensitive workflows.
🔒 Proposed fix: pin action versions
Example pins (check the latest release commit for each action):
- - uses: actions/checkout@v5
+ - uses: actions/checkout@eef61447b9ff4aafe5dcd4e0bbf5d482be7e7871 # v5
- - uses: actions/setup-go@v6
+ - uses: actions/setup-go@0a12ed9d6470c34bdae9eab0d4af20222284e98da # v6
- uses: golangci/golangci-lint-action@v7
+ uses: golangci/golangci-lint-action@971e8129edad04ea426c2b9e4eec8aaefb378f59 # v7
- - uses: actions/setup-node@v5
+ - uses: actions/setup-node@60edb5dd545925195646e3ea4c3f2f7fb38105ee # v5Find the actual commit SHAs via the GitHub action's release page or gh api repos/{owner}/{repo}/releases/{tag}.
Also applies to: 27-27, 47-47, 62-62, 64-64
🧰 Tools
🪛 zizmor (1.25.2)
[warning] 22-25: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 22-22: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 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 @.github/workflows/ci.yml at line 22, Replace all action version references
that use major version tags (such as `@v5`, `@v6`, `@v7`) with pinned commit hashes
throughout the workflow file. Locate each action reference (such as
actions/checkout and any other actions used) on lines 22, 27, 47, 62, and 64,
and update them from the format `action-name@vX` to `action-name@{commit-hash}`
where the commit hash is obtained from the action's latest release page or
GitHub API. This ensures the workflow uses specific, immutable versions rather
than moving target tags.
Source: Linters/SAST tools
Comment on lines
+74
to
+81
| - name: Type-check | ||
| run: pnpm type-check | ||
|
|
||
| - name: Lint | ||
| run: npx oxlint . | ||
|
|
||
| - name: Build | ||
| run: npx vite build |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for lint script in web/package.json
cat web/package.json | jq '.scripts.lint'Repository: cnjack/jcode
Length of output: 71
🏁 Script executed:
cat web/package.json | jq '.scripts'Repository: cnjack/jcode
Length of output: 353
Update the CI lint command to run the full linting workflow.
The CI runs npx oxlint . but the project defines a complete lint workflow in web/package.json that runs both oxlint and eslint via pnpm lint. The CI is currently skipping the eslint check. Update the lint step to: cd web && pnpm lint (or npm run lint from the web directory) to match the project's linting setup and ensure all linting rules are enforced in CI.
🤖 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 @.github/workflows/ci.yml around lines 74 - 81, The Lint step in the CI
workflow currently only runs oxlint via `npx oxlint .` but skips the eslint
checks that are part of the complete linting workflow defined in the project.
Update the Lint step to change the run command from `npx oxlint .` to `cd web &&
pnpm lint` to execute the full linting workflow that includes both oxlint and
eslint checks as configured in the project's package.json.
Source: Coding guidelines
Comment on lines
+155
to
+218
| desktop: | ||
| needs: prepare | ||
| runs-on: ${{ matrix.os }} | ||
| strategy: | ||
| fail-fast: false | ||
| matrix: | ||
| include: | ||
| # macOS Apple Silicon (M-series) | ||
| - os: macos-latest | ||
| triple: aarch64-apple-darwin | ||
| ext: '' | ||
| cgo: 1 | ||
| bundles: dmg | ||
| label: macos-arm64 | ||
| # macOS Intel | ||
| - os: macos-15-intel | ||
| triple: x86_64-apple-darwin | ||
| ext: '' | ||
| cgo: 1 | ||
| bundles: dmg | ||
| label: macos-x64 | ||
| # Windows x64 | ||
| - os: windows-latest | ||
| triple: x86_64-pc-windows-msvc | ||
| ext: '.exe' | ||
| cgo: 0 | ||
| bundles: 'msi,nsis' | ||
| label: windows-x64 | ||
| # Linux x64 | ||
| - os: ubuntu-22.04 | ||
| triple: x86_64-unknown-linux-gnu | ||
| ext: '' | ||
| cgo: 0 | ||
| bundles: 'deb,appimage' | ||
| label: linux-x64 | ||
|
|
||
| steps: | ||
| - name: Checkout code | ||
| uses: actions/checkout@v5 | ||
| with: | ||
| fetch-depth: 0 | ||
|
|
||
| - name: Set up Go | ||
| uses: actions/setup-go@v6 | ||
| with: | ||
| go-version-file: 'go.mod' | ||
|
|
||
| - name: Set up Node.js | ||
| uses: actions/setup-node@v5 | ||
| with: | ||
| node-version: '22' | ||
|
|
||
| - name: Install pnpm | ||
| run: npm install -g pnpm | ||
|
|
||
| - name: Set up Rust | ||
| uses: dtolnay/rust-toolchain@stable | ||
| with: | ||
| targets: ${{ matrix.triple }} | ||
|
|
||
| - name: Cache Rust | ||
| uses: swatinem/rust-cache@v2 | ||
| with: | ||
| workspaces: desktop/src-tauri |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
python3 - <<'PY'
from pathlib import Path
import re
path = Path(".github/workflows/release.yml")
for lineno, line in enumerate(path.read_text().splitlines(), 1):
match = re.search(r'^\s*uses:\s*([^#\s]+)', line)
if not match:
continue
ref = match.group(1)
if "@" not in ref or not re.search(r'@[0-9a-fA-F]{40}$', ref):
print(f"{path}:{lineno}: {ref}")
PYRepository: cnjack/jcode
Length of output: 1181
🏁 Script executed:
cat -n .github/workflows/release.yml | head -220Repository: cnjack/jcode
Length of output: 7143
🏁 Script executed:
cat -n .github/workflows/release.yml | sed -n '235,370p'Repository: cnjack/jcode
Length of output: 7002
🏁 Script executed:
cat -n .github/workflows/release.yml | sed -n '394,405p'Repository: cnjack/jcode
Length of output: 470
Harden the desktop release job before producing artifacts.
The desktop job (lines 155-218) and related build steps (lines 235-369, 394-405) run with the default checkout token persisted and use tag-pinned action references instead of immutable SHAs. The job inherits the global contents: write permission unnecessarily. Add least-privilege permissions, disable checkout credential persistence, and pin all action references to full commit SHAs.
All 13 actions in the affected ranges are tag-pinned (@v5, @v6, @stable, @v2) rather than SHA-pinned, creating supply chain risk.
🔒 Suggested hardening for the desktop job token boundary
desktop:
needs: prepare
+ permissions:
+ contents: read
runs-on: ${{ matrix.os }}
@@
- name: Checkout code
uses: actions/checkout@v5
with:
fetch-depth: 0
+ persist-credentials: falsePin all workflow action references to full SHAs across lines 155-218, 235-369, and 394-405. Example:
actions/checkout@v5→actions/checkout@ebd315eff573669d375a86f4f7fb08efb87bdc18actions/setup-go@v6→actions/setup-go@0a12ed9d6470115ce6e23a8cdc390f3116122b7adtolnay/rust-toolchain@stable→ use pinned commit SHAswatinem/rust-cache@v2→ use pinned commit SHA
🧰 Tools
🪛 zizmor (1.25.2)
[warning] 192-195: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 193-193: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 198-198: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 203-203: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 211-211: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 216-216: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 198-198: runtime artifacts potentially vulnerable to a cache poisoning attack (cache-poisoning): enables caching by default
(cache-poisoning)
[error] 203-203: runtime artifacts potentially vulnerable to a cache poisoning attack (cache-poisoning): enables caching by default
(cache-poisoning)
[error] 216-216: runtime artifacts potentially vulnerable to a cache poisoning attack (cache-poisoning): enables caching by default
(cache-poisoning)
🤖 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 @.github/workflows/release.yml around lines 155 - 218, The desktop release
job uses tag-pinned action references such as actions/checkout@v5,
actions/setup-go@v6, dtolnay/rust-toolchain@stable, and swatinem/rust-cache@v2,
which creates supply chain security risk. Replace all tag-pinned action
references throughout the desktop job configuration with their corresponding
full commit SHAs to ensure immutable versions. For each action, look up its
latest stable release commit SHA and update the reference from the tag format
(e.g., `@v5`) to the full commit SHA format to harden the workflow against supply
chain attacks.
Source: Linters/SAST tools
Comment on lines
+241
to
+270
| - name: Get version | ||
| id: get_version | ||
| shell: bash | ||
| run: | | ||
| if [ "${{ github.event_name }}" = "workflow_dispatch" ]; then | ||
| VERSION="${{ github.event.inputs.version }}" | ||
| else | ||
| VERSION="${GITHUB_REF#refs/tags/}" | ||
| fi | ||
| echo "version=$VERSION" >> "$GITHUB_OUTPUT" | ||
|
|
||
| - name: Sync bundle version | ||
| shell: bash | ||
| run: | | ||
| VER="${{ steps.get_version.outputs.version }}" | ||
| VER="${VER#v}" | ||
| node -e "const fs=require('fs');const f='desktop/src-tauri/tauri.conf.json';const j=JSON.parse(fs.readFileSync(f));j.version='${VER}';fs.writeFileSync(f, JSON.stringify(j,null,2)+'\n')" | ||
| echo "Set desktop bundle version to ${VER}" | ||
|
|
||
| - name: Build sidecar | ||
| shell: bash | ||
| env: | ||
| CGO_ENABLED: ${{ matrix.cgo }} | ||
| run: | | ||
| VERSION="${{ steps.get_version.outputs.version }}" | ||
| COMMIT="$(git rev-parse --short HEAD)" | ||
| mkdir -p desktop/src-tauri/binaries | ||
| go build -trimpath \ | ||
| -ldflags "-s -w -X github.com/cnjack/jcode/internal/command.Version=${VERSION} -X github.com/cnjack/jcode/internal/command.GitCommit=${COMMIT}" \ | ||
| -o "desktop/src-tauri/binaries/jcode-${{ matrix.triple }}${{ matrix.ext }}" \ |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find and read the workflow file
git ls-files .github/workflows/release.ymlRepository: cnjack/jcode
Length of output: 86
🏁 Script executed:
# Get the file and check the specific lines
cat -n .github/workflows/release.yml | sed -n '240,275p'Repository: cnjack/jcode
Length of output: 1881
🏁 Script executed:
# Let's also check the full context of these steps to understand the flow
cat -n .github/workflows/release.yml | sed -n '200,300p'Repository: cnjack/jcode
Length of output: 4800
Pass version input through environment variables and validate the format before embedding in scripts.
Lines 246, 255, and 265 inject version strings directly from GitHub Actions context expressions into bash and Node scripts, creating injection vulnerabilities. A crafted version like v1.0.0"; rm -rf /; echo " would escape the bash context and execute arbitrary commands. Adopt safer practices:
- Use environment variables to isolate untrusted input from script evaluation
- Validate the version format against a semantic version regex before use
- Pass data to Node/Go through environment variables rather than inline script strings
See suggested implementation in the details section below:
🛡️ Safer version handling
- name: Get version
id: get_version
shell: bash
+ env:
+ WORKFLOW_VERSION: ${{ github.event.inputs.version }}
run: |
- if [ "${{ github.event_name }}" = "workflow_dispatch" ]; then
- VERSION="${{ github.event.inputs.version }}"
+ if [ "$GITHUB_EVENT_NAME" = "workflow_dispatch" ]; then
+ VERSION="$WORKFLOW_VERSION"
else
VERSION="${GITHUB_REF#refs/tags/}"
fi
- echo "version=$VERSION" >> "$GITHUB_OUTPUT"
+ if [[ ! "$VERSION" =~ ^v?[0-9]+[.][0-9]+[.][0-9]+(-[0-9A-Za-z.-]+)?([+][0-9A-Za-z.-]+)?$ ]]; then
+ echo "::error::invalid release version: $VERSION"
+ exit 1
+ fi
+ printf 'version=%s\n' "$VERSION" >> "$GITHUB_OUTPUT"
@@
- name: Sync bundle version
shell: bash
+ env:
+ RELEASE_VERSION: ${{ steps.get_version.outputs.version }}
run: |
- VER="${{ steps.get_version.outputs.version }}"
- VER="${VER#v}"
- node -e "const fs=require('fs');const f='desktop/src-tauri/tauri.conf.json';const j=JSON.parse(fs.readFileSync(f));j.version='${VER}';fs.writeFileSync(f, JSON.stringify(j,null,2)+'\n')"
+ VER="${RELEASE_VERSION#v}"
+ export BUNDLE_VERSION="$VER"
+ node <<'NODE'
+ const fs = require('fs')
+ const f = 'desktop/src-tauri/tauri.conf.json'
+ const j = JSON.parse(fs.readFileSync(f, 'utf8'))
+ j.version = process.env.BUNDLE_VERSION
+ fs.writeFileSync(f, JSON.stringify(j, null, 2) + '\n')
+ NODE
echo "Set desktop bundle version to ${VER}"
@@
env:
CGO_ENABLED: ${{ matrix.cgo }}
+ RELEASE_VERSION: ${{ steps.get_version.outputs.version }}
run: |
- VERSION="${{ steps.get_version.outputs.version }}"
+ VERSION="$RELEASE_VERSION"
COMMIT="$(git rev-parse --short HEAD)"🧰 Tools
🪛 zizmor (1.25.2)
[error] 246-246: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[info] 255-255: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[info] 265-265: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
🤖 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 @.github/workflows/release.yml around lines 241 - 270, The version strings
from GitHub Actions context are being injected directly into bash and Node.js
scripts, creating command injection vulnerabilities. In the get_version step,
add validation to ensure the version matches a semantic version regex pattern
before outputting it. In the Sync bundle version and Build sidecar steps, pass
the version as an environment variable instead of embedding it directly in
inline scripts, then modify the Node.js script that updates tauri.conf.json to
read the version from the environment variable rather than using template
literal injection, and ensure the go build command similarly references the
version through environment variables instead of direct interpolation.
Source: Linters/SAST tools
Comment on lines
+18
to
+31
| /// Ask the OS for an unused loopback port. There is a tiny TOCTOU window | ||
| /// between dropping this listener and the sidecar binding it, which is | ||
| /// acceptable for a local developer tool; the health poll below tolerates a | ||
| /// slow or failed bind. | ||
| fn pick_free_port() -> u16 { | ||
| TcpListener::bind("127.0.0.1:0") | ||
| .and_then(|l| l.local_addr()) | ||
| .map(|a| a.port()) | ||
| .unwrap_or(8799) | ||
| } | ||
|
|
||
| pub fn start(app: &AppHandle) -> Result<(), Box<dyn std::error::Error>> { | ||
| let port = pick_free_port(); | ||
| let url = format!("http://127.0.0.1:{port}"); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the sidecar.rs file to see the code at the specified lines
fd -type f -name "sidecar.rs" | head -5Repository: cnjack/jcode
Length of output: 227
🏁 Script executed:
# List the file to understand its structure
git ls-files | grep -i sidecarRepository: cnjack/jcode
Length of output: 89
🏁 Script executed:
cat -n desktop/src-tauri/src/sidecar.rs | head -150Repository: cnjack/jcode
Length of output: 6253
🏁 Script executed:
# Check the capabilities file mentioned in the review
fd -name "default.json" | xargs catRepository: cnjack/jcode
Length of output: 287
🏁 Script executed:
# Find the Go server health endpoint
fd -name "server.go" | head -1 | xargs cat -n | head -200Repository: cnjack/jcode
Length of output: 287
🏁 Script executed:
# Find and read the capabilities default.json
find . -name "default.json" -type f | head -5Repository: cnjack/jcode
Length of output: 102
🏁 Script executed:
# Look for the Go server implementation
find . -name "server.go" -type f | head -5Repository: cnjack/jcode
Length of output: 81
🏁 Script executed:
cat ./desktop/src-tauri/capabilities/default.jsonRepository: cnjack/jcode
Length of output: 810
🏁 Script executed:
cat ./internal/web/server.go | head -300Repository: cnjack/jcode
Length of output: 11283
🏁 Script executed:
# Search for the health handler in server.go
rg -A 20 "handleHealth" ./internal/web/server.goRepository: cnjack/jcode
Length of output: 1705
🏁 Script executed:
# Check if there's any authentication or nonce mechanism used elsewhere
rg -i "nonce|token|secret" ./desktop/src-tauri/src/sidecar.rsRepository: cnjack/jcode
Length of output: 38
🏁 Script executed:
# Check the full permissions granted by the Tauri capability
cat ./desktop/src-tauri/capabilities/default.json | grep -A 20 "permissions"Repository: cnjack/jcode
Length of output: 464
🏁 Script executed:
# Get the full handleHealth implementation to see the complete response
rg -A 50 "func (s \*Server) handleHealth" ./internal/web/server.goRepository: cnjack/jcode
Length of output: 38
🏁 Script executed:
# Check if there's any auth/nonce passed to the sidecar process
rg -B 5 -A 15 "sidecar.*jcode" ./desktop/src-tauri/src/sidecar.rsRepository: cnjack/jcode
Length of output: 1352
🏁 Script executed:
# Get more complete handleHealth implementation
sed -n '1,/^func (s \*Server) handleHealth/p' ./internal/web/server.go | tail -1 && sed -n '/^func (s \*Server) handleHealth/,/^func (s \*Server)/p' ./internal/web/server.go | head -60Repository: cnjack/jcode
Length of output: 1035
🏁 Script executed:
# Search for any nonce or secret mechanism in the entire codebase
rg -i "nonce|auth.*secret|shared.*secret" ./internal ./desktopRepository: cnjack/jcode
Length of output: 966
🏁 Script executed:
# Check what Tauri remote capability actually grants
rg -B 5 -A 5 "remote" ./desktop/src-tauri/capabilities/Repository: cnjack/jcode
Length of output: 977
Authenticate the sidecar before navigating the privileged webview.
Between pick_free_port() (line 22) and the sidecar binding, another local process can win the port race and respond to the /api/health probe. The health check at line 144 accepts any HTTP 200 response containing "status", which an attacker's spoofed listener can easily satisfy. Once the webview navigates to the attacker's origin, it gains all Tauri permissions granted by default.json—window management, app control, events, notifications, and dialogs—via the wildcard "remote": {"urls": ["http://127.0.0.1:*"]} capability.
Pass an unguessable nonce from the Rust launcher to the jcode web sidecar and require /api/health to echo it in the response before calling w.navigate(), or retain listener ownership across the launch to eliminate the race window.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@desktop/src-tauri/src/sidecar.rs` around lines 18 - 31, Implement nonce-based
authentication between the Rust launcher and the sidecar to prevent port
spoofing attacks. In the start() function, generate an unguessable nonce before
calling pick_free_port(), then ensure this nonce is passed to the sidecar during
initialization. Modify the health check probe (currently at line 144) to require
the sidecar echo back the nonce in its /api/health response before calling
w.navigate() to the webview. This prevents an attacker's spoofed listener from
satisfying the health check and gaining access to privileged Tauri capabilities
via the wildcard "remote" capability.
Comment on lines
+2963
to
2967
| if origin != "" && isAllowedWebOrigin(r) { | ||
| w.Header().Set("Access-Control-Allow-Origin", origin) | ||
| w.Header().Set("Vary", "Origin") | ||
| w.Header().Set("Access-Control-Allow-Methods", "GET, POST, PUT, DELETE, OPTIONS") | ||
| w.Header().Set("Access-Control-Allow-Headers", "Content-Type, Authorization") |
There was a problem hiding this comment.
Allow PATCH in CORS preflights for task updates.
PATCH /api/tasks/{id} is now part of the API and the frontend sends PATCH, but the CORS allow-method list omits it, so allowed cross-origin clients fail preflight before metadata updates can reach the handler.
Proposed fix
- w.Header().Set("Access-Control-Allow-Methods", "GET, POST, PUT, DELETE, OPTIONS")
+ w.Header().Set("Access-Control-Allow-Methods", "GET, POST, PUT, PATCH, DELETE, OPTIONS")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if origin != "" && isAllowedWebOrigin(r) { | |
| w.Header().Set("Access-Control-Allow-Origin", origin) | |
| w.Header().Set("Vary", "Origin") | |
| w.Header().Set("Access-Control-Allow-Methods", "GET, POST, PUT, DELETE, OPTIONS") | |
| w.Header().Set("Access-Control-Allow-Headers", "Content-Type, Authorization") | |
| if origin != "" && isAllowedWebOrigin(r) { | |
| w.Header().Set("Access-Control-Allow-Origin", origin) | |
| w.Header().Set("Vary", "Origin") | |
| w.Header().Set("Access-Control-Allow-Methods", "GET, POST, PUT, PATCH, DELETE, OPTIONS") | |
| w.Header().Set("Access-Control-Allow-Headers", "Content-Type, Authorization") |
🤖 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 `@internal/web/server.go` around lines 2963 - 2967, The CORS preflight
configuration in the isAllowedWebOrigin check is missing the PATCH HTTP method
in the Access-Control-Allow-Methods header, but the API now supports PATCH for
task updates. Add PATCH to the comma-separated list of allowed methods in the
Access-Control-Allow-Methods header line, placing it alongside the existing GET,
POST, PUT, DELETE, and OPTIONS methods.
Comment on lines
+75
to
+79
| RUST_TARGET := $(shell rustc -vV 2>/dev/null | sed -n 's/^host: //p') | ||
| # Tauri's externalBin resolver requires the OS executable suffix, so Windows | ||
| # sidecars must be jcode-<triple>.exe. | ||
| SIDECAR_EXE := $(if $(findstring windows,$(RUST_TARGET)),.exe,) | ||
|
|
There was a problem hiding this comment.
Fail fast when Rust host triple detection fails.
If RUST_TARGET is empty, Line 89 produces a sidecar name that won’t match Tauri’s expected external-bin naming, causing confusing runtime failures.
💡 Proposed fix
desktop-sidecar: generate build-web
+ `@test` -n "$(RUST_TARGET)" || (echo "rustc host target not detected; install Rust toolchain (rustc) first." && exit 1)
`@echo` "Building jcode sidecar for $(RUST_TARGET)..."
`@mkdir` -p $(SIDECAR_DIR)
go build -ldflags "$(LDFLAGS)" -o $(SIDECAR_DIR)/jcode-$(RUST_TARGET)$(SIDECAR_EXE) $(PKG)Also applies to: 87-89
🤖 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 `@Makefile` around lines 75 - 79, The RUST_TARGET variable may be empty if the
rustc command fails or produces no output, which causes SIDECAR_EXE to have an
incorrect value that won't match Tauri's expected external-bin naming
convention. Add a check immediately after the RUST_TARGET assignment that halts
the build with an error message if RUST_TARGET is empty, ensuring the sidecar
name generated by SIDECAR_EXE is always valid and prevents confusing runtime
failures downstream.
|
|
||
| # Run the desktop app in development (hot window; rebuilds the sidecar first). | ||
| desktop-dev: desktop-sidecar | ||
| cd $(DESKTOP_DIR) && (pnpm install 2>/dev/null || npm install) && pnpm tauri dev |
There was a problem hiding this comment.
Fix broken fallback logic for package manager invocation.
Line 93 and Line 97 fall back to npm install but still execute pnpm tauri ..., so both targets fail exactly when pnpm is missing.
💡 Proposed fix
desktop-dev: desktop-sidecar
- cd $(DESKTOP_DIR) && (pnpm install 2>/dev/null || npm install) && pnpm tauri dev
+ cd $(DESKTOP_DIR) && if command -v pnpm >/dev/null 2>&1; then pnpm install && pnpm tauri dev; else npm install && npx tauri dev; fi
desktop-build: desktop-sidecar
- cd $(DESKTOP_DIR) && (pnpm install 2>/dev/null || npm install) && pnpm tauri build
+ cd $(DESKTOP_DIR) && if command -v pnpm >/dev/null 2>&1; then pnpm install && pnpm tauri build; else npm install && npx tauri build; fiAlso applies to: 97-97
🤖 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 `@Makefile` at line 93, The fallback logic in both the desktop dev and build
targets (lines 93 and 97) attempts to install packages with pnpm first and falls
back to npm if pnpm is unavailable, but then unconditionally executes pnpm tauri
dev or pnpm tauri build. This causes the targets to fail when pnpm is not
installed since the fallback only covers the install step. Update both targets
to ensure the tauri invocation also has a fallback mechanism, either by using
the same package manager that was successfully used for installation or by
applying the conditional fallback pattern to the tauri command as well.
Comment on lines
+117
to
+120
| const colon = p.host.lastIndexOf(':') | ||
| form.host = colon >= 0 ? p.host.slice(0, colon) : p.host | ||
| form.port = p.port || 22 | ||
| form.user = p.user || 'root' |
There was a problem hiding this comment.
Host parsing breaks plain IPv6 addresses.
Using lastIndexOf(':') assumes every trailing colon segment is a port, so values like 2001:db8::10 get split incorrectly in both prefill and alias flows.
💡 Proposed fix
+function splitHostPort(raw: string): { host: string; port?: number } {
+ const v = raw.trim()
+ if (!v) return { host: '' }
+ // [ipv6]:port
+ if (v.startsWith('[')) {
+ const end = v.indexOf(']')
+ if (end > 0) {
+ const host = v.slice(1, end)
+ const rest = v.slice(end + 1)
+ if (rest.startsWith(':')) {
+ const p = Number.parseInt(rest.slice(1), 10)
+ return Number.isFinite(p) ? { host, port: p } : { host }
+ }
+ return { host }
+ }
+ }
+ // host:port (single-colon only); otherwise treat as raw host (e.g. ipv6)
+ const first = v.indexOf(':')
+ const last = v.lastIndexOf(':')
+ if (first > 0 && first === last) {
+ const p = Number.parseInt(v.slice(last + 1), 10)
+ if (Number.isFinite(p)) return { host: v.slice(0, last), port: p }
+ }
+ return { host: v }
+}
+
function applyPrefill(p: Prefill) {
- const colon = p.host.lastIndexOf(':')
- form.host = colon >= 0 ? p.host.slice(0, colon) : p.host
- form.port = p.port || 22
+ const parsed = splitHostPort(p.host)
+ form.host = parsed.host
+ form.port = p.port || parsed.port || 22
form.user = p.user || 'root'
step.value = 'config'
}
@@
- const colon = host.lastIndexOf(':')
- if (colon >= 0) {
- form.port = parseInt(host.slice(colon + 1), 10) || 22
- host = host.slice(0, colon)
- }
+ const parsed = splitHostPort(host)
+ host = parsed.host
+ if (parsed.port) form.port = parsed.portAlso applies to: 178-181
🤖 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 `@web/src/components/RemoteConnectWizard.vue` around lines 117 - 120, The host
parsing logic in both locations (lines 117-120 and 178-181) using
lastIndexOf(':') incorrectly splits IPv6 addresses since colons are part of the
address format. To fix this, improve the host and port parsing to properly
detect IPv6 addresses (either in bracket notation like [2001:db8::10]:8080 or
plain format) and only treat the last colon as a port separator for non-IPv6
addresses. For IPv6 addresses in bracket notation, extract the address from
within the brackets and parse the port after the closing bracket. For plain IPv6
addresses without explicit port notation, assign the address to form.host
without splitting on colons and use the default port value.
Comment on lines
+25
to
+27
| const colon = hostPort.lastIndexOf(':') | ||
| const port = colon >= 0 ? parseInt(hostPort.slice(colon + 1), 10) || 22 : 22 | ||
| return { host: hostPort, user, port, remotePath } |
There was a problem hiding this comment.
Split host and port before returning RemoteMeta.
Line 27 returns host: hostPort, so a label like ssh://alice@example.com:2222/work yields host="example.com:2222" and port=2222. The reconnect flow then sends both host and port, which can break /api/remote/connect payload semantics.
Suggested fix
- const colon = hostPort.lastIndexOf(':')
- const port = colon >= 0 ? parseInt(hostPort.slice(colon + 1), 10) || 22 : 22
- return { host: hostPort, user, port, remotePath }
+ const colon = hostPort.lastIndexOf(':')
+ const host = colon >= 0 ? hostPort.slice(0, colon) : hostPort
+ const port = colon >= 0 ? parseInt(hostPort.slice(colon + 1), 10) || 22 : 22
+ return { host, user, port, remotePath }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const colon = hostPort.lastIndexOf(':') | |
| const port = colon >= 0 ? parseInt(hostPort.slice(colon + 1), 10) || 22 : 22 | |
| return { host: hostPort, user, port, remotePath } | |
| const colon = hostPort.lastIndexOf(':') | |
| const host = colon >= 0 ? hostPort.slice(0, colon) : hostPort | |
| const port = colon >= 0 ? parseInt(hostPort.slice(colon + 1), 10) || 22 : 22 | |
| return { host, user, port, remotePath } |
🤖 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 `@web/src/stores/project.ts` around lines 25 - 27, The return statement in the
RemoteMeta object is setting host to the full hostPort string, which includes
the port number when present (e.g., "example.com:2222"). This breaks the API
payload semantics. Use the colon variable that was already calculated to extract
just the hostname portion from hostPort before returning it - when colon is >=
0, set host to hostPort.slice(0, colon), otherwise set host to hostPort. This
ensures the host field contains only the hostname without the port number in the
RemoteMeta return object.
This was referenced Jun 23, 2026
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
A task-centric redesign of the jcode web UI plus Phase 0 of the multi-project/parallel architecture. Design doc:
docs/web-task-architecture.md.UI redesign
--color-successstatus token.Task architecture — Phase 0
GET /api/workspace→ gitbranch+dirty(fixes the hardcoded-nullbranch in TopBar).GET /api/taskslists sessions across all projects;PATCH /api/tasks/{id}updates pin/archive/unread/title (SessionMetagained these additive fields).submit-prbuiltin skill: agent commits/pushes/opens a PR when triggered by a user query — no manual git UI.Requirements traceability
submit-prskill$/#entry points/already = skills ($redundant);#cross-conversation linking needs the Phase-1 task model.Testing
internal/web/tasks_test.go:/api/workspace(non-git),/api/tasks(empty + multi-project),PATCH /api/tasks/{id}(pin/archive/unread/rename + 404).internal/skills/skills_test.go:submit-prskill loads + is reachable via/submit-pr.go vet,go build ./...,vue-tsc,vite buildall pass.Note
dist/is gitignored (regenerated bymake build). Rebuild + restart the server to pick up the new embedded frontend.🤖 Generated with Claude Code
Summary by CodeRabbit
/submit-prskill guide