refactor: simplify model-facing coding surface - #172
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
📝 WalkthroughWalkthroughDevSpace now presents a concise workspace-scoped product model. MCP tool schemas and workspace results expose less internal metadata. Codex process controls use reduced inputs. Documentation and tests describe the revised contracts. ChangesWorkspace-scoped interface refinement
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)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
Greptile SummaryThe PR simplifies DevSpace’s model-facing language and schemas while moving process, provider, and diagnostic choices behind the server.
Confidence Score: 4/5The process-output recovery defect should be fixed before merging because completed high-output commands direct callers to a session that no longer exists. Truncated process output is irreversibly discarded, yet the changed response instructs callers to retrieve more through write_stdin; completed sessions have already been deleted, so that follow-up fails. Files Needing Attention: src/server.ts
|
| Filename | Overview |
|---|---|
| src/server.ts | Simplifies model-facing schemas and responses, but the new truncation guidance promises recovery that the process-session lifecycle cannot provide. |
| src/server.test.ts | Adds coverage for the reduced workspace and process schemas but omits completed-command truncation behavior. |
| skills/subagent-delegation/SKILL.md | Updates delegation guidance to select configured profiles without exposing provider-specific controls. |
| AGENTS.md | Reframes repository guidance around concise model-facing outcomes, product vocabulary, and hidden implementation mechanics. |
| docs/chatgpt-coding-workflow.md | Relabels the workflow as an integration reference and documents the reduced agent and process surfaces. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[exec_command starts process] --> B[Wait using internal yield limit]
B --> C[Drain bounded output snapshot]
C --> D{Output truncated?}
D -- No --> E[Return command status and output]
D -- Yes --> F[Discard omitted middle]
F --> G{Process still running?}
G -- Yes --> H[write_stdin can read only newly produced output]
G -- No --> I[Delete process session]
I --> J[Suggested write_stdin call fails as unknown session]
Reviews (1): Last reviewed commit: "refactor: simplify model-facing coding s..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const outputNote = snapshot.outputTruncated | ||
| ? " Output is partial; continue with write_stdin for more." |
There was a problem hiding this comment.
When a command exceeds the fixed output budget, this response tells the caller to retrieve more through write_stdin, but draining the snapshot has already discarded the omitted output; when the command has completed, its session has also been deleted, so the suggested follow-up fails with Unknown process session.
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/server.ts (1)
490-515: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winRemove internal diagnostics from card metadata too.
The reduced
structuredContentcontracts are incomplete._meta.card.summarystill receiveswallTimeMsfrom Lines 570-576 and Lines 612-618._meta.cardstill includesagentProviders,providerAvailable, andproviderUnavailableReasonfrom Lines 744-752 and Lines 819-828.Remove these fields from card metadata. Add integration assertions that card metadata excludes timing and provider diagnostics.
src/server.ts#L490-L515: removewallTimeMsfrom the process card summaries.src/server.ts#L766-L843: remove provider availability and provider diagnostic fields from the workspace card payload.🤖 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/server.ts` around lines 490 - 515, Update src/server.ts lines 490-515 in processToolResponse and the process-card summary paths at lines 570-576 and 612-618 to exclude wallTimeMs from _meta.card.summary; update the workspace card payload at lines 766-843 to remove agentProviders, providerAvailable, and providerUnavailableReason. Add integration assertions covering both card metadata paths to verify timing and provider diagnostics are absent.Sources: Coding guidelines, Learnings
🤖 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 `@docs/setup.md`:
- Around line 46-50: Update the label in the Server Port section before the
loopback URL from “The MCP URL” to “The server listens at,” leaving the URL and
subsequent public client endpoint guidance unchanged.
In `@README.md`:
- Around line 146-148: Update the DevSpace description in the README to use
complete product-oriented wording: state that DevSpace provides remote access to
selected project folders, while preserving the surrounding explanation.
---
Outside diff comments:
In `@src/server.ts`:
- Around line 490-515: Update src/server.ts lines 490-515 in processToolResponse
and the process-card summary paths at lines 570-576 and 612-618 to exclude
wallTimeMs from _meta.card.summary; update the workspace card payload at lines
766-843 to remove agentProviders, providerAvailable, and
providerUnavailableReason. Add integration assertions covering both card
metadata paths to verify timing and provider diagnostics are absent.
🪄 Autofix
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: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e743c263-820f-41b0-bb3d-9363560a0a32
📒 Files selected for processing (16)
AGENTS.mdREADME.mddocs/artifact-exchange.mddocs/chatgpt-coding-workflow.mddocs/configuration.mddocs/gotchas.mddocs/security.mddocs/setup.mdexamples/agents/codex-qa-tester.mdpackage.jsonskills/subagent-delegation/SKILL.mdsrc/artifact-tools.tssrc/cli.tssrc/oauth-provider.tssrc/server.test.tssrc/server.ts
| ### Server Port | ||
|
|
||
| The default is `7676`. | ||
|
|
||
| The local MCP URL is: | ||
| The MCP URL is: |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Distinguish the server listener from the client endpoint.
The text labels http://127.0.0.1:7676/mcp as “The MCP URL.” This is the server's loopback listener. The public client endpoint appears later in the guide. Rename this label to “The server listens at” so users do not configure a remote MCP client with the loopback URL.
🤖 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 `@docs/setup.md` around lines 46 - 50, Update the label in the Server Port
section before the loopback URL from “The MCP URL” to “The server listens at,”
leaving the URL and subsequent public client endpoint guidance unchanged.
| DevSpace is remote access to selected project folders. | ||
|
|
||
| You decide which roots are allowed. The MCP client still has powerful local | ||
| You decide which roots are allowed. The MCP client still has powerful coding |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use a complete product description.
DevSpace is remote access to selected project folders describes a capability as if it were the product. Use DevSpace provides remote access to selected project folders. This wording is clearer for users.
🧰 Tools
🪛 LanguageTool
[grammar] ~148-~148: Ensure spelling is correct
Context: ...cted project folders. You decide which roots are allowed. The MCP client still has p...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 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 `@README.md` around lines 146 - 148, Update the DevSpace description in the
README to use complete product-oriented wording: state that DevSpace provides
remote access to selected project folders, while preserving the surrounding
explanation.
d196a71 to
5281bfb
Compare
5281bfb to
dcfffd4
Compare
1 participant
DevSpace's runtime model-facing contract now favors actionable choices and concise outcomes while keeping tooling mechanics behind the server. Tool descriptions, server instructions, workspace/process/artifact responses, and lifecycle text are shorter; provider and runtime diagnostics plus process tuning remain managed by DevSpace.
Verified with
npm run typecheck,npm test, andnpm run build.