fix(chat): harden local chat path links config defaults and self-heal#267
Open
DerrickBarra wants to merge 5 commits intodaggerhashimoto:masterfrom
Open
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughCentralizes chat path-links config into a new module, re-exports it server-side/client-side, adds platform-aware default/template generation, makes the workspace route auto-regenerate a missing local Changes
Sequence DiagramsequenceDiagram
autonumber
actor Client
participant WorkspaceRoute as "Workspace Route"
participant Filesystem as "Filesystem"
participant ConfigModule as "ChatPathLinks Config Module"
Client->>WorkspaceRoute: GET /api/workspace/chatPathLinks
WorkspaceRoute->>Filesystem: read CHAT_PATH_LINKS.json
alt file exists
Filesystem-->>WorkspaceRoute: file content
WorkspaceRoute-->>Client: 200 { ok: true, content }
else file missing (ENOENT)
Filesystem-->>WorkspaceRoute: ENOENT
WorkspaceRoute->>ConfigModule: createChatPathLinksTemplate(platform, homeDir, user)
ConfigModule-->>WorkspaceRoute: generated JSON template
WorkspaceRoute->>Filesystem: write CHAT_PATH_LINKS.json (mkdir -p)
Filesystem-->>WorkspaceRoute: write complete
WorkspaceRoute->>WorkspaceRoute: console.warn("regenerated CHAT_PATH_LINKS.json")
WorkspaceRoute-->>Client: 200 { ok: true, content: template }
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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 (2)
src/features/workspace/tabs/ConfigTab.test.tsx (1)
81-115: Consider moving the assertion outside the mock for clearer test flow.The inline
expect(init.body)on line 95 inside the fetch mock works but can make test failures harder to diagnose. If the assertion fails, it throws inside the mock, which may produce a confusing error message.An alternative is to capture the request body and assert after
user.click:♻️ Suggested refactor
it('creates CHAT_PATH_LINKS.json with the shared template', async () => { const user = userEvent.setup(); const expectedTemplate = createChatPathLinksTemplate(); + let capturedBody: string | undefined; globalThis.fetch = vi.fn((input: string | URL | Request, init?: RequestInit) => { const url = String(input); // ... if (init?.method === 'PUT' && url === '/api/workspace/chatPathLinks') { - expect(init.body).toBe(JSON.stringify({ content: expectedTemplate, agentId: 'alpha' })); + capturedBody = init.body as string; return Promise.resolve(jsonResponse({ ok: true })); } // ... }) as typeof globalThis.fetch; // ... render and interact ... + expect(capturedBody).toBe(JSON.stringify({ content: expectedTemplate, agentId: 'alpha' })); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/workspace/tabs/ConfigTab.test.tsx` around lines 81 - 115, The test currently asserts the PUT request body inside the globalThis.fetch mock (inside the arrow handling the '/api/workspace/chatPathLinks' PUT), which can produce confusing failures; instead capture the request payload from that branch into a local variable (e.g., let capturedBody: string | undefined) when init?.method === 'PUT' and url === '/api/workspace/chatPathLinks', return the mocked response as before, then after the user.click that triggers creation and after awaiting the "File created" text, assert that capturedBody === JSON.stringify({ content: expectedTemplate, agentId: 'alpha' }); update the mock references to expectedTemplate and agentId to keep behavior identical but move the expect out of the fetch handler.src/features/workspace/tabs/ConfigTab.tsx (1)
153-162: Consider passing seed context for richer defaults.The UI calls
createChatPathLinksTemplate()without any seed context, yielding only["/workspace/"]. The server-side regeneration (inworkspace.ts) passesplatform,homeDir,username, andworkspaceRootto produce OS-aware prefixes.This asymmetry means manually creating the file via the UI produces a minimal template, while the auto-regenerated file is richer. This may be intentional (user can customize), but worth confirming it aligns with the expected UX.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/workspace/tabs/ConfigTab.tsx` around lines 153 - 162, The UI creates a minimal chatPathLinks template because handleCreate calls createChatPathLinksTemplate() with no seed; update handleCreate (and the call site using selectedKey === 'chatPathLinks') to pass the same seed context the server uses (platform, homeDir, username, workspaceRoot) into createChatPathLinksTemplate so the client-generated file matches server regeneration; ensure you source those values from available props/context (or derive them the same way as workspace.ts) and fall back to sensible defaults if any are missing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/features/chat/chatPathLinksConfig.ts`:
- Around line 1-10: The current file imports server-only symbols
(DEFAULT_CHAT_PATH_LINKS_CONFIG, createChatPathLinksTemplate,
createDefaultChatPathLinksConfig, normalizeChatPathLinkPrefixes,
parseChatPathLinksConfig, stringifyChatPathLinksConfig, ChatPathLinksConfig,
ChatPathLinksSeedContext) from a server-only module, which breaks client
production builds; fix by extracting the shared logic into a
client/server-available module (e.g., move the implementations and types into a
shared module under the app source tree) and then update this file to import
those same symbols from that new shared module so both client and server builds
resolve them correctly.
---
Nitpick comments:
In `@src/features/workspace/tabs/ConfigTab.test.tsx`:
- Around line 81-115: The test currently asserts the PUT request body inside the
globalThis.fetch mock (inside the arrow handling the
'/api/workspace/chatPathLinks' PUT), which can produce confusing failures;
instead capture the request payload from that branch into a local variable
(e.g., let capturedBody: string | undefined) when init?.method === 'PUT' and url
=== '/api/workspace/chatPathLinks', return the mocked response as before, then
after the user.click that triggers creation and after awaiting the "File
created" text, assert that capturedBody === JSON.stringify({ content:
expectedTemplate, agentId: 'alpha' }); update the mock references to
expectedTemplate and agentId to keep behavior identical but move the expect out
of the fetch handler.
In `@src/features/workspace/tabs/ConfigTab.tsx`:
- Around line 153-162: The UI creates a minimal chatPathLinks template because
handleCreate calls createChatPathLinksTemplate() with no seed; update
handleCreate (and the call site using selectedKey === 'chatPathLinks') to pass
the same seed context the server uses (platform, homeDir, username,
workspaceRoot) into createChatPathLinksTemplate so the client-generated file
matches server regeneration; ensure you source those values from available
props/context (or derive them the same way as workspace.ts) and fall back to
sensible defaults if any are missing.
🪄 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: a5af9c91-5005-4099-96d5-fd6f35abecb8
📒 Files selected for processing (8)
server/lib/chat-path-links-config.tsserver/routes/workspace.test.tsserver/routes/workspace.tssrc/features/chat/chatPathLinks.tssrc/features/chat/chatPathLinksConfig.test.tssrc/features/chat/chatPathLinksConfig.tssrc/features/workspace/tabs/ConfigTab.test.tsxsrc/features/workspace/tabs/ConfigTab.tsx
Contributor
Author
|
Addressed the CodeRabbit follow-ups on this branch:
Validation run:
Ready for re-review. |
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
CHAT_PATH_LINKS.jsonduring local workspace readsConfigTabmanual template creation with that shared helperIn Plain English
This PR makes local chat path links much harder to break on the machine running Nerve.
If the local
CHAT_PATH_LINKS.jsonfile disappears, Nerve now regenerates it automatically instead of leaving local chat links in a broken/manual-recovery state. The PR also removes drift between runtime behavior and the UI's manual create-template flow by having both come from the same shared helper.On top of that, the generated defaults are more OS-aware, so Linux, macOS, and Windows users get more sensible local path prefixes out of the box. This is intentionally a local-host hardening pass — not a remote/gateway parity change, and not a settings UX expansion.
Why
Closes #266.
This is the local-config hardening follow-up to the existing chat path links feature from #237 / #239.
Before this PR:
CHAT_PATH_LINKS.jsoncould leave the local experience broken until the file was recreated manuallyHow
GET /api/workspace/chatPathLinkshandling to regenerate and persistCHAT_PATH_LINKS.jsonwhen it is missing, then return the regenerated content immediatelyCHAT_PATH_LINKS.jsoncreate-template string withcreateChatPathLinksTemplate()Provenance
Clean branch commit stack:
7bc75f2—feat(chat): self-heal missing local chat path links configa69f7bc—fix(chat): keep chat path links helper in server tree7ed4496—feat(chat): add windows-aware path link defaultsValidated downstream via local
workhorsedogfood before packaging upstream, including the Windows-aware defaults follow-up while preserving Linux/Zorin behavior.Scope / Non-goals
In scope:
openclaw-nerveNot in scope:
openclawlane)Type of Change
Checklist
npm run lintpassesnpm run build && npm run build:serversucceedsnpm test -- --runpassesValidation run:
npm test -- --run server/routes/workspace.test.ts src/features/chat/chatPathLinksConfig.test.ts src/features/workspace/tabs/ConfigTab.test.tsxnpm run buildworkhorseSummary by CodeRabbit
New Features
Improvements
Tests