feat: device file operations (read/write/edit/delete/stat/list/glob/copy/move/symlink/chmod)#1
Open
feat: device file operations (read/write/edit/delete/stat/list/glob/copy/move/symlink/chmod)#1
Conversation
…verification, zip-slip hardening, CI cache) - B1: Add #[cfg(windows)] is_process_running using locale-independent PID matching in tasklist output (daemon.rs, admin.rs) - C2: Add zip-slip defense (starts_with check) in browser_init.rs tar extraction - C6: Add Swatinem/rust-cache@v2 to release-rust.yml CI workflow Note: B3 (upgrade checksum), C3 (install.ps1), C7 (taskkill) target code that does not yet exist in the codebase and will be addressed when those files are added. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add `bool interactive = 7` to JobRequest, and new StdinChunk (field 29) and TerminalResize (field 30) messages to Envelope.oneof payload for PTY support. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add `interactive: bool` to `NewJob`, `Job`, and `CreateJobRequest` structs, copy the value into `JobRequest` protobuf, and update all existing construction sites to set `interactive: false`. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Balance the metadata/capabilities grid ratio (3fr/2fr), prevent capabilities panel from stretching, handle long fingerprint overflow, and format fingerprint hex with 8-char grouping for readability. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ints Forwards StdinChunk and TerminalResize protobuf messages to devices via ConnectionRegistry; returns 404 if job not found, 410 if finished, 204 on success; requires dashboard_access auth; adds gone() and not_found() helpers to ApiError. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…e handlers Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add portable-pty dependency and implement run_job_pty function that allocates a pseudo-terminal for interactive process execution. The PTY merges stdout/stderr into a single stream and accepts stdin data and resize commands via an unbounded mpsc channel. Includes StdinInput enum and StdinSender type alias as the public API surface. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Extend JobRegistry with stdin sender storage and wire up the WebSocket message loop so StdinChunk/TerminalResize payloads reach the correct PTY job. spawn_job now branches on req.interactive to allocate a PTY with stdin channel instead of plain pipes. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…e mode Add InteractiveTerminal component using @xterm/xterm and @xterm/addon-fit, with dynamic imports to avoid SSR issues. Users can toggle between pipe (existing command-by-command) and interactive (full PTY session) modes. Interactive mode creates jobs with `interactive: true`, streams output via SSE, forwards keystrokes via POST to stdin, and handles terminal resize. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add POST /api/terminal/token for generating one-time tokens with 60s TTL and GET /ws/terminal for bidirectional browser-to-daemon terminal I/O. The WS handler bridges browser binary frames (keystrokes) to daemon StdinChunk messages, text frames (resize JSON) to TerminalResize, and daemon output events back as binary WS frames to the browser. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Define FileRequest/FileResponse messages with 14 operations (read text/binary/image, write, edit, delete, chmod, stat, list, glob, mkdir, copy, move, symlink) and wire them into Envelope payload oneof as fields 31/32. Generate both Rust and TypeScript types via prost_build and buf. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Introduce the FileManager module with FilePolicyChecker (path allowlist/denylist, traversal detection, dangerous-path approval hint) and wire FileRequest routing through ahand_client into a dispatch-ready FileManager::handle. Individual operations remain unimplemented placeholders until Tasks 2-6. Adds file_policy config section, image/glob/trash/encoding_rs/chardetng dependencies, and renames FileErrorCode and AclEntryType enum prefixes so prost strips them cleanly. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add the metadata-query and directory-creation operations in the new fs_ops submodule. Each handler maps std::io::Error to FileErrorCode, reports mtime/ size/symlink targets, and respects policy-resolved paths. FileList paginates by offset+max_results with mtime-desc ordering; FileGlob resolves patterns against an optional base_path and caps total results. Comes with 20 integration tests covering happy paths, pagination, hidden filtering, not-a-directory, path-traversal rejection, and policy denial. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Adds text_read submodule handling FileReadText with: - Triple-limit stopping: max_lines, max_bytes, target_end — whichever fires first - Start position by line, byte, or line+col - Per-line truncation via max_line_width with UTF-8-safe cuts and remaining_bytes - PositionInfo for both start and end positions (line, byte_in_file, byte_in_line) - Encoding auto-detection (BOM, chardetng) plus forced encoding via parameter - Total file bytes + total lines + remaining bytes after the stop point Includes 13 integration tests covering happy paths, all three stop reasons, each start variant, truncation semantics, empty files, UTF-8 boundary safety, forced GBK decoding, and error propagation. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add binary byte-range reading with offset + length + max_bytes caps, and on-device image reading with optional resize/format-conversion/compression. Image decoding happens in spawn_blocking so we don't block the tokio runtime. Lossy encoders (JPEG) iteratively lower quality to fit max_bytes; lossless formats (PNG) return a single pass. Comes with 9 integration tests covering full-file/range/max_bytes/past-EOF reads and image passthrough/resize/format-conversion/budget-compression/ not-an-image error propagation. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Adds the write_ops submodule with FullWrite, FileAppend, StringReplace, LineRangeReplace, and ByteRangeReplace for FileWrite, plus the edit variants that require existing files. StringReplace returns the proto MatchError code for not-found and multi-match cases (FileWrite errors out, FileEdit reports match_error without touching the file). All methods enforce max_write_bytes from policy. Comes with 14 integration tests covering happy paths, overwrite, append, create_parents, replace-all vs single, line/byte-range semantics, too-large writes, and edit-on-missing-file. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add the remaining mutation operations in fs_ops: - FileDelete: TRASH (via system trash crate) and PERMANENT modes with recursive guard (returns NotEmpty when non-empty + recursive=false) - FileCopy: file + recursive directory copy with overwrite control - FileMove: rename with cross-filesystem copy+delete fallback - FileCreateSymlink: Unix symlink creation (gated by cfg(unix)) - FileChmod: Unix mode setting with optional recursive descent; chown returns PermissionDenied for now Comes with 12 integration tests covering all mutation paths including recursive delete, copy, chmod, plus overwrite guards and error cases. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add POST /api/devices/{device_id}/files that accepts a base64-encoded
FileRequest proto, forwards it to the connected device via the WebSocket
gateway, and waits (30s) for the correlated FileResponse to come back.
Responses are matched by request_id via a new PendingFileRequests map
owned by AppState and resolved from JobRuntime::handle_device_frame when a
FileResponse arrives on the device socket.
Also fixes two pre-existing compile breaks in the hub crate caused by
proto field additions (HelloAccepted.update_suggestion,
JobRequest.interactive) that hadn't been updated in the hub sources.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Introduces the S3Config section in hub config, an S3Client wrapper around
aws-sdk-s3 for generating presigned PUT/GET URLs and uploading/downloading
raw bytes, and a new POST /api/devices/{device_id}/files/upload-url endpoint
that hands out presigned upload URLs to dashboard clients.
AppState now carries an optional Arc<S3Client>. When S3 is not configured,
the upload-url endpoint returns 503 Service Unavailable with a stable error
code; the inline-content file flow continues to work below the configured
transfer threshold. Hub-driven upload/download inside the existing file
operation endpoint is still inline-only — wiring the full threshold-based
read/write transfer path is left as a focused follow-up.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Adds 17 end-to-end tests that exercise the full proto round trip for every file operation: encode a FileRequest into bytes, decode, run it through FileManager::handle, encode the FileResponse, then decode and validate the result. Complements tests/file_ops.rs (which tests the handler surface) by catching any wire-format regressions that type-level tests would miss. Covers the happy path for all 13 operations plus policy-denied and not-found error flows, for a total of 68 + 17 = 85 file-op integration tests passing on the daemon side. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Pre-existing compile break from the update_suggestion field that was added to HelloAccepted alongside the auto-update feature but never propagated to the roundtrip test. Caught while running the full test suite for the file operations branch. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Round 1 review flagged that POST /api/devices/{id}/files/upload-url returns
a valid presigned PUT URL but the daemon still rejects any FileRequest
carrying FullWrite.s3_object_key, so the half-wired S3 path is only usable
up to the S3 PUT itself. Remove the route and handler to avoid exposing a
half-working API surface while keeping the underlying s3.rs infrastructure,
S3Config, and AppState.s3_client ready for the follow-up PR that wires the
full hub-side transfer path.
Also adds the Round 1 review-fixes plan and task tracker.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Round 1 review #15: the POST /api/devices/{id}/files handler accepted an
opaque JSON shape ({"proto_b64": "..."}) instead of following hub REST
convention or just using the proto binary directly. Switch to raw
application/x-protobuf for both the request body (axum::body::Bytes) and
response body (IntoResponse with an explicit content-type header).
Deletes the hand-rolled simple_base64 module (fixes #27 padding bug for
free), removes FileOperationRequest/FileOperationResponse wrapper structs,
and simplifies the handler signature. Hub lib tests still pass.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Round 1 review #9 and #21: - Before decoding, read the image header via ImageReader::into_dimensions() and reject when width * height * 4 exceeds max_read_bytes (policy). Avoids decompression-bomb OOM before the pixel buffer gets allocated. - When the caller supplies no max_width/max_height/max_bytes/quality and leaves output_format at Original, return the original file bytes byte-for-byte instead of running them through decode→encode. That's what the proto spec promised by "omit compression params = raw transfer". Propagates policy.max_read_bytes() into handle_read_image via a new third argument. All 5 existing image tests still pass; byte-for-byte passthrough assertion will be added in task T19. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Round 1 review #13: fs_ops.rs unconditionally imported std::os::unix::fs::PermissionsExt and used Permissions::from_mode / .mode() in handle_mkdir, handle_chmod, set_unix_mode_sync, and unix_permission_from_metadata. That would break Windows compilation on first contact with the crate. Gate the Unix-only pieces behind #[cfg(unix)]. On Windows: - handle_mkdir ignores the requested mode (advisory, no warning) - handle_chmod's Unix branch returns Unspecified "not supported on this platform" until a proper Windows ACL implementation lands - unix_permission_from_metadata returns mode: None The existing Windows ACL fallback path is left untouched. All 68 file_ops integration tests still pass on macOS. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Round 1 review #17: s3.rs had zero tests. Add four tests that run without any real S3 endpoint or AWS credentials: - s3_config_default_values: S3Config::default() field values - s3_client_constructs_with_fake_endpoint: S3Client::new against an unreachable localhost endpoint doesn't panic, accessors return config - s3_client_generate_upload_url_returns_populated_url: presigned PUT URL has the forced endpoint, bucket in path-style form, object key, and a positive expiration - s3_client_generate_download_url_returns_populated_url: same for GET Uses synthetic AWS_ACCESS_KEY_ID/AWS_SECRET_ACCESS_KEY env vars because the aws-sdk-s3 presigner needs *something* to HMAC with, even though the computation is purely local. Full round-trip tests against MinIO remain follow-up work. Hub lib test count: 31 → 35. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Round 1 review #10: apply_byte_range_replace computed (br.byte_offset + br.byte_length) as usize without checking for u64 overflow. Inputs like byte_offset=5, byte_length=u64::MAX wrapped below start, passed the bounds check, and panicked during Vec::with_capacity. Switch to u64::checked_add for overflow detection (returns InvalidPath on overflow) and do the bounds check in u64 before casting to usize. The existing out-of-bounds error code/message is preserved to avoid breaking downstream expectations. All existing byte-range tests still pass; the u64::MAX regression test is added in task T18. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…enabled Round 1 review #19: the daemon's Hello.capabilities only surfaced "exec" and optionally "browser", so hub/dashboard consumers had no way to tell which devices could take a FileRequest. Add a file_enabled parameter to build_hello_envelope and push "file" into the capabilities vector when true, mirroring the existing browser handling. Production call site in connect_with_auth now passes file_mgr.is_enabled(); hub_handshake tests pass false since they don't test this axis. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Round 1 review #2: FilePolicyChecker.check_path was doing purely lexical normalization, so a symlink inside the allowlist pointing at /etc/passwd would pass the check and any subsequent read/write would operate on the outside target. Replace `normalize_path` with `canonicalize_or_parent` which walks symlinks via std::fs::canonicalize. For paths that don't exist yet (destinations of Write/Mkdir/Copy/Move) walk up to find the deepest existing ancestor, canonicalize that ancestor, and re-append the remaining suffix. The `..` rejection in step 1 of check_path guarantees the suffix is free of traversal. Add a `no_follow_symlink: bool` parameter so operations that explicitly opt out of following the final component (e.g. `Stat { no_follow_symlink }` calling symlink_metadata downstream) get a resolved parent + preserved filename instead of a target-resolved path. Every dispatch site in FileManager is updated to pass req.no_follow_symlink when the op has one, false otherwise. CreateSymlink passes true because we're creating the symlink file itself at link_path. Test module rewritten to use real tempdir fixtures (previously used hardcoded `/home/user/**` patterns that don't exist on macOS). New test `symlink_pointing_outside_allowlist_is_rejected` pins the security fix. 12 policy tests + 68 file_ops integration tests pass. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Round 1 review #1: handle_glob fed req.pattern directly to glob::glob with no re-check on the resulting paths. An absolute pattern like `/etc/**` or a traversal pattern like `../../*` bypassed the allowlist entirely, and even benign `**/*.txt` patterns could surface symlinks inside the allowlist whose targets pointed outside. Two-part fix: 1. Dispatcher rejects glob patterns that start with `/` (absolute) or contain a `..` component before calling handle_glob. This stops the most obvious escape vectors at the API boundary. 2. handle_glob now receives a `&FilePolicyChecker` and re-checks every matched path via `policy.check_path(..., no_follow_symlink=false)`. Because policy canonicalizes, symlinks whose canonical target lies outside the allowlist are silently filtered out of the result set. Adds 3 new integration tests: - glob_absolute_pattern_without_base_is_rejected - glob_traversal_pattern_is_rejected - glob_skips_symlink_pointing_outside_allowlist All 71 file_ops tests pass. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Round 1 review #6: FilePolicyConfig.max_read_bytes was defined but only read by handle_read_image (via T10). Text and binary read handlers let the caller pass an arbitrarily large per-request max_bytes with no policy ceiling. Thread max_read_bytes into handle_read_text and handle_read_binary and clamp the effective per-call cap against it. Text reads now refuse to buffer the whole file when metadata.len() > max_read_bytes, returning FILE_ERROR_CODE_TOO_LARGE before any allocation. Binary reads clamp max = req.max_bytes.unwrap_or(DEFAULT_BINARY_MAX).min(max_read_bytes) so a caller-provided 1 GB max is still capped at the policy value. All 68+ file_ops integration tests and 12 policy unit tests still pass; T19 will add a dedicated max_read_bytes clamp regression test. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Round 1 review #18: FileWrite.encoding and FileEdit.encoding were proto fields but write_ops never consulted them, silently downgrading any requested encoding (e.g. GBK) to UTF-8 writes. V1 scope is UTF-8 only; full encoding support is future work. Add ensure_encoding_supported() at the top of handle_write and handle_edit to reject anything that isn't empty/utf-8/utf8 (case-insensitive) with FILE_ERROR_CODE_ENCODING before any filesystem I/O. Update the proto comment on both FileWrite and FileEdit encoding fields to document the v1 limitation and regenerate TypeScript bindings. All 71 file_ops tests still pass; T18 will add the "FileWrite with encoding=gbk returns encoding error" regression test. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Round 1 review #3: handle_file_request was treating
SessionDecision::NeedsApproval as Allow, so STRICT session mode let
file-op requests through without any approval round trip. Only jobs had
a real approval path.
Mirror handle_job_request's approval flow:
1. Synthesize a JobRequest with tool="file", job_id=request_id, args=[op]
2. On NeedsApproval, submit to ApprovalManager → get (ApprovalRequest, rx)
3. Send the ApprovalRequest to cloud via WS and broadcast to IPC clients
4. Spawn a task that awaits the response with the default timeout
5. On approved → dispatch to FileManager as usual
6. On denied/timeout → send FileResponse{error: PolicyDenied}
The spawned task mirrors the job approval shape but produces FileResponse
(not JobRejected). ApprovalResponse's job_id field lines up with the
synthetic request_id so approval_mgr.resolve routes correctly. Thread
approval_mgr and approval_broadcast_tx into handle_file_request at the
call site. Wire the dispatch loop to pass them in.
All 148 ahandd tests still pass; T5 will plumb dangerous_paths into the
same approval path, and T18/T19 will add the strict-mode file-op approval
integration tests.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Round 1 review #5: PolicyResult.needs_approval was computed but never consulted, so paths listed in FilePolicyConfig.dangerous_paths never actually forced STRICT approval regardless of session mode — they just got the same Allow treatment as any other path. Two-part fix in handle_file_request: 1. Pre-flight policy check: before consulting the session manager, walk every path the FileRequest touches (via the new FileManager::check_request_approval helper + collect_request_paths) and record whether any path is in dangerous_paths. Outright policy denials short-circuit with a FileError before we ever consult the session manager. 2. Unify Allow-dangerous and NeedsApproval into the same approval- submission path. Allow + dangerous uses the reason "path is listed in dangerous_paths" with no prior refusals; NeedsApproval forwards the session reason and previous_refusals. Both fall through to a single spawned task that awaits the approval response and dispatches the FileRequest on approval (or returns PolicyDenied on deny/timeout). The new collect_request_paths helper mirrors the dispatch arms so future op additions must keep both call sites in sync — documented in the helper's rustdoc. All 155 ahandd tests still pass. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…positions Two tightly coupled Round 1 fixes (T7 + T8): **T7 — target_end / max_bytes can now stop partway through a line.** The previous implementation only evaluated stop conditions at line boundaries, so a target_end pointing into the middle of line N still returned the whole line, and max_bytes=120 could overshoot to the next newline. The rewritten loop computes per-line cut points for both limits, stripped to the earliest, and the emitted TextLine content is truncated at that exact raw byte. end_byte / bytes_accumulated / PositionInfo all advance to the cut, not to the line boundary. **T8 — all reported positions are now on-disk raw bytes, not decoded UTF-8.** Line offsets are computed on the raw buffer via compute_line_offsets_raw; start_byte / target_end / byte_in_file / byte_in_line / remaining_bytes are all measured in raw bytes. Line bodies are decoded per-line via encoding_rs::Encoding::decode only for the output string, so non-UTF-8 inputs (GBK, Shift-JIS) get correct file-byte positions instead of decoded-buffer offsets. This preserves the proto contract for FileReadText.PositionInfo.byte_in_file. Special case: target_end landing exactly on a line boundary stops WITHOUT emitting an empty TextLine, matching the existing read_text_respects_target_end_line test's "exclusive" semantics. All 13 read_text tests + 68 other file_ops tests pass. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Round 1 review #14: FileWrite.no_follow_symlink, FileEdit.no_follow_symlink, and FileChmod.no_follow_symlink were proto fields but every handler ignored them. Callers setting the flag to avoid touching a symlink's target still had it modified because write/edit/chmod go through path-based syscalls (tokio::fs::write, fs::set_permissions, etc.) that follow symlinks on the final component. Add a shared pub(super) helper `reject_if_final_component_is_symlink` in file_manager::mod that uses symlink_metadata (never follows) to inspect the final component. When the flag is set and the component is a symlink, return FILE_ERROR_CODE_INVALID_PATH before any filesystem mutation. Missing files fall through so the downstream handlers keep their existing NotFound behavior. Called from handle_write and handle_edit in write_ops.rs (after the existing encoding check, before any I/O) and from handle_chmod in fs_ops.rs (after the permission presence check, before the permission branches). delete/copy/move keep their own symlink handling — those already use symlink_metadata or have different semantics around following. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Round 1 review #4: PendingFileRequests keyed only on request_id, which is caller-chosen, so two devices with a colliding ID could cross- contaminate — one device's FileResponse could resolve the other's waiting HTTP handler. Even a single device could clobber its own in-flight request on retry. Change the DashMap key from `String` to `(String, String)` tuple `(device_id, request_id)`. register/resolve/cancel now take both, and the three call sites in `file_operation` (pre-send register, send-failure cancel, timeout cancel) pass `&device_id` alongside the request id. The FileResponse arm of `handle_device_frame` in jobs.rs passes its existing `device_id: &str` parameter when resolving. T17 will add the cross-device-collision regression test; current 35 hub lib tests still pass. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Round 1 review #12: PendingFileRequests had no admission control, so an authenticated dashboard user could spam /api/devices/*/files and force the hub to retain an unbounded number of 30-second waiters until timeout. Add a hard cap (`MAX_PENDING_FILE_REQUESTS = 1024`) enforced inside `register`, which now returns a `Result<oneshot::Receiver, AtCapacity>` instead of a bare Receiver. The file_operation handler maps the capacity error to a 503 Service Unavailable with the stable `FILE_REQUESTS_SATURATED` error code. Makes `PendingFileRequests` a proper struct with a capacity field, plus a `new(capacity)` constructor and a test-only `in_flight()` accessor for T17's saturation regression test. 35 hub lib tests still pass. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…None)
Covers the gaps Reviewer C pinned in Round 1:
- file_write_string_replace_not_found_returns_error: FileWrite path for
the old_string-not-found case (T23 from review). Pins that FileWrite
errors outright while FileEdit uses match_error (already tested).
- line_range_replace_start_line_past_eof_errors: start_line beyond
total_lines is rejected.
- line_range_replace_end_line_clamped_past_total: end_line > total is
clamped to the last line rather than failing.
- byte_range_replace_at_eof_inserts: pure insertion at EOF with
byte_length=0.
- byte_range_replace_pure_insert_mid_file: same with an interior offset.
- byte_range_replace_u64_overflow_returns_error: T9 regression —
byte_length=u64::MAX returns InvalidPath instead of panicking.
- file_write_encoding_non_utf8_rejected + file_edit_encoding_non_utf8_*: T14
regression — FileWrite/FileEdit with encoding="gbk" return Encoding
error and leave the target untouched.
- write_refuses_symlink_when_no_follow_set + edit_refuses_* +
chmod_refuses_*: T11 regressions — no_follow_symlink=true on a symlink
errors without touching the target.
- delete_symlink_no_follow_removes_link_not_target: delete with
no_follow_symlink=true removes just the symlink file, leaving the
target intact.
- copy_recursive_overwrite_merges_into_existing_destination: T23 gap.
- file_request_with_no_operation_returns_unspecified_error: T20 tiny
hole — FileRequest { operation: None } → Unspecified.
Test count: 71 → 85 (+14). Full ahandd test suite green.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…Requests Round 1 review #16: http/files.rs had zero tests. Add two test layers: **Inline unit tests** in `src/http/files.rs`: - pending_file_requests_cross_device_collision_does_not_resolve_other_waiter (T12 regression — caller-chosen request_ids do not bleed across devices) - pending_file_requests_resolves_correct_device (positive path) - pending_file_requests_admission_control_rejects_over_capacity (T13 regression — the 3rd register past capacity=2 returns AtCapacity) - pending_file_requests_admission_control_accepts_after_cancel (capacity frees up) **Axum integration tests** in `crates/ahand-hub/tests/http_files.rs`: - file_operation_happy_path_returns_device_response — encodes a proto FileRequest, attaches a fake device via the support helpers, drives the full HTTP round trip, and verifies the returned proto body - file_operation_unauthenticated_returns_401 - file_operation_device_offline_returns_409 (pre-registered but never attached) - file_operation_empty_body_returns_400 - file_operation_malformed_proto_returns_400 - file_response_from_wrong_device_does_not_resolve_other_waiter — device-b sends a FileResponse with device-a's request_id; device-a's HTTP waiter stays blocked, then device-a's real response completes it Added `TestDevice::recv_file_request` and `TestDevice::send_file_response` to `tests/support/mod.rs`, plus `TestServer::state_ref()` so tests can seed extra device records and issue dashboard tokens. Timeout test is explicitly deferred — the 30s default is compile-time constant in files.rs and exposing a test-only override would add more production surface than the test is worth. Hub lib tests: 35 → 39. New http_files integration tests: 6 passing. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Covers the gaps Reviewer C pinned for Task 3 (text) and Task 4 (image): **Text (T8 regressions):** - read_text_reports_total_lines_and_full_position_info: total_lines is populated, start_pos and end_pos both set line/byte_in_file/byte_in_line. - read_text_start_line_col_reports_full_position_info: PositionInfo is byte-accurate after a mid-line start_line_col. - read_text_gbk_autodetect_without_forced_encoding: chardetng detects GBK for a buffer that's not valid UTF-8 and decodes it without requiring the caller to set encoding. - read_text_byte_positions_match_raw_on_disk_bytes_for_gbk: PositionInfo is in raw bytes (4, 8) not decoded bytes (6, 12) for GBK input. This is the core T8 contract. **Image (T10 regressions):** - read_image_passthrough_is_byte_for_byte_identical: passthrough mode now returns the original file bytes unchanged (no decode/encode round trip). - read_image_max_height_only_preserves_aspect_ratio: max_height-only resize scales width proportionally. - read_image_both_axis_resize_preserves_aspect_ratio: both axes set, smaller axis wins, ratio preserved. - read_image_jpeg_source_can_be_read: non-PNG source works end-to-end. - read_image_quality_clamp_accepts_out_of_range_values: quality=0 and quality=200 both produce valid JPEG output (clamp not reject). - read_image_bomb_guard_rejects_oversized_dimensions: 1024x1024 RGBA = 4MB decoded > 1MB max_read_bytes → TooLarge, rejected before decode. Uses a tight-budget FileManager to exercise the header-only path. **T20 plan doc alignment:** - Update the inline code comment in the plan for test_read_text_target_end to say "4 lines returned" (exclusive) with a note that target_end is EXCLUSIVE. Matches the implementation and the existing read_text_respects_target_end_line test assertion. Test count: 85 → 95. Full ahandd suite green. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
crates/ahandd/src/file_manager/(policy, fs_ops, text_read, binary_read, write_ops) with session-mode integration and path allowlist/denylist enforcement.POST /api/devices/{id}/fileswith request_id correlation and 504 timeout handling, plus thePOST /api/devices/{id}/files/upload-urlendpoint backed by aws-sdk-s3 (503 when S3 disabled).What's included
feat(protocol): add file operations proto definitionsfeat(daemon): add FileManager skeleton with policy and routingfeat(daemon): implement FileStat, FileList, FileGlob, FileMkdirfeat(daemon): implement FileReadText with triple-limit paginationfeat(daemon): implement FileReadBinary and FileReadImagefeat(daemon): implement FileWrite and FileEdit operationsfeat(daemon): implement Delete, Copy, Move, Symlink, Chmodfeat(hub): add file operation HTTP endpoints and response routingfeat(hub): add S3 pre-signed URL flow for large file transfertest(daemon): add comprehensive file operations E2E testsfix(protocol): add update_suggestion field to HelloAccepted test caseTest plan
cargo build -p ahand-protocol -p ahandd -p ahand-hubcargo test -p ahand-protocol— 8 passedcargo test -p ahandd— 148 passed (32 unit + 26 openclaw + 68 file_ops + 17 e2e + 5 hub_handshake)cargo test -p ahand-hub --lib— 31 passedNotes
/files/upload-urlendpoint returns 503 Service Unavailable with a stableS3_DISABLEDerror code when no[s3]config is present, so environments without S3 see a deterministic fallback rather than a crash.POST /filesstill transfers the full FileResponse body. Wiring the threshold-based S3 upload inside that endpoint is left as a focused follow-up PR so the daemon-side work can land first.cargo test -p ahand-protocolondevand I took it out of the way while running the full test sweep.🤖 Generated with Claude Code