feat: add Windows cross-platform support#2
Open
winrey wants to merge 19 commits into
Open
Conversation
… path Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add fs_perms module providing restrict_owner_only (chmod 600 equivalent) and restrict_owner_and_group (chmod 660 equivalent) that work on both Unix (via PermissionsExt) and Windows (via Win32 ACL APIs). Includes 9 integration tests covering happy paths, error cases, idempotency, and permission transitions. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add [lib] and [[bin]] sections to Cargo.toml for clarity when both lib.rs and main.rs coexist. Improve fs_perms_test.rs with better test isolation (tempdir for error cases), separate nonexistent-file tests for both functions, and proper #[cfg(unix)] gating on platform-specific assertions. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add protect/unprotect functions wrapping CryptProtectData/CryptUnprotectData for encrypting Ed25519 private keys at rest on Windows, bound to the current user account. Module is cfg(windows)-gated and skipped on other platforms. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Generic handle_ipc_conn<R: AsyncRead, W: AsyncWrite> - serve_ipc dispatches to serve_ipc_unix / serve_ipc_windows via #[cfg] - spawn_ipc_handler helper prevents generic propagation - Windows peer identity via GetNamedPipeClientProcessId + LookupAccountSidW - Replace set_permissions with fs_perms::restrict_owner_and_group Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace all 5 UnixStream::connect call sites (ipc_exec, ipc_cancel, ipc_approve, ipc_policy, ipc_session) with a single ipc_connect() helper that abstracts over Unix domain sockets and Windows named 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>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…latform paths Use #[cfg(windows)]/#[cfg(unix)] to branch Node.js download format (zip vs tar.xz), binary paths (node.exe vs bin/node, npm.cmd vs bin/npm), playwright-cli paths, and clean logic (taskkill on Windows). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…dmin Replace the bash-shelling upgrade.rs with a pure Rust implementation that uses reqwest to fetch releases from GitHub and downloads binaries directly. Includes Windows support with .exe rename-before-overwrite strategy. Replace admin panel's bash browser-setup subprocess with an SSE message directing users to the native ahandd browser-init command. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Exercises the length-prefixed framing layer (write_frame / read_frame) that underpins cross-platform IPC. Covers: basic roundtrip, protobuf envelope serialisation, multiple sequential frames, empty frames, bidirectional request-response, oversized-frame rejection, and truncated-payload error handling. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… config Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…64 platform suffix Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add integration tests that exercise the real serve_ipc production server over Unix sockets, including JobRequest->JobRejected/JobFinished roundtrip, AutoAccept mode job execution, SessionQuery->SessionState roundtrip, query-all-sessions, and peer identity format verification. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Fix tasklist sentinel string in daemon.rs (C1) - Make Named Pipe accept loop resilient to transient errors (C2) - Fix install.ps1 checksum filename mapping (I1) - Add write_frame size guard matching read side (I2) - Fix admin.rs is_process_running for Windows (I3) - Use enclosed_name() to prevent zip-slip in browser_init (I4) - Replace sleep-based test sync with socket polling (I5) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- B1: locale-safe tasklist PID check (daemon.rs, admin.rs) - B3: SHA-256 checksum verification on upgrade/install-daemon downloads - C2: zip-slip defense-in-depth (starts_with check in browser_init) - C3: install.ps1 asserts both binaries were verified - C6: add Swatinem/rust-cache@v2 to CI workflows - C7: remove system-wide taskkill /IM node.exe from clean() Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Reject Named Pipe connections with unverifiable identity (B-1) - Guard against zero-size GetTokenInformation buffer (B-2) - Hard-fail on checksum fetch/missing entry in upgrade and install-daemon (B-3) - Zero DPAPI plaintext buffer before LocalFree (B-8) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
winrey
added a commit
that referenced
this pull request
Apr 27, 2026
…istency - docs/remote-control-roadmap.md §2: list the new POST /api/control/browser endpoint and CloudClient.browser() under "Current status". - packages/sdk/CHANGELOG.md: fix Before-table types (Buffer optional, not Uint8Array | null) to match the actual pre-rename type; clarify that hub-side dedupe is tracked as cross-repo spec follow-up #3. - crates/ahand-hub/src/browser_service.rs: correct the NOTE(idempotency) comment to point at follow-up #3 (hub-side idempotency) instead of #2 (tool-args redaction); fully-qualify the cross-repo spec path so a reader inside the aHand checkout doesn't follow a broken link. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
winrey
added a commit
that referenced
this pull request
Apr 27, 2026
* feat(sdk): add timeout error code; map HTTP 504
Extend CloudClientErrorCode union with 'timeout' and update HTTP error
mapping to route 504 responses to the new timeout error code. This enables
the new CloudClient.browser() method to surface gateway timeouts without
resorting to generic server_error. Existing spawn() call sites are
unaffected, as hub timeouts have always reached them via SSE error events.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(sdk): add CloudClient.browser() — control-plane browser invoke
Implements POST /api/control/browser as a single request-response RPC
(no SSE) matching the wire schema Task 9 will land. Decodes base64
binary_data into Uint8Array under result.binary, maps snake_case fields
(device_id, session_id, timeout_ms, correlation_id, duration_ms) to
camelCase BrowserParams/BrowserResult, and reuses the existing
toTypedHttpError taxonomy (401/403/404/429/504/5xx + abort/network).
Renames the existing connection.ts WebSocket-side BrowserResult to
DeviceBrowserResult to free the public BrowserResult export for the
new control-plane shape. No external consumer references the renamed
symbol — verified across aHand workspace and the team9-agent-pi
feature worktree.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(hub): extract browser_service::execute() shared function
Move the device-lookup, capability-check, oneshot-registration,
envelope-construction, WS-dispatch, and timeout-await machinery out of
the dashboard `POST /api/browser` handler into a new shared service
function `browser_service::execute()`. The dashboard handler is now a
thin wrapper that handles auth + body parsing + JSON encoding around
the service call. `map_service_error` lives `pub(crate)` in
`http/browser.rs` so Task 9's `/api/control/browser` endpoint can reuse
it.
Pure refactor: external behavior of `POST /api/browser` is unchanged
(all 6 dashboard browser_api integration tests pass without
modification). `BrowserCommandInput.correlation_id` is wired now but
unused; the dashboard passes None, Task 9's worker handler will pass
the caller-supplied id.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(hub): POST /api/control/browser — worker-side browser endpoint
Mounts under control_plane::router() so the existing
require_control_plane_jwt middleware handles auth. The handler
verifies device ownership (claims.external_user_id ==
device.external_user_id) before delegating to browser_service::execute(),
mirrors the dashboard wire format with one addition (duration_ms),
and shares the same error mapper as the dashboard endpoint.
Tests cover 200/401/403/404/400/504 plus a binary base64 round-trip.
* fix(hub): /api/control/browser scope check + tests for allowlist/rate-limit; clarify idempotency doc
Parity with sibling control-plane handlers (`create_job`, `stream_job`,
`cancel_job`):
- `browser_command_control` now validates `claims.scope == "jobs:execute"`
before any DB / WS work, mirroring the other handlers. A token minted
with a non-execute scope (e.g. `jobs:read`) is rejected 403 FORBIDDEN.
- New test `control_browser_403_when_device_not_in_allowlist` covers the
`device_ids` allowlist branch (mirrors
`create_job_rejects_device_not_in_allowlist`).
- New test `control_browser_returns_429_when_rate_limited` covers the
per-user rate-limit branch (mirrors `rate_limit_returns_429`).
- Module doc-comment tightened to clarify that hub-layer idempotency
applies only to `POST /api/control/jobs` today; `/api/control/browser`
accepts `correlation_id` in the wire schema but does not currently
dedupe (tracked as a follow-up).
Spec drift on idempotency for `/api/control/browser` will be addressed
in a follow-up commit on the team9-agent-pi side (cross-repo).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(sdk): wrap res.json/strict success check; cover post-token abort + 400/418/502/503 mappings
Source fixes in cloud-client.ts:
- Wrap success-path `res.json()` in try/catch so AbortError during body
read becomes CloudClientError("abort"), SyntaxError (HTML 502 page
with 200 status) becomes server_error, others become network. Error
path was already wrapped via toTypedHttpError.
- Replace `success: json.success === true` coercion with strict
typeof === "boolean" check; throw server_error on malformed shapes
(e.g. success: 1, missing field) rather than silently flipping to
false. Catches future hub serializer drift early.
Tests added (7 new, 45 → 52 in src/cloud-client.test.ts):
- Post-token abort path: signal aborted while getAuthToken pending →
fetch never called, code=abort. Regression guard for the second
abort fast-path.
- success === true strict: missing field → server_error;
non-boolean (success: 1) → server_error.
- Status mappings: 400 → bad_request; 502 / 503 → server_error
(non-mapped 5xx default branch); 418 → bad_request (non-mapped 4xx
default branch).
* test(hub): cover scope/idempotency/channel-closed/wire-omission paths for /api/control/browser
Add 5 new tests + strengthen 1 existing assertion to close gaps
identified during review of the /api/control/browser endpoint:
- T1: control_browser_403_when_scope_not_jobs_execute pins the
claims.scope guard at control_plane.rs:626-632 (mirrors the
sibling create_job_rejects_wrong_scope template).
- T2: control_browser_correlation_id_does_not_dedupe_currently
pins the documented "no dedupe" behavior of correlation_id so
a future regression that accidentally implements idempotency
fails loudly.
- T3: control_browser_500_when_response_channel_closed exercises
the BrowserServiceError::ChannelClosed -> 500 INTERNAL_ERROR
branch by reaching into state.browser_pending and dropping the
oneshot tx after dispatch.
- T4: control_browser_404_device_offline_on_send_failure is
filed as #[ignore] with a sketch + unblock condition because
ConnectionRegistry has no trait surface for clean injection.
- T5: control_browser_returns_429_when_rate_limited gains a
follow-up assertion that non-429 responses are exactly 404
DEVICE_NOT_FOUND, guarding against a regression where the
rate-limiter fires before auth/lookup.
- T6: control_browser_200_with_valid_jwt_and_owner_match now
asserts the optional fields (error / binary_data / binary_mime)
are OMITTED from the JSON object, not serialized as null —
matching the dashboard test invariant.
Also adds mint_cp_jwt_with_scope helper for T1.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(hub): RAII cleanup for browser_pending; sanitize internal store error
Two review findings on `browser_service::execute()`:
A2 (Important): when the HTTP client disconnects between the
`browser_pending.insert(request_id, tx)` and the `tokio::time::timeout(rx)`
resolving, axum cancels the handler future at the `.await`. The explicit
`.remove()` calls in each error branch never run, so the entry leaks for
the lifetime of the hub process. Replace the per-branch removes with an
RAII `PendingGuard` whose `Drop` removes the entry — runs on every exit
path, including future cancellation. `DashMap::remove` returns `Option<_>`
so the guard's drop and the WS gateway's success-path remove at
`device_gateway.rs:685` are both idempotent.
A4 (Important): the device-store error message was being echoed to
clients via `format!("device store: {e}")` -> `Internal(msg)` ->
`map_service_error` -> response body. Pre-refactor this went through
`HubError::Internal` -> ApiError mapping which produced a generic
"Internal server error" string, so the refactor regressed the error
contract by leaking driver / SQL details. Sanitize at the source: log
the raw error via `tracing::error!` with structured fields, return a
generic "device store error" string to the client.
Regression test `browser_command_pending_cleared_on_client_cancel`
already exists in HEAD (added in 6a1695b) and exercises A2 directly —
verified it FAILS with the pre-fix code (entry leaks) and PASSES with
the guard.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(hub): restore 'Internal server error' message; ignored stub for device-store error path
* fix(sdk): guard malformed JSON root; cover res.json catch branches + signal forwarding
* docs: add SDK CHANGELOG note for BrowserResult rename; clarify deferred-idempotency comment
- packages/sdk/CHANGELOG.md (new): document the WS-side BrowserResult → DeviceBrowserResult rename and the new HTTP-side BrowserResult shape; flag correlation_id as not-yet-deduped on the hub side.
- browser_service.rs: replace the misleading "suppress unused-field warning" comment with a NOTE(idempotency) block pointing at the module doc-comment and the spec follow-up.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(hub): apply rustfmt + Copilot review nits (PendingGuard doc, SendFailed comment)
- cargo fmt: bring tests/browser_api.rs and browser_service.rs in line with CI rustfmt.
- PendingGuard doc: drop stale "unless explicitly disarmed" — there is no disarm path; the guard always fires on Drop. (Copilot review #3144279775)
- SendFailed branch comment: was "via Internal", actual variant is SendFailed. (Copilot review #3144279781)
Rejected Copilot review: double device-store lookup. Pre-existing pattern from create_job; documented as follow-up in spec; perf overhead acceptable for v1.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* test: cover timeout floor, malformed body, camelCase rejection, daemon-error wire, SDK array-root
Adds 6 missing tests identified by a test-completeness audit:
- T1: control_browser_timeout_ms_zero_floors_to_1000ms — pins the
`timeout_ms.max(1000)` floor in browser_service::execute. A regression
that drops the floor would surface as flaky sub-second 504s.
- T2: browser_command_returns_400_for_malformed_json_body /
browser_command_returns_400_for_missing_required_field — pins the
JsonRejection -> 400 VALIDATION_ERROR contract on the dashboard
/api/browser handler.
- T3: control_browser_returns_400_for_malformed_json_body /
control_browser_returns_400_for_missing_required_field — same
coverage on the worker-side /api/control/browser handler.
- T4: control_browser_400_when_request_uses_camelcase_keys — pins the
snake_case wire contract on ControlBrowserRequest. A future commit
that flipped to camelCase would silently change the API.
- T5: control_browser_daemon_failure_returned_as_200_with_error_echoed —
pins that daemon-reported success: false is NOT promoted to an HTTP
4xx/5xx; the body carries success: false + error verbatim with a
duration_ms field, so SDK consumers can branch on body.success.
- T6 (TS): rejects malformed root: response is a JSON array — pins
that the SDK strict guard rejects array roots (typeof [] === "object"
but .success !== boolean -> server_error).
All tests-only; no source changes.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* docs: update roadmap, CHANGELOG, browser_service comment for doc-consistency
- docs/remote-control-roadmap.md §2: list the new POST /api/control/browser endpoint and CloudClient.browser() under "Current status".
- packages/sdk/CHANGELOG.md: fix Before-table types (Buffer optional, not Uint8Array | null) to match the actual pre-rename type; clarify that hub-side dedupe is tracked as cross-repo spec follow-up #3.
- crates/ahand-hub/src/browser_service.rs: correct the NOTE(idempotency) comment to point at follow-up #3 (hub-side idempotency) instead of #2 (tool-args redaction); fully-qualify the cross-repo spec path so a reader inside the aHand checkout doesn't follow a broken link.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore(sdk): bump @ahandai/sdk to 0.1.6 for browser() release
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
winrey
added a commit
that referenced
this pull request
Apr 27, 2026
…opy/move/symlink/chmod) (#1) * feat(protocol): add file operations proto definitions 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> * feat(daemon): add FileManager skeleton with policy and routing 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> * feat(daemon): implement FileStat, FileList, FileGlob, FileMkdir 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> * feat(daemon): implement FileReadText with triple-limit pagination 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> * feat(daemon): implement FileReadBinary and FileReadImage 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> * feat(daemon): implement FileWrite and FileEdit operations 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> * feat(daemon): implement Delete, Copy, Move, Symlink, Chmod 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> * feat(hub): add file operation HTTP endpoints and response routing 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> * feat(hub): add S3 pre-signed URL flow for large file transfer 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> * test(daemon): add comprehensive file operations E2E tests 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> * fix(protocol): add update_suggestion field to HelloAccepted test case 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> * fix(hub): gate S3 upload-url flow until end-to-end transfer lands 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> * fix(hub): swap base64 JSON wrapper for application/x-protobuf on /files 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> * fix(daemon): image read bomb guard and raw passthrough mode 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> * fix(daemon): gate fs_ops Unix-only APIs behind cfg(unix) 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> * test(hub): add unit tests for s3.rs construction and presigning 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> * fix(daemon): ByteRangeReplace uses checked_add to reject u64 overflow 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> * feat(daemon): advertise file capability in Hello when FileManager is 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> * fix(daemon): canonicalize paths in policy checker to resolve symlinks 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> * fix(daemon): FileGlob rejects hostile patterns and re-checks every match 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> * fix(daemon): enforce policy max_read_bytes for text and binary reads 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> * fix(daemon): reject non-UTF-8 write encoding with clear error 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> * fix(daemon): strict-mode file operations now go through approval flow 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> * fix(daemon): dangerous_paths force approval even in Allow sessions 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> * fix(daemon): text read stops mid-line at byte limits and reports raw 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> * fix(daemon): honor no_follow_symlink in write/edit/chmod handlers 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> * fix(hub): key PendingFileRequests by (device_id, request_id) 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> * fix(hub): cap PendingFileRequests at 1024 waiters (admission control) 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> * test(daemon): add write/edit/fs edge-case tests (T18 + T20 operation=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> * test(hub): cover POST /api/devices/{id}/files handler and PendingFileRequests 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> * test(daemon): add text/image edge-case coverage (T19 + T20 plan doc fix) 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> * fix(daemon): text read truncate_line uses raw bytes, not decoded UTF-8 Round 2 C1: truncate_line previously cut decoded.as_bytes() (UTF-8 byte length of the decoded string) and then subtracted cut.len() from the raw on-disk slice length. For non-UTF-8 encodings (GBK, Shift-JIS, …) the decoded UTF-8 length differs from the raw length, so a GBK file with max_line_width=4 reported the wrong content and wrong remaining_bytes. Rewrite truncate_line to take (raw_line: &[u8], encoding, max_width), cut the raw slice at max_width raw bytes first (backing up to a UTF-8 char boundary only for UTF-8 input), then decode only the retained prefix. remaining_bytes is now always in raw on-disk bytes, matching the proto contract. decode_slice and safe_utf8_prefix helpers dropped since everything flows through truncate_line. The new read_text_truncates_gbk_in_raw_bytes_not_decoded_bytes test pins the fix: GBK "你好世界" (8 raw bytes) with max_line_width=4 returns content="你好" and remaining_bytes=4. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(hub): PendingFileRequests atomic admission + reject duplicate keys Round 2 C2 + C3: register() did a separate len()-then-insert which could over-subscribe the cap under concurrent callers, and it unconditionally replaced existing entries for the same (device_id, request_id) — causing the first waiter to fail through the closed-channel path with a 500. Switch to an AtomicUsize-backed in_flight counter: register() fetch_adds first, rolls back on over-capacity, and only then enters a DashMap::entry critical section to check for duplicates. The new PendingFileRequestsError enum has AtCapacity and Duplicate variants; file_operation maps them to 503 FILE_REQUESTS_SATURATED and 409 FILE_REQUEST_DUPLICATE respectively. Two new regression tests pin the behavior: - pending_file_requests_rejects_duplicate_keys_explicitly - pending_file_requests_capacity_is_atomic_under_concurrent_registers (spawns 32 parallel tasks against a cap-4 table and asserts exactly 4 accepted / 28 rejected) Hub lib tests: 35 → 37 (plus 2 from R1). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(daemon): image read enforces max_read_bytes and fails on unreachable budget Round 2 C5 + C6: - handle_read_image called tokio::fs::read() BEFORE checking the file size against max_read_bytes. A 500 MB file with tiny image dimensions (header-only payload) blew the daemon's read budget at the read step. Passthrough mode had the same hole. Fix: check metadata.len() against max_read_bytes right after the is_file check, returning TooLarge before the read. This covers both passthrough and the normal pipeline. - encode_image previously returned the last encode even if it still exceeded the requested max_bytes after the quality floor. For JPEG / WebP this meant callers silently got oversize output; for PNG (lossless) max_bytes was essentially ignored. Now both paths return TooLarge when the floor/lossless encode still exceeds the budget. Adds two regression tests to file_ops.rs. The noise-image tests use wrapping arithmetic so debug-mode u8 overflow doesn't panic. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(daemon): list uses symlink_metadata + bounded heap (no follow, no DoS) Round 2 C8 + S9: handle_list had two problems. 1. entry.metadata() follows symlinks, so a symlink in an allowed directory that points at /etc/passwd would surface the target's type, size, and mtime in the listing — leaking metadata for a file outside the allowlist. 2. Every directory entry was collected into an unbounded Vec before sorting. A malicious or just very large directory with millions of entries could OOM the daemon regardless of max_read_bytes. Rewrite handle_list to use tokio::fs::symlink_metadata on each entry (never follows) and a bounded min-heap keyed by (mtime, index) that caps retained entries at offset + max_results + 64 (and an absolute ceiling of 100_000). Total_count still tracks every non-hidden scanned entry so has_more stays correct. The new list_does_not_follow_symlink_into_target_metadata test (cfg(unix)) pins the symlink-leak fix by creating a symlink pointing at /etc/hosts inside the listed directory and asserting that the reported file_type is Symlink (not File) with the target surfaced via symlink_target. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(hub): s3.rs tests use explicit credentials instead of env mutation Round 2 I7 + S16: the s3 test module previously called `unsafe { std::env::set_var(...) }` in an ensure_fake_aws_creds helper. Cargo runs tests in parallel, so env mutation across threads is unsound (Rust 2024 edition explicitly marks it unsafe) and can intermittently corrupt other tests' env state. Replace the env mutation with an explicit credentials provider built via `aws_credential_types::Credentials::new("test", "test", ...)`. Add a new `#[cfg(test)] impl S3Client { fn for_test(...) }` constructor that accepts a pre-built `aws_sdk_s3::Client`, bucket, threshold, and expiration. Tests now build the SDK client through `sdk_client_with_explicit_creds` which goes through the explicit `aws_sdk_s3::config::Builder` path, never touching process env. Also moves the newly-required `aws-credential-types = "1"` dep from `[dependencies]` to `[dev-dependencies]` — it's now only referenced by the test module, so ahand-hub's runtime dependency graph loses that crate (resolving the original "unused aws-credential-types" concern too). Production `S3Client::new` is unchanged; the 4 existing s3 tests pass. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * test(daemon): add Round 2 Wave 1 regression tests (R0, R3, R5, R13) Tests for the Wave 1 fixes: - read_text_truncates_gbk_in_raw_bytes_not_decoded_bytes (R0) - read_image_input_size_exceeds_max_read_bytes_is_rejected (R3) - read_image_max_bytes_unreachable_returns_too_large (R3) - list_does_not_follow_symlink_into_target_metadata (R5/R13) Also lands the Round 2 fixes plan doc and task tracker. 99 file_ops tests passing. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(daemon): symlink target check, glob approval escalation, recursive delete STRICT, TOCTOU mitigation Four related policy-layer fixes to file_manager/mod.rs (Round 2 C4, C7, S2, S1): **R2 (symlink target check)**: FileCreateSymlink dispatch previously only validated the link_path, so a caller could create an allowed symlink pointing at /etc/passwd and later use it as an allowlist bypass surface. Now collect_request_paths emits BOTH link_path AND target, and the dispatch arm re-checks the target (resolved against the link's parent if relative) before calling handle_create_symlink. **R4 (glob approval escalation)**: check_request_approval previously only checked base_path for `dangerous_paths` hits. A glob over an allowed directory could surface files listed in `dangerous_paths` without triggering the approval flow. The new glob_has_dangerous_match helper expands the glob (up to a 10_000-match safety cap) and re-checks each match against policy. check_request_approval is now async so it can perform this walk. **R9 (recursive PERMANENT delete → STRICT)**: spec rule (design.md:635) requires recursive DELETE_MODE_PERMANENT in TRUST mode to escalate to STRICT approval. check_request_approval now returns true for that shape regardless of which paths are involved. **R10 (nonexistent-path TOCTOU mitigation)**: canonicalize_or_parent rebuilds a non-existing path from its deepest existing ancestor, so an attacker could swap a component for a symlink between the policy check and the actual operation. The new verify_post_create helper re-canonicalizes the resolved path AFTER write/mkdir/copy/move/ symlink creation and deletes (best-effort) anything that escaped the allowlist. This is a mitigation — full TOCTOU protection requires openat2+RESOLVE_NO_SYMLINKS and is deferred to a follow-up PR, as documented in the helper's rustdoc. All 102 file_ops integration tests pass. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(daemon): text read empty-encoding auto-detect, LineCol clamp, max_bytes=0 Three small Round 2 fixes in text_read.rs: R7 — empty encoding string means auto-detect, not UTF-8. The proto contract says encoding="" should auto-detect via BOM/chardetng, but resolve_encoding returned UTF_8 on empty. Now `Some("")` falls through to the same auto-detect path as `None`. R12 — LineCol.col is clamped against the current line's end, not raw_len. For both resolve_start_raw and resolve_position_raw, when col exceeds the line's printable length the resolved byte position lands at the line's end (or the trailing newline) instead of spilling into the next line. Honours the proto's "byte offset within line" semantics. R16 — max_bytes=0 returns zero lines (no empty TextLine). The previous implementation emitted one empty `TextLine` then stopped with STOP_REASON_MAX_BYTES; now we early-break when the byte budget is exhausted before pushing. Three new regression tests pin all three fixes: - read_text_empty_encoding_triggers_auto_detect - read_text_line_col_with_col_past_eol_clamps_to_line_end - read_text_max_bytes_zero_returns_no_lines Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(daemon): TRASH delete populates trash_path on macOS / freedesktop Linux Round 2 S10: handle_delete previously hardcoded trash_path: None even in TRASH mode. The trash crate (5.x) doesn't expose a per-item destination from `trash::delete`, but on platforms with a deterministic per-user trash directory we can reproduce the path well enough to give the caller a useful hint. Add `guess_trash_path(original)` that resolves: - macOS: $HOME/.Trash/<basename> - Freedesktop Linux: $XDG_DATA_HOME/Trash/files/<basename> (fallback to $HOME/.local/share/Trash/files/<basename>) - Windows / non-Unix / no $HOME: None The handle_delete TRASH branch now calls this and reports it in the result. A long rustdoc comment on both the call site and the helper documents the limits: the trash system can rename on collisions and non-home volumes use $VOLUME/.Trashes/<uid>/, so this is a hint, not a guarantee. Two new unit tests + one #[ignore]d real-trash integration test (gated to multi_thread runtime because handle_delete uses block_in_place). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(daemon): config loader expands ~ in file_policy path patterns Round 2 S4: the design spec uses `~/.ssh/**`, `~/.gnupg/**`, and `~/.bashrc` in its example file_policy config, but FilePolicyChecker stores patterns verbatim and glob_match compares them literally against canonicalized absolute paths. So `~/.ssh/**` would never actually match `/home/user/.ssh/id_rsa` and the spec's own examples were silently broken. Add `expand_tilde(&str)` and `expand_tildes_in_place(&mut Vec<String>)` helpers in config.rs. Supports `~/...` (Unix), `~\...` (Windows), and bare `~`. When `dirs::home_dir()` returns `None`, log a `tracing::warn!` and leave the original pattern unchanged so daemon startup is never blocked by tilde expansion failures. Wire into `Config::load` so FilePolicyChecker only ever sees absolute paths. `path_allowlist`, `path_denylist`, and `dangerous_paths` are all expanded. Three unit tests inside config.rs cover the home-rooted, leave-alone, and middle-tilde cases. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(hub): file_operation enforces application/x-protobuf content-type Round 2 S11 / Path A #6: the handler documented that the request body must be application/x-protobuf, but never actually checked. A client sending a valid protobuf body with Content-Type: application/json was silently accepted, which masked schema confusion until much later in the device runtime. Add a content-type check immediately after auth: if the header is present and its base type isn't application/x-protobuf (after stripping any `; charset=...` suffix), return 415 Unsupported Media Type with the stable error code UNSUPPORTED_MEDIA_TYPE. Missing or empty Content-Type is still tolerated for backwards compatibility with older test harnesses that don't set it. The existing 6 http_files integration tests still pass, plus the new file_operation_rejects_non_protobuf_content_type which posts a valid protobuf body labelled application/json and asserts 415. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(daemon): namespace file approvals, include paths, cancel on connection close Three related Round 2 fixes to the file-request approval flow in handle_file_request: R6 — file approvals now use a dedicated `file-req:<request_id>` namespace inside ApprovalManager. Previously file requests reused the caller-chosen request_id as the synthetic JobRequest.job_id, which shares ApprovalManager's HashMap with real job approvals. A file request could evict an unrelated pending job approval (or vice versa). The namespaced key is opaque to cloud — the wire envelope still echoes back exactly the job_id we sent, so no cloud-side change is needed. R8 — the synthetic JobRequest's args now contains the actual paths the request touches (via FileManager::request_paths), not just `[op_name]`. The dashboard approval prompt now shows the user what file is being approved instead of just `tool=file, args=[read_text]`. R20 — the detached approval-await task now selects on a watch::Receiver fed by a CloseGuard owned by connect_with_auth. When the WS connection closes (cleanly or by error), the guard drops, the watch flips, and all pending file-approval tasks bail out immediately instead of waiting up to 24 hours for the ApprovalManager default timeout. ApprovalManager.expire is still called on the bail path so the manager's HashMap doesn't leak. FileManager::request_paths is a thin wrapper over the existing collect_request_paths helper that returns just the path strings, keeping ahand_client.rs from needing to know the helper exists. All 104 file_ops integration tests pass. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * chore(plan): mark Round 2 Wave 2 tasks completed (R6, R7, R8, R11, R12, R14-R16, R20) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * refactor(hub): move PendingFileRequests out of http into its own module Round 2 S15: PendingFileRequests was defined in src/http/files.rs but referenced by JobRuntime::handle_device_frame in the WS layer (src/http/jobs.rs) and by AppState (src/state.rs). That coupled WebSocket frame handling to an HTTP endpoint module, which is a layering inversion — the HTTP and WS layers should both depend on a transport-neutral piece of state. Move PendingFileRequests + PendingFileRequestsError + MAX_PENDING_FILE_REQUESTS + new_pending_requests + the inline #[cfg(test)] mod into a new top-level module `crate::pending_file_requests`. http/files.rs now imports PendingFileRequestsError from there; state.rs and http/jobs.rs do the same for PendingFileRequests itself. No behavior change. All 6 inline unit tests still pass under the new module path; the http_files integration tests are unaffected. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * test(daemon): cover dangerous_paths approval, recursive delete, file capability Round 2 R21 — close the test coverage gaps Reviewer B flagged. Adds 6 new tests in tests/file_ops.rs: - check_request_approval_returns_true_for_dangerous_path_read: a read of a path listed in dangerous_paths returns Ok(true) so the approval flow fires regardless of session mode. - check_request_approval_returns_false_for_normal_path: control case. - check_request_approval_returns_true_for_recursive_permanent_delete: R9 spec rule (design.md:635) — recursive PERMANENT delete always escalates, even on a path NOT in dangerous_paths. - check_request_approval_returns_false_for_non_recursive_permanent_delete: control case proving the escalation is recursive-only. - check_request_approval_returns_true_for_dangerous_path_trash_delete: TRASH mode is not auto-escalated, but a dangerous path still is. - check_request_approval_denies_symlink_target_outside_allowlist (R2): CreateSymlink with target=/etc/passwd is rejected at preflight with PolicyDenied, never reaching the dispatch handler. Adds 2 new tests in tests/hub_handshake.rs: - build_hello_envelope_includes_file_capability_when_enabled: capabilities.contains("file") when file_enabled=true (the positive case was previously uncovered — every existing handshake test passes false). - build_hello_envelope_excludes_file_capability_when_disabled: control. Plus a new manager_with_dangerous helper for tests that need to set up a temp-dir-scoped FilePolicyConfig with dangerous_paths populated. 110 file_ops tests + 7 hub_handshake tests passing. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(daemon): C1 break connection-teardown deadlock in ahand_client Round 3 critical: the existing CloseGuard formed a circular wait on disconnect — send_handle.await blocked on tx_clone, the spawned approval task held tx_clone and waited on close_rx, close_rx waited on close_tx, close_tx waited on _close_guard's drop, and the guard waited for connect_with_auth to return — which was waiting on send_handle.await. The result was a permanent hang on disconnect rather than a clean reconnect. Fix the cycle so the close signal fires before the joins, letting spawned tasks observe shutdown and release tx_clone. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(hub): I1 release pending file-request slot on client cancel Round 3 important: when an HTTP client closed the connection mid-flight (browser navigates away, network drop), Axum dropped the file_operation handler future without ever reaching the timeout/error/channel-close branches that explicitly call cancel(). The slot stayed in PendingFileRequests until the device responded — and if the device never did, the entry was permanently leaked. After 1024 such leaks the admission cap rejected every new request, producing a slow DoS. Wrap the slot in a PendingSlotGuard whose Drop calls cancel(). Now every exit path — explicit error returns, timeout, channel close, the success return after encoding the response, AND the early future-drop case from client cancellation — releases the slot. cancel() is idempotent against a slot already removed by resolve(), so the success path is a no-op. Promote PendingFileRequests::in_flight() from cfg(test) to a public observability helper so integration tests (and future metrics) can count live slots. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(daemon): I2 fail-loud on tilde expansion when HOME unset Round 3 important: expand_tilde used to log a warning and return the original pattern when dirs::home_dir() resolved to None. The downstream glob_match compares canonicalized absolute paths against the literal pattern, so an unexpanded "~/.ssh/**" silently never matches anything. That fails the *wrong* way for path_denylist / dangerous_paths — the operator's intent ("this path is dangerous") flips to "this path is allowed" without any error surface. Reject the config at startup instead. expand_tilde_with(pattern, home) returns Result; a tilde pattern paired with home == None now produces an anyhow::Error that propagates out of Config::load. Patterns without a leading tilde pass through unchanged so containerized deployments (no HOME) continue to work as long as they use absolute paths. The home_dir lookup is also factored into a wrapper so the failure case is testable without mucking with environment variables. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(daemon): file-ops review round 3 — C2/C3/C4/C5 + I3/I4/I6 Bundles the remaining file-ops daemon fixes from Round 3 because their changes share types (EscalationKind, ApprovalEscalation) and test fixtures, and splitting them per item would leave intermediate states where check_request_approval's signature didn't match its call sites. C2 — relative symlink targets bypassing dangerous_paths approval. collect_request_paths only handed absolute targets to the dangerous-path scan; a crafted FileCreateSymlink with target "../secret.txt" and a link_path inside the allowlist slipped past the check. We now resolve relative targets against link_path's parent (lexical normalization, no filesystem touch) before classifying. C3 — glob approval scan failed open at the safety cap. After scanning GLOB_APPROVAL_SCAN_CAP (10_000) matches without seeing a dangerous one, the old code returned "no approval needed" — but match #10_001 might have been dangerous. We now distinguish CapHit from Clean: if the glob iterator still has items past the cap, we fail closed with the new EscalationKind::GlobScanCapHit. The cap is now a static AtomicUsize + public set_glob_approval_scan_cap helper so tests can lower it without materializing 10k+ files. C4 — paginated list with offset > LIST_HEAP_RETAIN_CAP returned an empty page with has_more=true forever. The bounded-heap retains at most LIST_HEAP_RETAIN_CAP entries, so we structurally cannot serve a page past that window. handle_list now refuses up front with InvalidPath and a clear hint to use FileGlob for deep pagination. C5 — TRASH delete ignored the recursive flag. PERMANENT delete enforced "non-empty + recursive=false → NotEmpty"; TRASH did not, silently moving whole subtrees to the system trash. The TRASH branch now runs the same guard before invoking trash::delete(), and items_deleted reflects the recursive count instead of always being 1. I3 — replace/edit OOM on oversized existing files. apply_string_replace / apply_line_range_replace / apply_byte_range_replace and the inline read in handle_string_replace_edit all called read_to_string / read on the existing file *before* any size check. A 100 GB file would OOM during the read even though the post-read enforce_size_limit was guaranteed to reject. New enforce_existing_size_limit stat-checks the file first and returns TooLarge before the slurp. I4 — structured ApprovalEscalation reason. check_request_approval used to return bool; the approval prompt then hardcoded "path is listed in dangerous_paths" regardless of why. It now returns Option<ApprovalEscalation> with a discriminated EscalationKind (RecursivePermanentDelete / DangerousPath / DangerousGlobMatch / GlobScanCapHit), the specific path that triggered escalation when applicable, and a precise human-readable reason for the prompt. I6 — EXDEV detection wasn't portable. handle_move only treated raw_os_error == Some(18) as cross-device, which is EXDEV on Linux/ macOS but NOT Windows' ERROR_NOT_SAME_DEVICE = 17. A cross-volume MoveFile on Windows would skip the copy+delete fallback and surface as a generic IO error. New is_cross_device_error prefers the stable io::ErrorKind::CrossesDevices and keeps numeric checks (Unix 18, Windows 17) as a defensive belt-and-suspenders. Tests: regression coverage for each item — relative-target escalation (C2), glob CapHit via lowered cap with RAII guard (C3), deep-offset list returns InvalidPath (C4), TRASH non-recursive on non-empty dir returns NotEmpty without touching the system trash (C5), oversized existing-file refusal for both Write StringReplace and Edit StringReplace paths (I3), updated approval tests to assert the new EscalationKind variants (I4), and platform-aware unit tests for is_cross_device_error (I6). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * style(hub): clear pre-existing fmt + clippy drift to unblock hub-CI The hub-CI workflow gates on \`cargo fmt --check\` and \`cargo clippy -- -D warnings\` over ahand-protocol / ahand-hub-core / ahand-hub-store / ahand-hub. This branch had accumulated 18 fmt diffs and 5 clippy errors before the round-3 review work, leaving CI red on every push. None of the issues were introduced by the round-3 fixes — they are all in modules the round-3 commits did not touch (http/jobs.rs, http/terminal.rs, ws/device_gateway.rs, output_stream.rs, support/mod.rs, etc.). This commit applies pure mechanical cleanup so hub-CI can pass: - \`cargo fmt -p ahand-protocol -p ahand-hub-core -p ahand-hub-store -p ahand-hub\` for the 18 formatting diffs (mostly use-statement ordering and closure-block indentation). - Three \`single_match\` rewrites in tests/support/mod.rs that flipped \`match { Binary(data) => ..., _ => {} }\` to \`if let Binary(data) = ...\` in recv_job_request / recv_file_request / recv_cancel_request. - Two \`collapsible_if\` rewrites: jobs.rs:263 (job-status guard) and device_gateway.rs:573 (bootstrap-reservation release on disconnect), both folding nested \`if let\` into \`if let ... && let ...\`. - One \`#[allow(clippy::too_many_arguments)]\` on JobRuntime::new (8 args / threshold 7); refactoring this constructor into a builder is out of scope for a cleanup commit. Verified: \`cargo fmt --check\` clean, \`cargo clippy -- -D warnings\` clean, and all non-Docker hub integration tests still pass (http_files / job_flow / device_gateway / system_api: 56 / 0). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * style(hub): rewrite loop+let-else-break as while-let for clippy 1.95 CI runs Rust 1.95.0, which added the \`clippy::while_let_loop\` lint. The previous cleanup commit verified locally on Rust 1.94.1, where the lint did not yet exist. The shape loop { let Some(x) = expr else { break }; ...body... } is semantically identical to \`while let Some(x) = expr { ...body... }\`, so flatten it accordingly. Affects only TestServer::collect_sse_body in tests/support/mod.rs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(hub): make sent_job_fails_after_disconnect_grace_without_reconnect robust on slow CI Pre-existing test (added in 2106912) that asserted job status was still "sent" exactly 40 ms after the device was dropped, then waited for it to flip to "failed". On slow CI runners the 40 ms snapshot caught the post-grace "failed" state and tripped the assertion — both auto-triggered and manual hub-CI runs on this branch failed identically at job_flow.rs:267. Two changes: 1. Wait for the server to confirm Pending → Sent before drop. On slow CI the local view of the job state lags the in-flight transitions performed by create_job, so dropping the device could race that transition and exercise the wrong code path (Pending ⇒ instant fail rather than Sent ⇒ grace). 2. Replace the 40-ms snapshot assertion with an elapsed-time check on the actual grace window. We measure the wall time from drop to "failed" and assert it ≥ 50 ms (tight enough to catch a regression to grace=0, loose enough to absorb scheduler jitter on a heavily loaded CI runner where 40 ms isn't reliable). Local re-run on the file-ops worktree: 11/0 (job_flow). The change is in test code only; the daemon/hub disconnect-grace plumbing is unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(daemon): thread file_mgr through run_with_reporter (merge follow-up) The previous merge commit (ef0d92b) intended to add \`file_mgr: Arc<FileManager>\` to \`run_with_reporter\`'s parameter list and to the inner \`run\` → \`run_with_reporter\` forwarding call so the manager reaches \`connect_reporting\` (which already accepts it). The edits landed in the working tree and the local build passed, but were never staged before \`git commit\`, so the merge commit shipped with the half-applied diff: \`connect_reporting\`'s body referenced \`file_mgr\` while \`run_with_reporter\` no longer defined it. CI caught it (\`cannot find value file_mgr in scope\` at the connect_reporting call site). This commit adds the two missing lines so the signature and the internal call site agree. \`cargo build --workspace\` passes and the local test suite still 0-fails. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(daemon): Step 2 spec/quality follow-ups (relative symlink dispatch, I3 line/byte coverage, cross-device fallback test) Surfaced by `winrey-toolkit:finish-feature` Step 2 (spec/quality check). Three Important findings, all real: 1. **Relative symlink dispatch path was missing lexical normalization.** `collect_request_paths` (the approval surface) normalized \`parent.join(target)\` so the dangerous-path scan saw a clean canonical string. The dispatch arm in \`FileManager::dispatch\` did not — it left raw \`..\` components in the policy-check input. Result: an operator who approved a request like \`target = "../target.txt"\` against a link inside \`/tmp/sub/\` would observe \`InvalidPath\` from \`policy.check_path\` at execution time, because the post-approval string still contained \`..\`. The approval prompt and the actual creation path disagreed on what the request meant. Fixed by reusing \`lexically_normalize\` so the dispatch policy check sees the same canonical string the prompt showed the operator. New test \`create_symlink_with_relative_target_escaping_parent_is_normalized\` pins the contract: a relative target that escapes its own parent directory but stays inside the allowlist must complete end-to-end. 2. **I3 (size-check before read) regression coverage was incomplete.** The original I3 fix added \`enforce_existing_size_limit\` to all four slurp sites — Write StringReplace / Edit StringReplace / Write LineRangeReplace / Write ByteRangeReplace — but the regression tests only covered the StringReplace pair. Added \`line_range_replace_refuses_oversized_existing_file_before_read\` and \`byte_range_replace_refuses_oversized_existing_file_before_read\` so a future refactor that drops the guard from the line/byte helpers fails the suite. 3. **\`handle_move\` cross-device fallback had no payload-level test.** \`is_cross_device_error\` is well unit-tested, but the actual copy+delete branch was only exercised through the end-to-end \`handle_move\` entry, which can't be tripped on a single-FS test environment. Extracted the payload into \`cross_device_move_fallback(req, src, dst)\` so the trigger (\`tokio::fs::rename\` returning \`CrossesDevices\`) and the payload (copy+delete with the right unlink primitive per file type) can be tested independently. Added two unit tests: one for files, one for directory trees. The wiring in \`handle_move\` is now a one-line call site that's small enough to inspect. ## Verification - \`cargo test -p ahandd --test file_ops\` — 119/0 (1 ignored, system trash) - \`cargo test -p ahandd --lib file_manager::fs_ops\` — 5/0 - \`cargo test -p ahand-hub --test http_files --test job_flow --test device_gateway --test system_api\` — 56/0 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(daemon): Step 4 test-completeness gap fills (Tier 1 + Tier 2, +31 tests) Surfaced by `winrey-toolkit:finish-feature` Step 4 (test-completeness audit). Three independent scenario auditors flagged 4 Critical + ~9 Important + a long tail of defensive/pedantic gaps. User picked Tier 1 + Tier 2 — Critical + Important — and skipped the rest. ## Tier 1 (Critical) 1. **store.rs::describe_payload exhaustive test.** All 25 Payload variants + the `None` envelope sentinel are now pinned by a table-driven test. The recently added FileRequest / FileResponse / Heartbeat arms had zero coverage; a regression that dropped one would silently emit the wrong trace label without any test failing. (Inline `#[cfg(test)] mod tests`.) 2. **Config::load end-to-end tilde wiring.** The I2 fix relies on `expand_tildes_in_place` running INSIDE `Config::load`, but only the helper itself was unit-tested. New `mod load_tilde_tests` covers: round-trip with home expansion across all three file_policy lists, post-load assertion that no `~/...` remains (proves the wiring is intact), absolute-path passthrough, malformed TOML error, missing-file error. 3. **EscalationKind::DangerousGlobMatch.** The C3 sister branch had no test. `check_request_approval_glob_escalates_when_match_is_in_dangerous_paths` pins the `FoundDangerous` outcome and asserts the escalation names the specific dangerous match (so the operator's prompt shows what tripped the gate). 4. **handle_file_request glue test (mock_hub extension).** Extended `tests/mock_hub/mod.rs` with `Behavior::SendFileRequest(_)` that accepts the handshake, injects a FileRequest envelope, and captures every FileResponse coming back. New test `daemon_responds_to_file_request_through_ahand_client_glue` proves the entire WS round-trip works: envelope decode in the read loop → `handle_file_request` dispatch → `FileManager::handle` → `BufferedEnvelopeSender` → wire → mock capture. Uses the disabled-FileManager early-return (PolicyDenied) which is exactly the contract `public_api::spawn` produces — testing the wiring without needing public-API enabled-policy support, which isn't wired yet. ## Tier 2 (Important) 14 bad-case tests in `tests/file_ops.rs`: - `read_text_on_directory_returns_is_a_directory` - `read_text_exceeding_max_read_bytes_returns_too_large` - `read_binary_on_missing_file_returns_not_found` - `read_binary_on_directory_returns_is_a_directory` - `read_image_on_directory_returns_is_a_directory` - `glob_with_invalid_pattern_returns_invalid_path` (unclosed character class — reaches `glob::glob()`, distinct from the dispatcher's absolute / `..` rejections) - `copy_directory_without_recursive_returns_is_a_directory` - `move_to_existing_destination_without_overwrite_returns_already_exists` - `write_with_no_method_returns_unspecified_error` - `edit_with_no_method_returns_unspecified_error` - `full_write_with_no_source_returns_unspecified_error` - `full_write_with_s3_object_key_source_returns_unspecified_error` (so the deferred S3 path fails loud rather than silently) - `append_exceeding_total_size_limit_returns_too_large` (existing file at limit + non-empty append must surface as TooLarge before any write) - `chmod_with_no_permission_returns_unspecified_error` - `chmod_with_unix_permission_but_no_mode_or_owner_returns_unspecified_error` ## Skipped Defensive (resolve/cancel idempotency on missing keys, S3 upload_bytes / download_bytes) and pedantic (lexically_normalize empty-input, WebP output, etc.) — recorded in audit report; follow-up if needed. ## Verification - `cargo test -p ahandd`: 345 / 0 (1 ignored — system trash test) → up from 314 by +31 tests - `cargo test -p ahand-hub --test http_files --test job_flow --test device_gateway --test system_api`: 56 / 0 - `cargo build --workspace`: clean Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(daemon,hub): Copilot review — populate FileError.path + case-insensitive content-type + saturating append size Five Copilot comments on PR #1, all genuinely valid: 1. **text_read.rs `resolve_encoding`** — when an explicit but unknown encoding…
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
AsyncRead/AsyncWriterefactor + Named Pipe server on Windows. Peer identity viaGetNamedPipeClientProcessId→LookupAccountSidW. Rejects connections with unverifiable identity.LocalFree). Cross-platform ACL file permissions (SetNamedSecurityInfoW/chmod).tasklistPID check,taskkill,CREATE_NO_WINDOW | DETACHED_PROCESS.tokio::signal::ctrl_c()on Windows, SIGTERM/SIGINT on Unix.upgrade.rs/install_daemon.rsfull Rust rewrite with SHA-256 checksum verification (hard-fail on missing/mismatch).install.ps1PowerShell installer with enforced checksum + PATH setup..zipon Windows with zip-slipstarts_withdefense-in-depth. Chrome/Edge detection.Swatinem/rust-cache@v2.Review Summary
Spec
docs/superpowers/specs/2026-04-05-windows-support-design.mdTest plan
cargo test -p ahandd -p ahandctlpasses on macOS/Linuxx86_64-pc-windows-msvc)install.ps1installs correctly on Windows🤖 Generated with Claude Code