Fix ide browse file targets#67
Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
📝 WalkthroughWalkthroughWorkspace sessions now carry an optional initial file, expose it through a scoped HTTP endpoint, and update it during browse requests. Hosted Tauri transport retrieves the value through the shared API mechanism, with backend, frontend, and smoke-test coverage. ChangesWorkspace initial-file flow
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant HostedTauriTransport
participant HTTPServer
participant WorkspaceSessionState
HostedTauriTransport->>HTTPServer: GET /api/initial-file
HTTPServer->>WorkspaceSessionState: Read scoped initial_file
WorkspaceSessionState-->>HTTPServer: Option<String>
HTTPServer-->>HostedTauriTransport: Return JSON initial file
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
This PR updates the “browse” flow so that explicit file targets can be surfaced to the hosted (HTTP) IDE session, and ensures already-open workspace sessions can be updated with a new requested file instead of only opening new sessions.
Changes:
- Frontend: route
getInitialFile()through the shared hosted/nativecallApi()transport. - Backend: add a scoped
/api/initial-fileendpoint and plumb per-sessioninitial_filethrough workspace resolution. - Tests: add regression coverage for the hosted transport initial-file call and backend session update/initial-file API behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/tauri.ts | Switches getInitialFile() to use the shared hosted/native API transport. |
| src/tauri.test.ts | Adds a hosted-transport test asserting the initial-file API call is made. |
| src-tauri/src/http_server.rs | Adds /api/initial-file, session refresh logic for initial-file, and regression tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/tauri.test.ts (1)
243-252: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExercise the scoped URL in this regression test.
The test uses an unscoped location and only checks
/api/initial-file, so it would still pass ifworkspacePathPrefix(window.location)stopped adding the workspace hash. Set the test location to/<hash>/and assert the full/<hash>/api/initial-fileGET request.🤖 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 `@src/tauri.test.ts` around lines 243 - 252, Update the “reads the initial file from the scoped hosted session” test to set the browser location to a hashed path such as /<hash>/ before importing or invoking getInitialFile, then assert fetch was called with the complete /<hash>/api/initial-file URL while preserving the existing GET request assertions.
🤖 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 `@src-tauri/src/http_server.rs`:
- Line 484: Decouple initial-file state from the non-unique workspace hash in
the session transport flow around resolve_workspace_by_hash, initial_file, and
the /api/initial-file handling. Enforce unique sessions per workspace root,
share initial_file at the workspace/hash level, or add a stable session
identifier to both browse updates and retrieval so each request consistently
targets the owning session.
In `@src/tauri.ts`:
- Around line 393-395: Update getInitialFile to normalize the hosted
get_initial_file response by converting null to undefined before returning it,
preserving its string | undefined contract for callers.
---
Nitpick comments:
In `@src/tauri.test.ts`:
- Around line 243-252: Update the “reads the initial file from the scoped hosted
session” test to set the browser location to a hashed path such as /<hash>/
before importing or invoking getInitialFile, then assert fetch was called with
the complete /<hash>/api/initial-file URL while preserving the existing GET
request assertions.
🪄 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 Plus
Run ID: 83da7987-e5ed-42a0-97da-b24a1e186f82
📒 Files selected for processing (3)
src-tauri/src/http_server.rssrc/tauri.test.tssrc/tauri.ts
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
src/tauri.ts:395
/api/initial-filereturns JSONnullwhen no file is requested (RustOption<String>), butgetInitialFile()is typed asstring | undefinedandreadApiResponse()will currently returnnullat runtime. This breaks the function’s contract and can leaknullinto React state.
export function getInitialFile() {
return callApi<string | null>("get_initial_file", "/api/initial-file").then(
(path) => path ?? undefined,
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
src-tauri/src/http_server.rs:506
update_open_browse_sessiononly updates the first session found for a given workspace hash (viaresolve_workspace_by_hash). Because multipleWorkspaceSessionStates can share the sameworkspace_root_hash(see tests likeworkspace_summaries_dedupe_by_hash), this can make the/api/initial-filevalue non-deterministic: the resolver may pick a different session than the one updated, leaving the hosted browser tab with a stale/cleared initial file.
Update all matching sessions’ initial_file locks (or otherwise make resolver/update deterministic and consistent) so any session selected for the hash reflects the latest requested file.
None => new_path.to_string(),
};
if let Ok(uri) = path_and_query.parse::<Uri>() {
*request.uri_mut() = uri;
}
}
/// Resolves a leading `/{hash}` path segment to an open workspace, rewriting the URI
/// to drop the prefix and stashing the resolved root + agent context as a request
/// extension. Runs before route matching so the existing `/api/...` and `/` routes
/// see the stripped path. Requests without a hash-shaped prefix get the
/// shared/default workspace, leaving the no-hash API surface unchanged.
///
/// A hash-shaped prefix is stripped even when it doesn't match an open workspace, so
Summary
Validation
./run-tests.sh(433 frontend tests, 242 Rust tests, build, smoke tests, and dependency audits)cargo fmt --checkgit diff --check