Native-client runtime API routes: files, artifacts, jobs, git, LSP, secrets, targets

[Hmbown]

Native-client runtime API routes for the GPUI desktop: jobs, workspace files
and session artifacts, git, diagnostics, context, targets/SSH/cloud, LSP, voice,
and a write-only provider credential route. 28 files, ~+5,900 lines, of which
1,758 are tests.

docs/RUNTIME_API.md documents the surface.

Why

The GPUI desktop is the product client (CURRENT_DECISIONS.md §14). It is
Engine-complete for reads but could not drive long-running jobs, review a diff,
export diagnostics, switch targets, or complete first-run provider setup without
dropping the user back into the TUI. Each of those needed a Codewhale-owned
route rather than a second registry in the desktop.

Closes

• Closes App-server: GET /v1/jobs for GPUI long-running job inspect and stop #6166 — GET /v1/jobs plus GET|POST /v1/threads/{id}/jobs,
  /jobs/{job_id}, /output, /stdin, /kill. Jobs created here carry an
  api:{thread_id} owner scope, so a foreign or reused id is rejected rather
  than controlled. Covered by
  jobs_api_lists_creates_streams_stdin_and_kills.
• Closes App-server: GET /v1/artifacts and GET /v1/files for GPUI Preview and Files #6163 — workspace files and session artifacts.
• Closes v0.9.14: App-server credential set/clear with source and writable metadata for GPUI providers #6179 — DELETE /v1/providers/{id}/key, plus credentialSource /
  credentialWritable on GET /v1/providers. See below.

It also turns main green

main is red at 9cdfa92bc: CI's blocking-calls gate is advisory on pull
requests and blocking on pushes to main, and the Lint job failed on
crates/tui/src/runtime_api/workspace.rs: std_fs sites 1 > budget 0.

That site is real. workspace_instructions called std::fs::canonicalize in
its async body, after awaiting the spawn_blocking it already had — a
blocking syscall on a Tokio worker. It now rides that same closure.

This branch had added four more of the same class, which would have made the
red worse on merge: list_logs, list_crashes and process_info did
directory walks, per-file stats and a /proc read inline, and four LSP
handlers called resolve_workspace_file — which canonicalizes and stats every
path component — straight from their async bodies. All now use the blocking
pool.

The gate could not have caught the LSP one. The scanner is per-file and
lexical: the std::fs calls it counts live in workspace.rs, while the
handlers calling through to them live in lsp.rs, so a helper is invisible at
its call site. Worth knowing before anyone reads a passing budget as proof.

The same lexical rule cuts the other way, which is why eight sites are
budgeted rather than rewritten: a sync fn holding std::fs is counted even
when every caller is already inside spawn_blocking. Every caller of
confined_directory, list_workspace_directory, precheck_file_target,
list_files, read_named_window and rss_bytes was read before budgeting.

#6179 — clearing a credential, and saying who owns one

The credential route could set a key but never clear one, and served no way to
tell "you have no key" apart from "your key is owned elsewhere and this control
cannot change it".

DELETE /v1/providers/{id}/key clears through a shared owner rather than a
second implementation: crates/config/src/credentials.rs gains
clear_provider_api_key, and codewhale auth clear now calls it — which
deletes its inline body and a helper. That move fixes a real CLI defect on the
way past: the secret-store delete was let _ = secrets.delete(...), so
auth clear printed success while the key could still be in the keyring. Both
callers now report the backend error.

GET /v1/providers gains credentialSource, credentialWritable and a
reason, so a client disables its control with a truthful explanation instead of
letting a write fail late. Both verbs refuse a credential Codewhale does not
own with 409 — notably a literal key in a config file, which still wins at
request time, so writing the secret store would have reported a success the
user could never observe. That is the "must not appear successfully
overwritten" the issue asks for.

The provider-catalog non-secret projection guard correctly failed on the new
fields. Widening an allow-list is an argument, not a formality: the fields are
admissible because credentialSource is one of four fixed enum spellings that
can never interpolate a value, a path or an environment variable name. The test
now asserts that closed vocabulary, not just the key set.

Partially addresses, and says so

#6179 is not closed by this PR. PUT /v1/providers/{id}/key satisfies the
set half: it persists through the same transactional path as
codewhale auth set, caps the body at 4 KiB, rejects control characters,
returns a redacted receipt (backend + post-write credentialState), and has
deliberately no GET — a route that can return a secret can leak one. Still
missing from that issue's acceptance criteria: clear/revoke, source and
writable metadata, and the named refusal for environment-managed credentials.
Tracked there, not silently folded in here.

Two defects the merge surfaced, and what was done about them

The merge of origin/main (51-commit v0.9.14 slice-2) hit three textual
conflicts, two of which hid problems a textual merge cannot see:

• GET /v1/commands was implemented twice. Both sides added it and axum
  rejects that at runtime ("Overlapping method route"). They are not duplicates:
  main's carries user-registered commands and shadowing; this branch's carried
  localized descriptions, argument-shape semantics and composer hints. That is a
  public API contract decision, not a merge mechanic, so main's shipped handler
  stays and this branch's route, module and test were removed. The lost fields
  are written up in GET /v1/commands: two contracts collided — fold the composer/argument-shape fields into the shipped one and settle the localization policy #6230 with the full comparison and are recoverable from
  060711b2f1.
• OP_KINDS never listed get_context_budget. The GetContextBudget
  variant and its kind_str() arm were added without the const array, which
  protocol_covers_engine_ops checks. Pre-existing on this branch rather than a
  merge artifact — it would have failed CI on its own. Added in variant order.

Evidence

Re-run on this exact head (0285df1ee), not inherited from the handoff:

Gate	Result
cargo fmt --all -- --check	clean
cargo clippy --workspace --all-targets --all-features --locked (CI's allow list)	clean
codewhale-config --lib	test result: ok. 704 passed; 0 failed; 1 ignored
codewhale-cli --lib	test result: ok. 390 passed; 0 failed; 0 ignored
runtime_api	see below
Changelog receipt	sync-changelog.sh + derive-changelog.mjs regenerate with no diff

The handoff claimed runtime_api "250 passed". The first run here reported
test result: FAILED. 251 passed; 2 failed; 0 ignored; 0 measured; 12494 filtered out.
The two failures were
compatibility_stream_closes_losslessly_across_replay_live_handoff and
compatibility_stream_exposes_and_resolves_user_input_without_answer_echo,
both "deadline has elapsed" on a subscription handshake.

They are not a regression, and the reason is worth recording: both tests
arrived on origin/main in #5717, older than this branch, and both use
ci_scaled(Duration::from_secs(2)) — 2s locally, 8s only when CI is set. That
run happened on a 14-core host at load average 17 with three concurrent cargo
lanes. Re-run isolated on the same head:

running 2 tests
test runtime_api::tests::compatibility_stream_closes_losslessly_across_replay_live_handoff ... ok
test runtime_api::tests::compatibility_stream_exposes_and_resolves_user_input_without_answer_echo ... ok

test result: ok. 2 passed; 0 failed; 0 ignored; 0 measured; 12745 filtered out

And the whole module at --test-threads=2 on the current head:
test result: ok. 258 passed; 0 failed; 0 ignored; 0 measured; 12494 filtered out.

So: green isolated and green at low concurrency, flaky under parallel load. That is a different claim from
"green", and the local gate being load-fragile is worth someone's attention
independently of this PR.

Lint gates: check-provider-registry.py,
test_check_command_crate_boundaries.py,
test_check_command_migration_manifest.py, check-dead-code-budget.py,
check-readme-translations.py, check-tui-locale-parity.py,
check-persistence-backlog-budget.py, check-readme-locales.sh,
check-tui-product-vocabulary.sh, web/scripts/check-locales.mjs — all pass.

check-runtime-contract-budget.py fails, and not because of this branch:

[runtime-contract-budget] ERROR: identity changed for Act full tool names
[`tool_catalog.modes.act.full.tool_names`] (added=['session_get', 'session_search'] removed=[])

Neither symbol appears anywhere in this branch's diff. Both landed on main in
eefdcb42f5 (#5715) without a matching update to
scripts/runtime-contract-budget.json, which still lists 50 Act-full tool names
and neither of these. This is exactly the case ci.yml:459-462 anticipates —
"a branch can fail it for debt it inherited rather than added" — so the gate is
advisory on pull requests and blocking on pushes to main. Flagging rather than
folding a manifest edit into an unrelated PR.

Windows: git_routes_drive_a_real_workspace_repo was the only test failing the
required Test (windows-latest) check (15280 run, 1 failed). Windows git
defaults to core.autocrlf=true, so the checkout that POST /v1/git/discard
performs rewrote v1\n to v1\r\n. That is git behaving as configured, so the
fixture pins core.autocrlf=false alongside the user.email/user.name it
already set. The assertion is unchanged and not weakened. This host cannot build
x86_64-pc-windows-msvc, so that half rests on CI.

CI owns the exhaustive run; cargo nextest --workspace was not run locally.

🤖 Generated with Claude Code

[Hmbown]

Merged current main in (6e3439f7e8) and this is now out of draft. The branch was 53 behind; it is now even with the 51-commit slice-2 merge.

Conflicts, and the two things a textual merge could not see

Three textual conflicts, all additive collisions — kept both sides in each. But two hid real problems:

The workspace.rs conflict cut through a function. This branch's #6163 files block and main's #6168 instructions block have no symbol collisions (34 vs 5 items, checked), so both belong. But the conflict boundary ran through write_workspace_file, whose closing brace sat on main's side. Taking both sides verbatim produced an unclosed delimiter. Restored.

GET /v1/commands was implemented twice, and axum rejects that at runtime:

Overlapping method route. Handler for `GET /v1/commands` already exists

They are not duplicates. Main's carries user-registered commands and shadowing; this branch's carries localized description + resolved locale, requires_required_argument (vs a flat takes_arguments), composer_wants_trailing_space, palette_runs_directly, show_in_empty_discovery and unlisted — a richer composer/palette contract.

Picking between them is a public API decision, not a merge mechanic, so I kept main's shipped handler (#6178 is closed against it) and removed this branch's route, module and test. The lost capability is written up field-by-field in #6230, and everything is recoverable from 060711b2f1. That decision should be made deliberately rather than by whoever resolves a conflict.

One pre-existing defect on this branch

OP_KINDS never listed get_context_budget. The branch added the GetContextBudget variant and its kind_str() arm but not the const array — and op.rs has a test asserting "OP_KINDS must list every variant in order". This would have failed CI on its own, independent of the merge. Fixed in variant order. Same for one assert_eq!(..., false) clippy error in the voice-route test.

Verification

• ./scripts/dev-test.sh tui — 12713 run, 12713 passed, 19 skipped (it was 12712/1 before the OP_KINDS fix)
• ./scripts/dev-test.sh protocol — 68 passed
• CI clippy gate --workspace --all-targets --all-features -D warnings — exits 0
• cargo fmt --all -- --check — clean

I did not write the original two commits; I brought them up to date and verified them. CI here is still their first hosted check.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[claude]

Claude finished @Hmbown's task in 3m 15s —— View job

---

Claude is reviewing this PR

• Gather context (PR body, changed files, review comments)
• Read new runtime_api modules (jobs, git, secrets, workspace confinement, runtime_threads)
• Check CodeQL findings already posted (allocation size, cleartext transmission)
• Check provider/model route safety invariant — no issues found
• Deep-dive remaining files (context, lsp, targets, voice, sessions artifacts, shell.rs, router wiring, credentials.rs, cli/lib.rs diff) — running in background
• Post final review

[claude]

Claude finished @Hmbown's task in 4m 20s —— View job

---

Claude finished reviewing this PR

• Gather context (PR body, changed files, prior review comments, CodeQL findings)
• Read new runtime_api modules (jobs, git, secrets, context, diagnostics, lsp, targets, voice, workspace, sessions)
• Review provider/model route safety invariant
• Review runtime_threads.rs shell-manager authority change
• Review credentials.rs, cli/lib.rs diff, engine.rs/ops.rs/protocol_parity.rs
• Post final review

Summary

Overall this is careful, well-scoped work — confinement (relative_request_path, confined_directory, WorkspaceFile's openat/O_NOFOLLOW chain), the git write path (non-interactive, argv-only, check-ref-format before switch), and the credential write (crates/config/src/credentials.rs, correctly extracted and reused by both the CLI and the new /v1/providers/{id}/key route rather than duplicated — good rung-2 discipline) are all solid. No provider/model-prefix inference issues found anywhere in the diff. The CodeQL findings on this PR are false positives (see below).

Correctness — real issue

crates/tui/src/tools/shell.rs:3100-3153 (read_output_chunk) — a long-poll holds the whole thread's shell-manager mutex.

read_output_chunk(&mut self, …) loops on shell.poll() + std::thread::sleep(50ms) for up to wait_ms (client-controlled, capped at 30s via OUTPUT_WAIT_MAX_MS in crates/tui/src/runtime_api/jobs.rs:34). The borrow of shell comes from self.processes.get_mut(task_id), so the loop keeps &mut self borrowed for its whole duration.

The caller, get_thread_job_output (jobs.rs:363-375), obtains self via manager.lock() on the thread's shared SharedShellManager = Arc<std::sync::Mutex<ShellManager>> — the same manager runtime_threads.rs now hands to the engine (shell_manager: Some(shell_manager), runtime_threads.rs:10143) so the model's own shell tool calls run on it too ("One authority" per jobs.rs's module doc).

Net effect: one HTTP client polling GET /v1/threads/{id}/jobs/{job_id}/output?wait_ms=30000 on a running job locks out, for up to 30 real seconds:

• every other job route on that thread (list_thread_jobs, create_thread_job, write_thread_job_stdin, kill_thread_job),
• the flat GET /v1/jobs listing (it locks each thread's manager in turn),
• and the model's own shell-tool execution on that thread, since it shares the same manager.

This directly undercuts the module doc's own claim that reads are "non-consuming" and won't interfere with other consumers — they don't corrupt the stream, but they do serialize all other shell activity on the thread behind the poll. A client that (reasonably) uses a large wait_ms for efficient long-polling turns into a 30-second stall for the agent's own tool calls.

Fix shape: don't hold the manager lock across the sleep loop — re-acquire manager.lock() each iteration (poll, check, drop, sleep, repeat) instead of holding one guard for the whole wait, mirroring what would be needed for get_output_delta_for_session if it's ever driven concurrently. Fix this →

No test currently exercises concurrent access during a long poll, so this wasn't caught by the (otherwise thorough) new test suite.

CodeQL findings — false positives

• diagnostics.rs:152 (Vec::with_capacity(limit.min(64 * 1024))) — limit is validated to 1..=READ_LIMIT_MAX (4 MiB) beforehand, and the capacity itself is separately clamped to 64 KiB via .min(64*1024); the actual read is bounded by file.take(limit as u64). Not exploitable.
• **tests.rs "cleartext transmission of session_id"(5 instances)** — these are test helpers building loopback HTTP URLs to the test server with a synthetic session id as a path segment. A session id is not a secret, and this is test-only code talking to127.0.0.1`. Not a real finding.

Minor (not blocking)

• write_workspace_file (workspace.rs, precheck_file_target → open_confined_file → replace) has a small TOCTOU window between the "does it already exist" check and the write: a file created concurrently between the two could be silently overwritten even without a matching expected_revision, and the response could report created: true when it actually replaced concurrent content. Low impact given this is a local, token-authenticated dev-tool surface, but worth a note if this route is ever exposed more broadly.

Nice touches worth calling out

• CODEWHALE_DISABLE_VOICE kill-switch for headless hosts (commands/groups/core/voice.rs).
• /v1/providers/{id}/key deliberately has no GET, and the key never appears in error messages — good secret hygiene.
• runtime_threads.rs's new shell_managers map correctly ties manager lifetime to thread lifetime (removed on discard_empty_thread and on thread-creation failure), not to engine LRU eviction, so background jobs survive engine unload as intended.

[claude]

Claude finished @Hmbown's task in 3m 40s —— View job

---

Claude finished reviewing this PR

• Gather context (PR body, prior reviews, CodeQL findings)
• Diff the two commits added since the last review (5dd4966, 848af76) against 0285df1
• Review DELETE /v1/providers/{id}/key and the credentialSource/credentialWritable classification (v0.9.14: App-server credential set/clear with source and writable metadata for GPUI providers #6179)
• Review the blocking-call fix (Blocking calls in async paths: audit thread::sleep and std::fs in tool paths, pick one convention #6149) in diagnostics.rs, lsp.rs, workspace.rs
• Re-check provider/model route safety invariant — no issues
• Post final review

This is a synchronize re-trigger, so this review focuses on what changed since the prior review (head 0285df1): two new commits, 5dd4966 (clear-credential route, #6179) and 848af76 (blocking-call fix, #6149).

5dd4966 — DELETE /v1/providers/{id}/key

Good shared-owner discipline: clear_provider_api_key lives once in crates/config/src/credentials.rs and is called by both codewhale auth clear and the new route, matching the extraction pattern already used for set_provider_api_key. It also fixes a real, independent CLI bug in the same commit: auth clear used to do let _ = secrets.delete(...), silently swallowing a keyring failure while printing success. That's now surfaced via ClearOutcome::secret_store_error and reported by both callers — a genuine correctness fix, not just refactor churn.

The new writable_provider(...) helper (secrets.rs) is shared between PUT and DELETE, and credential_writeability(...) correctly stays structural — it classifies by declared auth mode / consent state / the kind of a configured api_key value, never resolving or echoing a secret. The unit tests pin the interesting edge (the __KEYRING__ sentinel must classify as SecretStore, not Config) and that a refusal reason never contains the value it refused.

One gap: the 409 refusal path (config-owned literal key, or external_auth-owned consent) is only unit-tested at the credential_writeability function level — there's no integration test hitting DELETE /v1/providers/{id}/key (or PUT) over HTTP for either of those two source classes, only for the happy path (provider_key_clear_round_trips_and_reports_writability covers unauth + bad-id + a full writable round trip). Since writable_provider is new shared wiring in this commit, a single HTTP-level case for one of the two refusal sources would close the gap between "the classifier is correct" and "the route actually returns 409 for it." Not blocking — low risk given the unit coverage — but worth adding if this route sees more churn.

848af76 — blocking-call fix (#6149)

This is exactly the workspace rule in AGENTS.md (async code must not run blocking syscalls inline; move to spawn_blocking), and the fix is correctly scoped: list_logs/list_crashes/process_info in diagnostics.rs now do their directory walks and /proc reads inside spawn_blocking, and resolve_workspace_file in lsp.rs (which canonicalizes + stats every path component) is now async and rides the blocking pool too. The commit message calls out why the budget scanner couldn't have caught the lsp.rs case on its own (lexical, per-file, doesn't see through a helper call) — that's a useful note for the next person and matches the "misconfiguration/lint-gap fails loud" spirit.

The resolve_workspace_file signature change (fn → async fn) is propagated correctly to all four call sites (lsp_diagnostics, lsp_definition, lsp_references, lsp_symbols) with .await added.

Also updates check-blocking-calls-budget.json for the 8 sites the PR determined are safe as-is (sync helpers whose callers are already inside spawn_blocking) — the commit message documents which ones and why, consistent with the "misconfiguration fails loud" contract rather than quietly bumping a budget number.

Still open from the last review round (unchanged by these two commits)

The previously-flagged long-poll lock-holding bug in read_output_chunk (crates/tui/src/tools/shell.rs:3100-3153) is still present — &mut self (and therefore the thread's shared SharedShellManager mutex, also used by the engine's own shell tool calls) is held across the up-to-30s sleep loop. Neither of the two new commits touches shell.rs, so this remains an open item rather than a regression introduced here.

Summary

Both new commits are correct, well-scoped, and well-documented. 5dd4966 closes the "set but never clear" half of #6179 through genuine reuse rather than a parallel implementation, and incidentally fixes a real silent-failure bug in the existing CLI path. 848af76 is a clean, correctly-targeted fix for the blocking-calls convention with no behavior change and matching budget-file updates. No provider/model-prefix inference issues in either commit. The one gap worth a follow-up is HTTP-level test coverage for the two 409 refusal paths on the new DELETE route.

[Hmbown]

CodeQL triage — 10 high-severity alerts, all assessed

The CodeQL check reports 10 new alerts including 10 high severity security vulnerabilities. I read each one rather than waving at the aggregate. None block, and here is the reasoning for each so a reviewer does not have to redo it.

6 × rust/cleartext-transmission — crates/tui/src/runtime_api/tests.rs:15335-15407

All six are inside session_artifacts_list_and_bounded_read, a test that drives a loopback listener: http://127.0.0.1:<ephemeral> with bearer_auth and fixture tokens that are not credentials. The rule fires on http:// + a bearer header; there is no transmission off the machine and no real secret. Test-only.

1 × rust/uncontrolled-allocation-size — crates/tui/src/runtime_api/diagnostics.rs:152

The rule sees request-controlled limit reach Vec::with_capacity and misses the range check two lines above:

let limit = query.limit.unwrap_or(READ_LIMIT_DEFAULT);
if !(1..=READ_LIMIT_MAX).contains(&limit) {        // READ_LIMIT_MAX = 4 MiB
    return Err(ApiError::bad_request(...));
}
...
let mut window = Vec::with_capacity(limit.min(64 * 1024));   // preallocation capped at 64 KiB
file.take(limit as u64).read_to_end(&mut window)             // read bounded by limit

Validated range, preallocation capped independently of the validated value, bounded read. Not reachable as written.

2 × rust/cleartext-logging — crates/cli/src/lib.rs:2665, 2672

These are mine, added in 5dd4966f2, so they got the most scrutiny. What is interpolated is slot — a secret-store slot name like deepseek, never a value — and error, a SecretsError. That enum is entirely structural: Keyring(String) (OS backend status text), Io, Json (serde position, not content), InsecurePermissions { path, mode }, ReadOnly. No variant carries key material, and no key is read on this path — clear_provider_api_key deletes without fetching.

Worth stating plainly: the alternative to printing that error is what this commit removes. The previous code was let _ = secrets.delete(...), so codewhale auth clear reported success while the key could still be sitting in the keyring. Staying silent to keep the linter quiet would restore a real security defect to suppress a false one.

(crates/cli/src/lib.rs already carries 6 pre-existing alerts for this same rule on main.)

1 × rust/path-injection — crates/tui/src/session_manager.rs:1360

Pre-existing, not introduced here. main already reports exactly one rust/path-injection alert for this file, and git diff --name-only origin/main..HEAD does not include session_manager.rs — this PR does not touch it.

Also still red, and deliberately

Codewhale review fails with Complete PR review requires 2 passes at 200000 characters per pass, but max_passes is 1. That is a provider-spend decision, not a code defect, and raising max_passes is not mine to authorize.

---

For the record: I earlier read the passing Analyze (rust) job as meaning the CodeQL check was stale. It was not — the check aggregates alerts, and the Analyze job succeeding only means the scan ran. Corrected here.