feat(coder): path-jailed code worker for iii agents (v0.5.0) + console views#244
Merged
Conversation
…e views
Adds the `coder` worker — path-jailed file access (read/search/edit/create/
move) for iii harness agents — and the console terminal views that render its
calls. Squash of the feat/coder-multiroot-llm-dx branch.
coder worker
- Multi-root PathResolver: a fixed set of allowed roots with symlink/`..`
containment, in-root absolute paths, and `non_accessible_globs` protection.
Mirror-invariant with the shell worker (parity golden).
- Prescriptive C2xx errors with a redaction invariant: missing and denied are
byte-identical (C211), denied files are wholly absent from search, and
classification never depends on budget state.
- Functions: read-file (windowed/batch/stat probe/numbered lines, output +
batch budgets), search v2 (context lines, response byte budget, default
excludes), update-file (bounded per-op echoes, regex replace with
dot_matches_newline + expect_matches guard + pre-write $-capture-ref
validation), create/delete/move (batched, jailed, cross-root rollback),
tree/list-folder (slimmed wire shape + default_exclude_globs), and info
(discovery of roots, caps, budgets, globs).
- Structured per-entry {code,message} errors; canonical request examples in
every schema; golden wire-schema + error-format + parity + BDD suites with a
catalog/register drift guard; error-frequency instrumentation script.
- Teaching SKILL.md: 2-call edit workflow, window-first reads, wildcard
economy, bulk-rename and outline recipes.
- v0.5.0: runtime config sourced from the `configuration` worker (id "coder",
like database/storage). Storage-style reload split — the PathResolver is the
security jail, built once and never rebuilt: jail-defining fields
(base_paths, non_accessible_globs, default_exclude_globs) are
restart-required, the numeric tuning knobs hot-reload live via an
Arc<RwLock<Arc<CoderConfig>>> snapshot. on-config-change re-fetches the
authoritative value and ignores the trigger payload.
console
- Wire-accurate terminal views for all nine coder::* functions (parsers +
per-function views, replacing the raw-JSON fallback).
- Diff-styled update-file echoes (green additions, red deletion stubs) derived
from the v0.4.1 echo wire; new --color-ok token.
- worker::status view: four-way state pill (running / provisioning / stopped /
not installed), next-step hint, and stderr/stdout log tails.
shell
- host.rs: small addition to hold the path-resolution mirror-invariant with
coder's PathResolver.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughThe PR moves coder to multi-root allowed roots, integrates with the configuration worker for seeding/hot-reload, standardizes structured per-entry WireError, expands function schemas and handlers (info, move, read-file windowing, search, tree, list-folder), and updates console UI, tests, and golden snapshots. ChangesCoder refresh
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
Contributor
skill-check — worker0 verified, 14 skipped (no docs/).
Four for four. Nicely done. |
CI runs clippy on rustc 1.96 with --all-targets --all-features -D warnings; five lints (never hit before this branch reached CI) were denied as errors: - read_window.rs: explicit_counter_loop -> zip a RangeFrom counter - update_file.rs: unnecessary_sort_by -> sort_by_key + Reverse - path/mod.rs: needless_splitn -> split (only .next() is used) - tests/parity.rs: doc_overindented_list_items -> single-line matrix rows - tests/golden_errors.rs: io_other_error -> std::io::Error::other Behavior-preserving; verified on the 1.96 toolchain: fmt clean, clippy --all-targets -D warnings clean, 281 lib + golden/parity/path_jail/update_ops tests green.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
coder/src/functions/delete_file.rs (1)
140-157:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftRecursive delete still has a TOCTOU hole around protected children.
remove_dir_all_safe()only pre-scans the subtree and then hands the whole directory tostd::fs::remove_dir_all(). A non-accessible file created or swapped in after the scan but before the delete phase will bypass the guard and get removed anyway, so the current implementation does not actually preserve thenon_accessible_globsdelete guarantee.🤖 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 `@coder/src/functions/delete_file.rs` around lines 140 - 157, remove_dir_all_safe currently only pre-scans then calls std::fs::remove_dir_all, leaving a TOCTOU window where a protected child can be created between scan and delete; change remove_dir_all_safe to perform the deletion itself in a safe, contents-first traversal so each entry is re-checked with resolver.is_non_accessible immediately before removal and abort with CoderError::not_found_or_denied_subtree if any protected entry is observed. Concretely: replace the final std::fs::remove_dir_all(abs) call with a WalkDir over abs configured contents_first(true).follow_links(false) (or otherwise iterate in post-order), filter_map ok entries, and for each entry call resolver.is_non_accessible(entry.path()) and return the same error if true; otherwise remove files with std::fs::remove_file and directories with std::fs::remove_dir, handling symlinks appropriately and mapping errors via CoderError::from so the deletion is atomic with respect to the protection checks.coder/src/functions/list_folder.rs (1)
123-137:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSymlink entries are being reported as files/directories.
e.metadata()follows the link, soclassify(&entry_md)can never seeis_symlink() == true. A symlink to a file currently serializes askind: "file"and a symlink to a directory askind: "dir", which breaks theEntryKind::Symlinkcontract for downstream callers.Suggested fix
- let entry_md = match e.metadata() { + let entry_ft = match e.file_type() { + Ok(ft) => ft, + Err(_) => continue, + }; + let entry_md = match std::fs::symlink_metadata(e.path()) { Ok(m) => m, Err(_) => continue, }; let abs_entry = e.path(); all.push(DirEntry { name, - kind: classify(&entry_md), + kind: classify(&entry_ft), size: entry_md.len(), mtime: unix_mtime(&entry_md), non_accessible: resolver.is_non_accessible(&abs_entry), });fn classify(ft: &std::fs::FileType) -> EntryKind { if ft.is_symlink() { EntryKind::Symlink } else if ft.is_dir() { EntryKind::Dir } else if ft.is_file() { EntryKind::File } else { EntryKind::Other } }🤖 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 `@coder/src/functions/list_folder.rs` around lines 123 - 137, Replace the follow-symlink metadata call so symlinks are detected: call e.symlink_metadata() (or at minimum e.file_type()) and pass its FileType into classify (change classify signature to accept &std::fs::FileType and check is_symlink() first), then use whatever metadata you need for size/mtime as appropriate; update the DirEntry construction that currently uses e.metadata() / entry_md so kind is computed from the symlink-aware FileType while preserving resolver.is_non_accessible(&abs_entry) and use the new classify(&file_type) call.coder/src/functions/create_file.rs (1)
153-166:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMake
overwrite=falseatomic.The
abs.exists()precheck is racy: another request can create the file after this check andstd::fs::write()will still truncate it, which violates the advertisedoverwrite=falsecontract. Use an atomic create path (OpenOptions::create_new(true)) when overwrite is false.Suggested direction
- if abs.exists() && !spec.overwrite { - return Err(CoderError::AlreadyExists(format!( - "{} already exists; pass overwrite=true to replace", - spec.path - ))); - } + if !spec.overwrite { + let mut file = std::fs::OpenOptions::new() + .write(true) + .create_new(true) + .open(abs) + .map_err(|e| match e.kind() { + std::io::ErrorKind::AlreadyExists => CoderError::AlreadyExists(format!( + "{} already exists; pass overwrite=true to replace", + spec.path + )), + _ => CoderError::io_for_path(e, &spec.path), + })?; + use std::io::Write; + file.write_all(bytes) + .map_err(|e| CoderError::io_for_path(e, &spec.path))?; + } else { + std::fs::write(abs, bytes).map_err(|e| CoderError::io_for_path(e, &spec.path))?; + }🤖 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 `@coder/src/functions/create_file.rs` around lines 153 - 166, The precheck using abs.exists() is racy; change the write path so when spec.overwrite is false you atomically create the file using std::fs::OpenOptions::new().write(true).create_new(true) and then write the bytes (e.g., via write_all) instead of calling std::fs::write; preserve the parents handling (create_dir_all(parent)) and map any I/O errors through CoderError::io_for_path just as you do now, and keep the existing behavior when spec.overwrite is true (fallback to truncating write).console/web/src/components/chat/coder/UpdateFileView.tsx (1)
546-560:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse stable unique list keys even when the same file path appears more than once.
key={file.path}is reused in multiple list renders. If a request includes duplicate paths, React reconciliation can merge/reuse wrong rows.Suggested fix
- {req.files.map((file) => { + {req.files.map((file, i) => { ... - <tr - key={file.path} + <tr + key={`${file.path}:${i}`} className="border-b border-rule-2 last:border-b-0 align-top" >- req.files.map((file, i) => { + req.files.map((file, i) => { const result = resp.results[i] ... - return <FileEchoSection key={file.path} file={file} result={result} /> + return ( + <FileEchoSection + key={`${file.path}:${i}`} + file={file} + result={result} + /> + ) })Also applies to: 608-622
🤖 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 `@console/web/src/components/chat/coder/UpdateFileView.tsx` around lines 546 - 560, The list rows in UpdateFileView map over req.files using key={file.path}, which is not unique when the same path appears multiple times; update both map callbacks (the one mapping req.files around the current snippet and the similar one at the later block) to use a stable unique key per item, e.g., combine file.path with a stable identifier or the array index (like `${file.path}-${idx}`) or prefer a unique property on the file object if available (e.g., file.id or file.uid) to ensure React reconciliation doesn't merge distinct rows.
🧹 Nitpick comments (5)
coder/tests/integration.rs (1)
49-53: Integration boot config is compatible with legacybase_path; no key-compatibility break here.
CoderConfigstill supportsbase_pathas a legacy single-root form that’s honored as a one-entrybase_pathslist; the startup error is only when bothbase_pathandbase_pathsare set.- Optional: update the test YAML to
base_pathsfor consistency with the current config examples/harness.🤖 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 `@coder/tests/integration.rs` around lines 49 - 53, The test writes legacy single-key "base_path" into the generated YAML which is still supported but inconsistent with current examples; update the YAML written into the `yaml` variable so it uses `base_paths:` with an array containing `base.path().display()` instead of `base_path:` (the write call to `std::fs::write(&cfg_path, yaml)` can remain unchanged), ensuring the test config matches the modern multi-root form recognized by `CoderConfig`.coder/tests/features/path_security.feature (1)
36-44: ⚡ Quick winAdd an explicit top-level success assertion for the per-item create case.
This scenario validates per-entry failure semantics, so it should also assert the batch call itself succeeded to lock in that contract.
Suggested test tweak
Scenario: an absolute path outside all roots is rejected per item on create When I call coder::create-file with payload: """ {"files": [ {"path": "/tmp/abs.txt", "content": "x", "mode": "0644", "parents": false, "overwrite": false} ]} """ + Then the call succeeded Then the result for "/tmp/abs.txt" failed with code "C215"🤖 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 `@coder/tests/features/path_security.feature` around lines 36 - 44, Add an explicit top-level success assertion for the batch create call in this Scenario: after the When I call coder::create-file ... payload step and before the per-item check, insert the step that asserts the overall batch call succeeded (e.g. "Then the call succeeded" or "Then the result succeeded") so the feature locks in that the RPC succeeded while individual item "/tmp/abs.txt" failed with code "C215".coder/tests/steps/common.rs (1)
119-125: 💤 Low valueConsider using
.expect()for clearer test diagnostics.The
.unwrap_or("")fallback on line 121 will return an empty string whenentry["error"]["code"]is missing or not a string. While the fullentryis printed in the assertion message, using.expect()would immediately surface a malformed error structure with a clearer panic message.♻️ Proposed refinement
- let err_code = entry["error"]["code"].as_str().unwrap_or(""); + let err_code = entry["error"]["code"] + .as_str() + .expect("entry[\"error\"][\"code\"] must be a string"); assert_eq!( err_code, code, "expected error code {code:?} for {path:?}; got entry: {entry:?}" );🤖 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 `@coder/tests/steps/common.rs` around lines 119 - 125, Replace the silent fallback on entry["error"]["code"] with a fail-fast expectation: change the extraction that sets err_code (currently using entry["error"]["code"].as_str().unwrap_or("")) to use .as_str().expect(...) so the test clearly panics when the error code field is missing or not a string; include a concise message in expect that references the path and entry (e.g., "missing or non-string error.code for {path:?}: {entry:?}") to aid diagnostics in the assertion that follows.console/web/src/components/chat/coder/ReadFileView.tsx (1)
110-125: 💤 Low valueUnconditional
success: trueassumption couples UI to wire-contract parsing.Line 115 always sets
success: truewhen a response is parsed in single-path mode, relying on the comment that "Single-path failures arrive as top-level handler errors." If the wire contract evolves to include explicitsuccessfields in single-path responses, or if the parser unexpectedly succeeds on malformed input, this assumption would break silently.Consider checking
resp.successif the field exists on the wire, or adding a runtime assertion during development to validate this assumption.🤖 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 `@console/web/src/components/chat/coder/ReadFileView.tsx` around lines 110 - 125, The code in ReadFileView.tsx unconditionally sets success: true for the parsed response (variable result of type ReadEntryResult) which couples the UI to a wire-contract assumption; change the construction of result to use the incoming resp.success when present (e.g., set success to resp.success if the key exists, otherwise fall back to true) and add a dev-only runtime assertion (or console.warn) to surface unexpected or non-boolean resp.success values during development so malformed or future-expanded wire payloads don't silently misrepresent success.coder/tests/golden/schemas/coder.info.json (1)
73-75: ⚡ Quick winConsider removing the
primary_rootduplication.The
primary_rootfield is documented as a "convenience duplicate ofbase_paths[0]". This redundancy could lead to inconsistencies if the implementation fails to keep both values synchronized. If API consumers need quick access to the primary root, they can trivially accessbase_paths[0]themselves.If the duplication is retained for backward compatibility or explicit API design reasons, ensure the implementation includes a runtime assertion that
primary_root === base_paths[0].🤖 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 `@coder/tests/golden/schemas/coder.info.json` around lines 73 - 75, Remove the redundant "primary_root" property from the JSON schema (it duplicates base_paths[0]) and update any code that reads coder.info.primary_root to instead read coder.info.base_paths[0]; alternatively, if you must keep primary_root for backwards compatibility, add a runtime assertion in the CoderInfo validation path (e.g., inside your validateCoderInfo/parseCoderInfo logic) that enforces primary_root === base_paths[0] and throw a clear validation error if they differ.
🤖 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 `@coder/README.md`:
- Around line 6-8: Update the README intro to use the exact configuration key
name `non_accessible_globs` instead of `non_accessible`; locate the paragraph
that describes the glob-based list and replace the incorrect key so examples and
prose consistently reference `non_accessible_globs` (ensure any nearby examples,
headings, or mentions are updated to match this exact symbol).
In `@coder/scripts/error-frequency.py`:
- Around line 179-180: The current truthy check "if limit:" causes --sessions 0
to be treated as False and skip slicing; change the condition to explicitly
check for None (e.g., "if limit is not None") where sessions are sliced so that
a numeric 0 correctly results in sessions = sessions[:0]; update the check in
the block that assigns sessions (the variable names 'limit' and 'sessions' in
error-frequency.py) and ensure limit is parsed as an int beforehand so slicing
behaves as expected.
- Around line 451-455: The epilog construction for argparse.ArgumentParser
assumes __doc__ is a string and will fail when __doc__ is None (e.g., -OO);
update the epilog expression used when creating parser (the
argparse.ArgumentParser call that sets epilog=...) to safely handle a missing
docstring by treating __doc__ as an empty string (e.g., use (__doc__ or "") or a
small conditional) before doing the "USAGE" split so the check "USAGE" in
__doc__ and subsequent split operations never run on None.
In `@coder/src/config.rs`:
- Around line 518-523: Test from_yaml_expands_env_var mutates global environment
via std::env::set_var/remove_var; make it deterministic by scoping the env
change with a test-level mutex or a scoped env helper. For example, add a serial
test guard (e.g., annotate from_yaml_expands_env_var with serial_test::serial)
or replace the manual set/remove with a scoped helper (e.g., temp_env::with_var
or similar) so the environment is restored on drop; ensure the change is applied
only for the duration of CoderConfig::from_yaml call and that CODER_TEST_ROOT is
reliably restored after the test.
In `@coder/src/functions/create_file.rs`:
- Around line 166-167: The code currently writes the file before
validating/parsing the requested mode, and apply_mode() re-parses mode_str
afterwards (and trim_start_matches('0') mishandles "0000"), causing bad input to
produce C210 after the file is created; fix by parsing/validating spec.mode up
front (before calling std::fs::write) into an Option<fs::Permissions> or a
numeric mode value, preserve the zero-permission case (do not drop all zeros
with trim_start_matches), and then call apply_mode(abs, parsed_mode) or
equivalent so that write only happens if mode parsing succeeds (update
create_file logic around std::fs::write, apply_mode, and CoderError::io_for_path
to use the pre-parsed mode).
In `@coder/src/functions/read_file.rs`:
- Around line 558-568: The code currently checks md.len() but then uses
std::fs::read(abs) which can TOCTOU if the file grows; replace the full read
with an open+bounded read: call std::fs::File::open(abs) (map errors with
CoderError::io_for_path), then read via file.take(cfg.max_read_bytes as u64 + 1)
into a buffer, and if the number of bytes read is > cfg.max_read_bytes return
the same CoderError::TooLarge message (using wire_path and cfg.max_read_bytes);
this ensures md.len() is only a quick pre-check and the actual read is safely
bounded and reports TooLarge rather than allocating an unbounded buffer.
In `@coder/src/manifest.rs`:
- Around line 25-26: The default_config JSON in manifest.rs currently includes
"/tmp" which widens allowed roots; update the default_config value (the
serde_json! call for "base_paths") to remove "/tmp" and only include the
repo/workdir root (e.g., ["./"]); leave any examples/tests that need absolute
behavior to set explicit absolute paths instead of relying on the manifest
default so no production default allows system temp.
In `@coder/tests/golden/schemas/coder.update-file.json`:
- Around line 29-35: The JSON schema allows 0 for fields that are documented as
1-based coordinates; update the numeric constraints for the 1-based line
properties (e.g., at_line, start_line, end_line) to use "minimum": 1 (and remove
or change any "minimum": 0.0) so schema-valid payloads cannot include 0 for
those 1-based line numbers, and ensure their descriptions remain consistent with
the 1-based contract.
In `@console/web/src/components/chat/coder/UpdateFileView.tsx`:
- Around line 589-591: The header's failed count only tallies resp.results with
success === false, but missing response entries (resp.results.length <
req.files.length) are rendered as "· no result" and should be counted as
failures; update the logic that computes failedCount (used near showEchoes and
the similar block at 611-618) to add the number of missing responses (max(0,
req.files.length - (resp?.results.length ?? 0))) to the existing count of
explicit failures (resp?.results.filter(r => !r.success).length), so the chip
reflects both explicit failures and missing entries.
---
Outside diff comments:
In `@coder/src/functions/create_file.rs`:
- Around line 153-166: The precheck using abs.exists() is racy; change the write
path so when spec.overwrite is false you atomically create the file using
std::fs::OpenOptions::new().write(true).create_new(true) and then write the
bytes (e.g., via write_all) instead of calling std::fs::write; preserve the
parents handling (create_dir_all(parent)) and map any I/O errors through
CoderError::io_for_path just as you do now, and keep the existing behavior when
spec.overwrite is true (fallback to truncating write).
In `@coder/src/functions/delete_file.rs`:
- Around line 140-157: remove_dir_all_safe currently only pre-scans then calls
std::fs::remove_dir_all, leaving a TOCTOU window where a protected child can be
created between scan and delete; change remove_dir_all_safe to perform the
deletion itself in a safe, contents-first traversal so each entry is re-checked
with resolver.is_non_accessible immediately before removal and abort with
CoderError::not_found_or_denied_subtree if any protected entry is observed.
Concretely: replace the final std::fs::remove_dir_all(abs) call with a WalkDir
over abs configured contents_first(true).follow_links(false) (or otherwise
iterate in post-order), filter_map ok entries, and for each entry call
resolver.is_non_accessible(entry.path()) and return the same error if true;
otherwise remove files with std::fs::remove_file and directories with
std::fs::remove_dir, handling symlinks appropriately and mapping errors via
CoderError::from so the deletion is atomic with respect to the protection
checks.
In `@coder/src/functions/list_folder.rs`:
- Around line 123-137: Replace the follow-symlink metadata call so symlinks are
detected: call e.symlink_metadata() (or at minimum e.file_type()) and pass its
FileType into classify (change classify signature to accept &std::fs::FileType
and check is_symlink() first), then use whatever metadata you need for
size/mtime as appropriate; update the DirEntry construction that currently uses
e.metadata() / entry_md so kind is computed from the symlink-aware FileType
while preserving resolver.is_non_accessible(&abs_entry) and use the new
classify(&file_type) call.
In `@console/web/src/components/chat/coder/UpdateFileView.tsx`:
- Around line 546-560: The list rows in UpdateFileView map over req.files using
key={file.path}, which is not unique when the same path appears multiple times;
update both map callbacks (the one mapping req.files around the current snippet
and the similar one at the later block) to use a stable unique key per item,
e.g., combine file.path with a stable identifier or the array index (like
`${file.path}-${idx}`) or prefer a unique property on the file object if
available (e.g., file.id or file.uid) to ensure React reconciliation doesn't
merge distinct rows.
---
Nitpick comments:
In `@coder/tests/features/path_security.feature`:
- Around line 36-44: Add an explicit top-level success assertion for the batch
create call in this Scenario: after the When I call coder::create-file ...
payload step and before the per-item check, insert the step that asserts the
overall batch call succeeded (e.g. "Then the call succeeded" or "Then the result
succeeded") so the feature locks in that the RPC succeeded while individual item
"/tmp/abs.txt" failed with code "C215".
In `@coder/tests/golden/schemas/coder.info.json`:
- Around line 73-75: Remove the redundant "primary_root" property from the JSON
schema (it duplicates base_paths[0]) and update any code that reads
coder.info.primary_root to instead read coder.info.base_paths[0]; alternatively,
if you must keep primary_root for backwards compatibility, add a runtime
assertion in the CoderInfo validation path (e.g., inside your
validateCoderInfo/parseCoderInfo logic) that enforces primary_root ===
base_paths[0] and throw a clear validation error if they differ.
In `@coder/tests/integration.rs`:
- Around line 49-53: The test writes legacy single-key "base_path" into the
generated YAML which is still supported but inconsistent with current examples;
update the YAML written into the `yaml` variable so it uses `base_paths:` with
an array containing `base.path().display()` instead of `base_path:` (the write
call to `std::fs::write(&cfg_path, yaml)` can remain unchanged), ensuring the
test config matches the modern multi-root form recognized by `CoderConfig`.
In `@coder/tests/steps/common.rs`:
- Around line 119-125: Replace the silent fallback on entry["error"]["code"]
with a fail-fast expectation: change the extraction that sets err_code
(currently using entry["error"]["code"].as_str().unwrap_or("")) to use
.as_str().expect(...) so the test clearly panics when the error code field is
missing or not a string; include a concise message in expect that references the
path and entry (e.g., "missing or non-string error.code for {path:?}:
{entry:?}") to aid diagnostics in the assertion that follows.
In `@console/web/src/components/chat/coder/ReadFileView.tsx`:
- Around line 110-125: The code in ReadFileView.tsx unconditionally sets
success: true for the parsed response (variable result of type ReadEntryResult)
which couples the UI to a wire-contract assumption; change the construction of
result to use the incoming resp.success when present (e.g., set success to
resp.success if the key exists, otherwise fall back to true) and add a dev-only
runtime assertion (or console.warn) to surface unexpected or non-boolean
resp.success values during development so malformed or future-expanded wire
payloads don't silently misrepresent success.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1dcb0c1e-4522-4247-8d12-0ad1fccd52da
⛔ Files ignored due to path filters (1)
coder/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (87)
coder/CHANGELOG.mdcoder/Cargo.tomlcoder/README.mdcoder/config.collect.yamlcoder/config.yamlcoder/config.yaml.examplecoder/iii.worker.yamlcoder/scripts/error-frequency.pycoder/skills/SKILL.mdcoder/src/config.rscoder/src/configuration.rscoder/src/error.rscoder/src/functions/create_file.rscoder/src/functions/delete_file.rscoder/src/functions/info.rscoder/src/functions/list_folder.rscoder/src/functions/mod.rscoder/src/functions/move_file.rscoder/src/functions/read_file.rscoder/src/functions/read_window.rscoder/src/functions/search.rscoder/src/functions/tree.rscoder/src/functions/update_file.rscoder/src/lib.rscoder/src/main.rscoder/src/manifest.rscoder/src/path/mod.rscoder/tests/common/helpers.rscoder/tests/common/workers.rscoder/tests/features/delete_file.featurecoder/tests/features/path_security.featurecoder/tests/features/search.featurecoder/tests/features/tree.featurecoder/tests/features/update_file.featurecoder/tests/golden/errors.jsoncoder/tests/golden/schemas/coder.create-file.jsoncoder/tests/golden/schemas/coder.delete-file.jsoncoder/tests/golden/schemas/coder.info.jsoncoder/tests/golden/schemas/coder.list-folder.jsoncoder/tests/golden/schemas/coder.move.jsoncoder/tests/golden/schemas/coder.read-file.jsoncoder/tests/golden/schemas/coder.search.jsoncoder/tests/golden/schemas/coder.tree.jsoncoder/tests/golden/schemas/coder.update-file.jsoncoder/tests/golden_errors.rscoder/tests/golden_schemas.rscoder/tests/integration.rscoder/tests/parity.rscoder/tests/path_jail.rscoder/tests/steps/common.rscoder/tests/steps/read.rscoder/tests/steps/search.rscoder/tests/support/mod.rscoder/tests/update_ops.rsconsole/web/src/components/chat/FunctionCallMessage.tsxconsole/web/src/components/chat/coder/CoderDiff.tsxconsole/web/src/components/chat/coder/CreateFileView.tsxconsole/web/src/components/chat/coder/DeleteFileView.tsxconsole/web/src/components/chat/coder/InfoView.tsxconsole/web/src/components/chat/coder/ListFolderView.tsxconsole/web/src/components/chat/coder/MoveView.tsxconsole/web/src/components/chat/coder/ReadFileView.tsxconsole/web/src/components/chat/coder/SearchView.tsxconsole/web/src/components/chat/coder/TreeView.tsxconsole/web/src/components/chat/coder/UpdateFileView.tsxconsole/web/src/components/chat/coder/__tests__/ReadFileView.test.tsconsole/web/src/components/chat/coder/__tests__/SearchView.test.tsconsole/web/src/components/chat/coder/__tests__/UpdateFileView.test.tsconsole/web/src/components/chat/coder/__tests__/parsers.test.tsconsole/web/src/components/chat/coder/__tests__/treeListFolderViews.test.tsconsole/web/src/components/chat/coder/entryShared.tsxconsole/web/src/components/chat/coder/index.tsxconsole/web/src/components/chat/coder/parsers.tsconsole/web/src/components/chat/sandbox/FsGrepView.tsxconsole/web/src/components/chat/sandbox/__tests__/highlight.test.tsconsole/web/src/components/chat/sandbox/highlight.tsxconsole/web/src/components/chat/worker/WorkerStatusView.tsxconsole/web/src/components/chat/worker/__tests__/parsers.test.tsconsole/web/src/components/chat/worker/index.tsxconsole/web/src/components/chat/worker/parsers.tsconsole/web/src/index.cssconsole/web/src/stories/fixtures/coder-fixtures.tsconsole/web/src/stories/fixtures/worker-fixtures.tsconsole/web/src/stories/playground/scenarios/coder-mutate.tsconsole/web/src/stories/playground/scenarios/coder-update.tsconsole/web/src/stories/playground/scenarios/index.tsshell/src/fs/host.rs
💤 Files with no reviewable changes (1)
- console/web/src/components/chat/coder/CoderDiff.tsx
Comment on lines
+6
to
+8
| glob-based `non_accessible` list keeps sensitive files (`.env`, `*.pem`, | ||
| anything under `secrets/`) visible to directory listings but unreadable | ||
| and unwritable. |
There was a problem hiding this comment.
Use the exact config key name in the intro.
The intro says non_accessible, but the documented/runtime key is non_accessible_globs. Using the exact key here avoids configuration mistakes from copy/paste.
Suggested edit
-glob-based `non_accessible` list keeps sensitive files (`.env`, `*.pem`,
+glob-based `non_accessible_globs` list keeps sensitive files (`.env`, `*.pem`,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| glob-based `non_accessible` list keeps sensitive files (`.env`, `*.pem`, | |
| anything under `secrets/`) visible to directory listings but unreadable | |
| and unwritable. | |
| glob-based `non_accessible_globs` list keeps sensitive files (`.env`, `*.pem`, | |
| anything under `secrets/`) visible to directory listings but unreadable | |
| and unwritable. |
🤖 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 `@coder/README.md` around lines 6 - 8, Update the README intro to use the exact
configuration key name `non_accessible_globs` instead of `non_accessible`;
locate the paragraph that describes the glob-based list and replace the
incorrect key so examples and prose consistently reference
`non_accessible_globs` (ensure any nearby examples, headings, or mentions are
updated to match this exact symbol).
Comment on lines
+179
to
+180
| if limit: | ||
| sessions = sessions[:limit] |
There was a problem hiding this comment.
--sessions 0 currently scans all sessions instead of zero.
Line 179 uses a truthy check, so 0 skips slicing and returns the full list. This makes the CLI do the opposite of the requested limit.
Suggested fix
def _live_sessions(limit: int | None = None) -> list[dict]:
data = _iii("session-tree::list")
if not data or not isinstance(data, dict):
return []
sessions = data.get("sessions", [])
- if limit:
- sessions = sessions[:limit]
+ if limit is not None:
+ sessions = sessions[: max(0, limit)]
return sessions🤖 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 `@coder/scripts/error-frequency.py` around lines 179 - 180, The current truthy
check "if limit:" causes --sessions 0 to be treated as False and skip slicing;
change the condition to explicitly check for None (e.g., "if limit is not None")
where sessions are sliced so that a numeric 0 correctly results in sessions =
sessions[:0]; update the check in the block that assigns sessions (the variable
names 'limit' and 'sessions' in error-frequency.py) and ensure limit is parsed
as an int beforehand so slicing behaves as expected.
Comment on lines
+451
to
+455
| parser = argparse.ArgumentParser( | ||
| description="Count coder C2xx errors by (code, function_id) from session exports or live engine.", | ||
| formatter_class=argparse.RawDescriptionHelpFormatter, | ||
| epilog=__doc__.split("USAGE")[1].split("SESSION-TREE")[0] if "USAGE" in __doc__ else "", | ||
| ) |
There was a problem hiding this comment.
Guard docstring-derived epilog when __doc__ is unavailable.
Line 454 assumes __doc__ is always a string. In optimized runs (-OO), __doc__ may be None, which can crash argument parser setup.
Suggested fix
def main() -> int:
+ doc = __doc__ or ""
+ epilog = doc.split("USAGE", 1)[1].split("SESSION-TREE", 1)[0] if "USAGE" in doc else ""
parser = argparse.ArgumentParser(
description="Count coder C2xx errors by (code, function_id) from session exports or live engine.",
formatter_class=argparse.RawDescriptionHelpFormatter,
- epilog=__doc__.split("USAGE")[1].split("SESSION-TREE")[0] if "USAGE" in __doc__ else "",
+ epilog=epilog,
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| parser = argparse.ArgumentParser( | |
| description="Count coder C2xx errors by (code, function_id) from session exports or live engine.", | |
| formatter_class=argparse.RawDescriptionHelpFormatter, | |
| epilog=__doc__.split("USAGE")[1].split("SESSION-TREE")[0] if "USAGE" in __doc__ else "", | |
| ) | |
| doc = __doc__ or "" | |
| epilog = doc.split("USAGE", 1)[1].split("SESSION-TREE", 1)[0] if "USAGE" in doc else "" | |
| parser = argparse.ArgumentParser( | |
| description="Count coder C2xx errors by (code, function_id) from session exports or live engine.", | |
| formatter_class=argparse.RawDescriptionHelpFormatter, | |
| epilog=epilog, | |
| ) |
🤖 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 `@coder/scripts/error-frequency.py` around lines 451 - 455, The epilog
construction for argparse.ArgumentParser assumes __doc__ is a string and will
fail when __doc__ is None (e.g., -OO); update the epilog expression used when
creating parser (the argparse.ArgumentParser call that sets epilog=...) to
safely handle a missing docstring by treating __doc__ as an empty string (e.g.,
use (__doc__ or "") or a small conditional) before doing the "USAGE" split so
the check "USAGE" in __doc__ and subsequent split operations never run on None.
Comment on lines
+518
to
+523
| fn from_yaml_expands_env_var() { | ||
| std::env::set_var("CODER_TEST_ROOT", "/tmp/expanded-root"); | ||
| let yaml = "base_paths:\n - \"${CODER_TEST_ROOT}\"\n"; | ||
| let cfg = CoderConfig::from_yaml(yaml).unwrap(); | ||
| assert_eq!(cfg.base_paths, vec![PathBuf::from("/tmp/expanded-root")]); | ||
| std::env::remove_var("CODER_TEST_ROOT"); |
There was a problem hiding this comment.
Serialize the env-mutating test.
std::env::set_var / remove_var change process-global state, so this test can race with any other test that reads or writes the environment. Guard it with a test mutex or a scoped env helper so cargo test stays deterministic.
🤖 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 `@coder/src/config.rs` around lines 518 - 523, Test from_yaml_expands_env_var
mutates global environment via std::env::set_var/remove_var; make it
deterministic by scoping the env change with a test-level mutex or a scoped env
helper. For example, add a serial test guard (e.g., annotate
from_yaml_expands_env_var with serial_test::serial) or replace the manual
set/remove with a scoped helper (e.g., temp_env::with_var or similar) so the
environment is restored on drop; ensure the change is applied only for the
duration of CoderConfig::from_yaml call and that CODER_TEST_ROOT is reliably
restored after the test.
Comment on lines
+166
to
+167
| std::fs::write(abs, bytes).map_err(|e| CoderError::io_for_path(e, &spec.path))?; | ||
| apply_mode(abs, &spec.mode)?; |
There was a problem hiding this comment.
Validate and parse mode before writing the file.
apply_mode() parses mode_str after std::fs::write(), so bad input returns C210 after the file has already been created or overwritten. Also, trim_start_matches('0') turns "0000" into "", so a valid zero-permission mode is rejected. Parse once up front and keep the zero case valid before any filesystem mutation happens.
Also applies to: 172-177
🤖 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 `@coder/src/functions/create_file.rs` around lines 166 - 167, The code
currently writes the file before validating/parsing the requested mode, and
apply_mode() re-parses mode_str afterwards (and trim_start_matches('0')
mishandles "0000"), causing bad input to produce C210 after the file is created;
fix by parsing/validating spec.mode up front (before calling std::fs::write)
into an Option<fs::Permissions> or a numeric mode value, preserve the
zero-permission case (do not drop all zeros with trim_start_matches), and then
call apply_mode(abs, parsed_mode) or equivalent so that write only happens if
mode parsing succeeds (update create_file logic around std::fs::write,
apply_mode, and CoderError::io_for_path to use the pre-parsed mode).
Comment on lines
+245
to
+263
| if let Ok(dst_meta) = std::fs::symlink_metadata(abs_to) { | ||
| if dst_meta.is_dir() && !src_meta.is_dir() { | ||
| // overwrite=true can't fix this (a file cannot replace a | ||
| // directory via rename), so don't send the caller down the | ||
| // C217 dead end — tell them the actual corrective call. | ||
| let fname = abs_from | ||
| .file_name() | ||
| .map(|f| f.to_string_lossy().into_owned()) | ||
| .unwrap_or_else(|| "<file>".to_string()); | ||
| return Err(CoderError::BadInput(format!( | ||
| "{wire_to}: destination is a directory; name the target \ | ||
| file inside it (e.g. {wire_to}/{fname})" | ||
| ))); | ||
| } | ||
| if !overwrite { | ||
| return Err(CoderError::AlreadyExists(format!( | ||
| "{wire_to} already exists; pass overwrite=true to replace" | ||
| ))); | ||
| } |
There was a problem hiding this comment.
Rollback corrupts the previous destination when overwrite=true.
Once an existing destination is allowed through, copy_and_delete() overwrites that file in place. If the later source-delete step fails, the rollback path just removes dst, so the original destination contents are gone even though the move did not complete. This can lose data on cross-root moves, and on same-root moves that fall back to copy+delete after EXDEV.
A safe overwrite path needs to preserve the old destination state until the source delete succeeds, e.g. by staging through a temp file / backup and restoring it on rollback.
Also applies to: 308-359
Comment on lines
558
to
+568
| if md.len() > cfg.max_read_bytes { | ||
| return Err(CoderError::TooLarge(format!( | ||
| "{} is {} bytes; max_read_bytes is {}", | ||
| req.path, | ||
| "{} is {} bytes, which exceeds max_read_bytes ({}). \ | ||
| Read a smaller file, raise max_read_bytes in coder config, \ | ||
| or read a slice with line_from/line_to.", | ||
| wire_path, | ||
| md.len(), | ||
| cfg.max_read_bytes | ||
| ))); | ||
| } | ||
| let bytes = std::fs::read(&abs)?; | ||
| let (content, is_utf8) = match String::from_utf8(bytes.clone()) { | ||
| Ok(s) => (s, true), | ||
| Err(_) => (String::from_utf8_lossy(&bytes).into_owned(), false), | ||
| let bytes = std::fs::read(abs).map_err(|e| CoderError::io_for_path(e, wire_path))?; |
There was a problem hiding this comment.
Bound the full-read path after the metadata check.
md.len() is checked first, but std::fs::read(abs) will still read the entire file if it grows between the stat and the read. That bypasses the advertised max_read_bytes ceiling and can turn a C213-sized read into an unbounded allocation. Read through a bounded handle (File::open(...).take(max_read_bytes + 1)) or revalidate after open so TOCTOU growth still fails safely.
🤖 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 `@coder/src/functions/read_file.rs` around lines 558 - 568, The code currently
checks md.len() but then uses std::fs::read(abs) which can TOCTOU if the file
grows; replace the full read with an open+bounded read: call
std::fs::File::open(abs) (map errors with CoderError::io_for_path), then read
via file.take(cfg.max_read_bytes as u64 + 1) into a buffer, and if the number of
bytes read is > cfg.max_read_bytes return the same CoderError::TooLarge message
(using wire_path and cfg.max_read_bytes); this ensures md.len() is only a quick
pre-check and the actual read is safely bounded and reports TooLarge rather than
allocating an unbounded buffer.
Comment on lines
25
to
+26
| default_config: serde_json::json!({ | ||
| "base_path": "./", | ||
| "base_paths": ["./", "/tmp"], |
There was a problem hiding this comment.
Don't ship /tmp as a default allowed root.
Line 26 widens first-run installs to the whole system temp tree. That breaks least-privilege and lets agents read/write unrelated temp files on shared hosts. Keep the published default rooted to the repo/workdir, and show absolute-path behavior in tests/examples instead of the manifest default.
Suggested change
- "base_paths": ["./", "/tmp"],
+ "base_paths": ["./"],📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| default_config: serde_json::json!({ | |
| "base_path": "./", | |
| "base_paths": ["./", "/tmp"], | |
| default_config: serde_json::json!({ | |
| "base_paths": ["./"], |
🤖 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 `@coder/src/manifest.rs` around lines 25 - 26, The default_config JSON in
manifest.rs currently includes "/tmp" which widens allowed roots; update the
default_config value (the serde_json! call for "base_paths") to remove "/tmp"
and only include the repo/workdir root (e.g., ["./"]); leave any examples/tests
that need absolute behavior to set explicit absolute paths instead of relying on
the manifest default so no production default allows system temp.
Comment on lines
+29
to
+35
| "description": "Insert `content` before line `at_line` (1-based). `at_line = lines+1` appends to end of file.", | ||
| "properties": { | ||
| "at_line": { | ||
| "format": "uint32", | ||
| "minimum": 0.0, | ||
| "type": "integer" | ||
| }, |
There was a problem hiding this comment.
The schema says 0 is valid for 1-based line operations.
Line 29, Line 54, and Line 81 describe 1-based coordinates, but the numeric constraints still use minimum: 0.0. That lets agents generate schema-valid payloads that the documented runtime contract should reject with C210.
Suggested change
"at_line": {
"format": "uint32",
- "minimum": 0.0,
+ "minimum": 1.0,
"type": "integer"
}, "from_line": {
"format": "uint32",
- "minimum": 0.0,
+ "minimum": 1.0,
"type": "integer"
}, "to_line": {
"format": "uint32",
- "minimum": 0.0,
+ "minimum": 1.0,
"type": "integer"
},Also applies to: 54-60, 81-100
🤖 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 `@coder/tests/golden/schemas/coder.update-file.json` around lines 29 - 35, The
JSON schema allows 0 for fields that are documented as 1-based coordinates;
update the numeric constraints for the 1-based line properties (e.g., at_line,
start_line, end_line) to use "minimum": 1 (and remove or change any "minimum":
0.0) so schema-valid payloads cannot include 0 for those 1-based line numbers,
and ensure their descriptions remain consistent with the 1-based contract.
Comment on lines
+589
to
591
| const showEchoes = !preview && !running && resp !== null | ||
| const failedCount = resp?.results.filter((r) => !r.success).length ?? 0 | ||
|
|
There was a problem hiding this comment.
Count missing response entries as failed in the header chip.
The UI renders · no result rows, but the failed chip only counts explicit success: false. This underreports failures when resp.results.length < req.files.length.
Also applies to: 611-618
🤖 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 `@console/web/src/components/chat/coder/UpdateFileView.tsx` around lines 589 -
591, The header's failed count only tallies resp.results with success === false,
but missing response entries (resp.results.length < req.files.length) are
rendered as "· no result" and should be counted as failures; update the logic
that computes failedCount (used near showEchoes and the similar block at
611-618) to add the number of missing responses (max(0, req.files.length -
(resp?.results.length ?? 0))) to the existing count of explicit failures
(resp?.results.filter(r => !r.success).length), so the chip reflects both
explicit failures and missing entries.
sergiofilhowz
approved these changes
Jun 11, 2026
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
2 participants
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.
Summary
Adds the
coderworker — path-jailed file access (read / search / edit / create / move) for iii harness agents — and the console terminal views that render its calls. This is the fullfeat/coder-multiroot-llm-dxline (coder v0.3.0 → v0.5.0), squashed and rebased onto currentmain.The design goal throughout: give harness LLM agents a code-access surface that is safe by construction (a hard path jail), token-frugal (windowed/bounded responses), and self-correcting (prescriptive errors that name the next call).
coder worker
..containment, in-root absolute paths, andnon_accessible_globsprotection.PathResolverowns all path logic and is mirror-invariant with the shell worker (coder/tests/parity.rs).read-file— windowed (line_from/line_to), batch (paths[]),statprobe,numberedlines; output + batch byte budgets.searchv2 — context lines, response byte budget, default noise excludes.update-file— bounded per-op echoes (no full-body before/after); regex replace hardened withdot_matches_newline, anexpect_matchesguard, and pre-write$-capture-ref validation (catches the JS/TS${name}template-literal footgun before it silently corrupts a file).create-file/delete-file/move— batched, jailed, with cross-root rollback and a self-move no-op guard.tree/list-folder— slimmed wire shape +default_exclude_globsnoise filter.info— discovery of roots, caps, budgets, and globs.{code,message}per-entry errors, canonical request examples in every schema, and golden wire-schema + error-format + parity + BDD suites with a catalog/register drift guard.SKILL.md— 2-call edit workflow, window-first reads, wildcard economy, bulk-rename and outline recipes.configurationworker (id"coder", likedatabase/storage). Storage-style reload split: thePathResolveris the security jail, built once and never rebuilt at runtime, so jail-defining fields (base_paths,non_accessible_globs,default_exclude_globs) are restart-required, while the numeric tuning knobs (byte caps, page sizes, response budgets) hot-reload live via anArc<RwLock<Arc<CoderConfig>>>snapshot.on-config-changere-fetches the authoritative value and ignores the trigger payload (a bus caller can't loosen the caps or the jail).console
coder::*functions (parsers + per-function views), replacing the raw-JSON fallback.update-fileechoes — green additions, red deletion stubs — derived from the v0.4.1 echo wire; new--color-oktheme token.worker::statusview — four-way state pill (running / provisioning / stopped / not installed), the daemon next-step hint, and stderr/stdout log tails.shell
fs/host.rs— a 6-lineMIRROR-INVARIANTnote documenting thatcanonicalize_with_fallback+normalize_lexicalare mirrored in coder'sPathResolverand must evolve in lockstep.Test plan
cargo test -p coder— 324 tests green (lib + golden wire-schema + error-format + parity + path-jail + update-ops + BDD),cargo fmt/clippyclean.pnpm typecheck+pnpm lint+pnpm test(642 tests) +pnpm build, all green.coder::on-config-changeis registered outsidecatalog()).main(shell v0.4.0); the oneshell/src/fs/host.rsoverlap resolved (kept shell v0.4.0's newconfine_path/lexical_operand_withand coder's mirror-invariant comment); shell compiles, coder parity tests pass.coder/scripts/error-frequency.py(expect the edit task at ≤2 calls).coder/src/path/mod.rs↔shell/src/fs/host.rscanonicalization mirror is still in lockstep before relying on the parity invariant.Summary by CodeRabbit
New Features
Improvements
Bug Fixes