Skip to content

feat: move LSP behind app server - #195

Merged
oratis merged 23 commits into
mainfrom
codex/lsp-protocol-client
Aug 2, 2026
Merged

feat: move LSP behind app server#195
oratis merged 23 commits into
mainfrom
codex/lsp-protocol-client

Conversation

@oratis

@oratis oratis commented Aug 1, 2026

Copy link
Copy Markdown
Owner

Summary

  • add a reusable Node stdio app-server connection with backpressure, bounded stderr diagnostics, graceful EOF shutdown, and timed termination
  • remove provider, credential, and RuntimeHost construction from the LSP process
  • expose native thread read/resume, turn interrupt, approval, and user-input protocol commands
  • emit the same structured protocol events used by desktop, including fast-turn buffering
  • replace credential-dependent tests with deterministic protocol tests and a real child-process transport test

Validation

  • pnpm install --frozen-lockfile
  • pnpm format:check
  • pnpm lint (0 errors; one pre-existing warning)
  • pnpm typecheck
  • pnpm test (957 passed, 12 skipped)
  • pnpm build
  • pnpm docs:check
  • real app-server child smoke: initialize + thread/start + thread/read + graceful close

Stack

Depends on #194.

@oratis
oratis force-pushed the codex/protocol-client branch from e59f194 to 27d1a82 Compare August 1, 2026 07:32
@oratis
oratis force-pushed the codex/lsp-protocol-client branch from 9df2aac to b81da04 Compare August 1, 2026 07:32
@oratis
oratis changed the base branch from codex/protocol-client to main August 2, 2026 06:36
@oratis
oratis marked this pull request as ready for review August 2, 2026 06:42
@oratis

oratis commented Aug 2, 2026

Copy link
Copy Markdown
Owner Author

Review: approved ✅

The LSP process stops being a runtime. Combined with #181 (which made its abort actually abort) and #184 (which gave it a real policy), the LSP client is now a genuinely thin protocol consumer.

What I checked

  • Provider, credential, and RuntimeHost construction removed from the LSP process. This is the substantive change — an editor language server had no business holding provider credentials.
  • The Node stdio connection handles backpressure, bounded stderr diagnostics, graceful EOF shutdown, and timed termination. Bounded stderr matters: an unbounded diagnostic buffer from a chatty child is a slow memory leak in a long-lived editor session. Timed termination matters because a sidecar that ignores EOF must not be able to outlive its parent forever.
  • Credential-dependent tests replaced with deterministic protocol tests plus a real child-process transport test. Good trade — the old tests needed real credentials to be meaningful, which is why they were weak. Keeping one real subprocess test means the transport is still exercised for real rather than entirely mocked away.

Validation: CI green; LSP 13 tests pass locally at the stack tip.

@oratis
oratis merged commit 25cc666 into main Aug 2, 2026
5 checks passed
oratis added a commit that referenced this pull request Aug 2, 2026
…#214)

Reworks #166, which was correct in approach but is now incomplete and
carries stale docs.

`permissions.additionalDirectories` has been declared in the settings
schema and read by /permissions for display, but consumed for
enforcement nowhere — /add-dir only printed "recorded ... (effective in
M3)". Users could reasonably believe it did something.

Why the sandbox is the right boundary: the file tools (Read/Write/Edit/
Glob/Grep) already accept any absolute path, so there is no cwd
containment to widen. The only thing that actually restricts writes is
the OS sandbox wrapping Bash. Enforcing /add-dir therefore means adding
those directories to `filesystem.allowWrite`.

The part #166 could not have covered: it predates the app-server, and
wired only CLI REPL + headless. Since #192/#195/#196, desktop, VS Code
and LSP all run through apps/server, so that wiring would have enforced
the setting in the CLI and silently ignored it everywhere else — for a
security-relevant setting, partial enforcement is worse than none,
because it is indistinguishable from full enforcement at the UI. All 7
sandbox assembly sites across 4 files now route through the helper.

- core `withAdditionalWritableDirs(sandbox, dirs, cwd?)`: pure, never
  mutates input, no-op when the sandbox is disabled (never silently
  enables it), dedupes, and drops non-absolute entries rather than
  handing them to profile writers that require absolute paths
- /add-dir validates the path is an existing directory, stores it
  absolute (resolved against cwd), refuses duplicates, and lists the
  current set with no args
- BEHAVIOR_PARITY: only the /add-dir row changes. #166 rewrote the whole
  table with June-era content that would have regressed /btw, /voice,
  /tasks and /background back to unshipped. (Prettier reflows the table
  columns; `git diff -w` shows the 2 real lines.)

tsc -b --force, lint (--max-warnings=0), format:check, docs:check clean;
1048 tests pass (9 new helper + 6 new /add-dir); build clean.

Co-authored-by: t <t@t>
Co-authored-by: Claude Opus 5 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant