From e3408d004a9dc268f80d6fb974bc2b13f2bb6952 Mon Sep 17 00:00:00 2001 From: nitrobass24 Date: Tue, 19 May 2026 11:18:20 -0500 Subject: [PATCH 1/5] feat(agent): add remote helper design doc and NDJSON proto types Condensed design doc (428 lines) replacing the 3,500-line whitepaper from PR #1820. Covers architecture, Backend interface, wire protocol, path safety, SSH credentials, schema, helper binary, and phased implementation plan (8 PRs). pkg/agent/proto is a leaf package (zero internal/ imports) defining the shared NDJSON wire types for qui <-> qui-helper communication: Command/Result envelopes, HelloBanner, and all op request/response payloads. --- documentation/design/remote-helper.md | 428 ++++++++++++++++++++++++++ pkg/agent/proto/ops.go | 181 +++++++++++ pkg/agent/proto/proto.go | 85 +++++ pkg/agent/proto/proto_test.go | 389 +++++++++++++++++++++++ 4 files changed, 1083 insertions(+) create mode 100644 documentation/design/remote-helper.md create mode 100644 pkg/agent/proto/ops.go create mode 100644 pkg/agent/proto/proto.go create mode 100644 pkg/agent/proto/proto_test.go diff --git a/documentation/design/remote-helper.md b/documentation/design/remote-helper.md new file mode 100644 index 000000000..3afc73f4a --- /dev/null +++ b/documentation/design/remote-helper.md @@ -0,0 +1,428 @@ +# Remote Helper Design + +qui-helper is a small static Go binary that qui pushes to a remote seedbox via SCP and drives over SSH. It enables all filesystem-dependent features (cross-seed inject, dirscan, orphanscan, automations, managed delete cleanup) to work against remote qBittorrent instances. + +No inbound port on the seedbox. No pairing tokens. SSH credentials are the only secret, encrypted at rest the same way qBittorrent passwords already are. + +## Architecture + +``` +qui host seedbox ++--------------------------+ +---------------------------+ +| services/dirscan | | qBittorrent | +| services/orphanscan | +---------------------------+ +| services/automations | | local FS +| services/crossseed | v ++-----------+--------------+ +---------------------------+ + | | ~/data, ~/seed, ... | + v +---------------------------+ ++--------------------------+ ^ +| internal/fsops | | os.* via +| +---------+ +---------+ | | os.Root (BENEATH) +| | Local | | Remote | | | +| +---------+ +----+----+ | +---------------------------+ ++-------------------+------+ | qui-helper serve --stdio | + | | (one persistent process) | + v | pkg/fsexec primitives | ++--------------------------+ +---------------------------+ +| internal/sshpool | ^ +| *ssh.Client per instance +--- SSH stdio ------+ NDJSON stdin/stdout ++--------------------------+ +``` + +**Lifecycle:** Lazy connect on first FS op per instance. One persistent `*ssh.Client` + `ssh.Session` runs `qui-helper serve --stdio --root --root `. Helper emits a `HelloBanner` (version, capabilities, reflink-supported roots). Commands/results flow as NDJSON on stdin/stdout. SSH keepalive (`ServerAliveInterval=15s`) prevents idle drops. Shutdown: close stdin, helper drains in-flight ops (30s grace), exits. + +## Filesystem Operations + +This is the complete RPC surface. Anything outside this table stays local to the qui host. + +| Feature | Ops used | Volume | Code locations | +|---|---|---|---| +| Cross-seed hardlink inject | MkdirAll, Lstat, Link, Remove, SameFilesystem | 1-100/match | dirscan/inject.go, pkg/hardlinktree | +| Cross-seed reflink inject | MkdirAll, Lstat, CoW clone, Remove | 1-100/match | dirscan/inject.go, pkg/reflinktree | +| Link-tree teardown | Remove (per file) | tens-100 | hardlinktree/reflinktree Rollback | +| Dirscan walk + FileID | WalkDir, Lstat, FileID | 10k+ files | dirscan/scanner.go, dirscan/fileid_index.go | +| Orphanscan walk | WalkDir, Lstat, FileID | 10k+ files | orphanscan/walker.go | +| Orphanscan delete | Lstat, Remove, RemoveAll | tens/run | orphanscan/delete.go | +| Missing-files condition | Stat (batch) | hundreds/cycle | automations/missing_files.go | +| Free-space condition | Statfs | 1/eval | automations/free_space.go | +| Hardlink-scope condition | Lstat + FileID (batch) | 10k+ files | automations/hardlink_index.go | +| Managed delete cleanup | Stat, Remove | tens/delete | qbittorrent/delete_cleanup.go | +| Same-filesystem check | Stat x2 + dev-id compare | 1/inject | pkg/fsutil/samefs.go | + +## Backend Interface + +`internal/fsops.Backend` abstracts all filesystem operations. Services depend on this interface, not `os.*` directly. + +```go +type Backend interface { + // Read + Stat(ctx context.Context, path string) (*FileInfo, error) + StatBatch(ctx context.Context, paths []string) ([]*FileInfo, []error, error) + Lstat(ctx context.Context, path string) (*LstatInfo, error) + LstatBatch(ctx context.Context, paths []string) ([]*LstatInfo, []error, error) + ReadDir(ctx context.Context, path string, maxEntries int) ([]DirEntry, bool, error) + WalkDir(ctx context.Context, root string, opts WalkOptions) (<-chan WalkEntry, error) + Statfs(ctx context.Context, path string) (*StatfsResult, error) + SameFilesystem(ctx context.Context, p1, p2 string) (bool, error) + FileID(ctx context.Context, path string) (hardlink.FileID, uint64, error) + + // Write + MkdirAll(ctx context.Context, path string, perm fs.FileMode) error + Remove(ctx context.Context, path string, opts RemoveOptions) error + + // Atomic tree ops + HardlinkTree(ctx context.Context, plan *hardlinktree.TreePlan) (*TreeCreateResult, error) + ReflinkTree(ctx context.Context, plan *hardlinktree.TreePlan) (*TreeCreateResult, error) + RemoveTree(ctx context.Context, plan *hardlinktree.TreePlan) error + + // Capabilities + SupportsReflink(ctx context.Context, path string) (bool, string, error) + + // Diagnostic + Info(ctx context.Context) (*BackendInfo, error) + HealthCheck(ctx context.Context) error +} +``` + +**Implementations:** +- `internal/fsops/local` -- thin adapter over `os.*`, pkg/hardlinktree, pkg/reflinktree, pkg/hardlink, pkg/fsutil +- `internal/fsops/remote` -- translates Backend calls into NDJSON commands dispatched via sshpool +- `internal/fsops.Pool` -- resolves instance ID to the correct Backend (local, remote, or noop) + +## Wire Protocol + +NDJSON over SSH stdio. Three streams on one `ssh.Session`: + +| Stream | Direction | Format | Purpose | +|---|---|---|---| +| stdin | qui -> helper | NDJSON Commands | Op dispatches, cancellations | +| stdout | helper -> qui | NDJSON Results | Op results, walk-stream frames | +| stderr | helper -> qui | structured JSON | Helper logs forwarded to qui | + +### Envelopes + +```go +type Command struct { + RequestID string `json:"requestID"` + Op string `json:"op"` + Args json.RawMessage `json:"args"` + Deadline string `json:"deadline,omitempty"` // RFC3339 +} + +type Result struct { + RequestID string `json:"requestID"` + OK bool `json:"ok"` + Code string `json:"code,omitempty"` + Error string `json:"error,omitempty"` + Payload json.RawMessage `json:"payload,omitempty"` + Done bool `json:"done,omitempty"` // last frame for streamed ops + Frame int `json:"frame,omitempty"` // monotonic counter (streamed only) +} + +type HelloBanner struct { + HelperVersion string `json:"helperVersion"` + ProtoVersion string `json:"protoVersion"` // "1" + Capabilities []string `json:"capabilities"` + AllowedRoots []string `json:"allowedRoots"` + ReflinkRoots []string `json:"reflinkRoots"` + Platform string `json:"platform"` + Hostname string `json:"hostname"` + PID int `json:"pid"` + StartedAt string `json:"startedAt"` // RFC3339 +} +``` + +Non-streaming ops: one Result with `Done: true`. Streaming ops (`fs.walk`): multiple Results with same requestID, final frame has `Done: true`. + +### Op Payloads + +All types live in `pkg/agent/proto`. Paths are absolute; helper rejects any not under an allowed root. + +```go +// fs.stat / fs.lstat +type StatRequest struct { Paths []string `json:"paths"` } +type StatEntry struct { Path string; Exists bool; Size int64; ModTime string; IsDir bool; Mode uint32; Err string } +type LstatRequest struct { Paths []string; WantFileID bool; WantNlinks bool } +type LstatEntry struct { StatEntry; IsSymlink bool; FileID []byte; Nlinks uint64 } + +// fs.walk (streaming) +type WalkRequest struct { Root string; SkipHidden bool; IgnoreDirNames []string; IgnorePaths []string; WantFileID bool; WantNlinks bool; MaxEntries int } +type WalkEntry struct { Path string; RelPath string; IsDir bool; IsSymlink bool; Size int64; ModTime string; Mode uint32; FileID []byte; Nlinks uint64; Err string; Truncated bool } + +// fs.statfs +type StatfsRequest struct { Path string } +type StatfsResponse struct { BytesAvailable int64; BytesTotal int64; Filesystem string } + +// fs.readdir +type ReadDirRequest struct { Path string; MaxEntries int } +type ReadDirResponse struct { Entries []DirEntry; Truncated bool } + +// fs.samefs +type SameFSRequest struct { Path1, Path2 string } +type SameFSResponse struct { Same bool } + +// fs.mkdir +type MkdirRequest struct { Path string; Perm uint32 } + +// fs.remove +type RemoveRequest struct { Path string; Recursive bool; IgnorePaths []string } +type RemoveResponse struct { Removed bool; Disposition string; RemovedBytes int64 } + +// tree.hardlink / tree.reflink +type TreeCreateRequest struct { Plan hardlinktree.TreePlan; Mode string } +type TreeCreateResponse struct { Created int; SkippedExists int; RolledBack bool; Err string } +type TreeRemoveRequest struct { Plan hardlinktree.TreePlan } + +// control.cancel +type CancelRequest struct { RequestIDs []string } +``` + +### Error Codes + +Stream errors (crash, disconnect): all in-flight ops fail with `connection_lost`, lazy reconnect on next call. + +Op error codes: `path_not_allowed`, `path_not_found`, `permission_denied`, `cross_device`, `tree_partial_rollback`, `version_skew`, `request_too_large`, `internal`, `connection_lost`, `cancelled`, `deadline_exceeded`. + +### Cancellation + +qui sends `control.cancel` on stdin with the requestIDs to abort. Helper cancels the matching contexts. Ops exit cooperatively via `ctx.Done()` checks between syscalls. Tree ops enter rollback path on cancel. + +### Per-Op Bounds + +| Field | Cap | Rationale | +|---|---|---| +| StatRequest.Paths / LstatRequest.Paths | 1024 | Per-torrent file lists | +| WalkRequest.IgnorePaths | 1024 | Orphanscan ignore list | +| TreeCreateRequest.Plan.Files | 10,000 | Single cross-seed match | +| Any single path | 4,096 bytes | Linux PATH_MAX | +| Per-command line length | 16 MiB | Stdio buffer cap | + +If a batch exceeds a cap, `fsops.Remote` chunks automatically and merges results. + +## Path Safety + +All enforcement is helper-side via `pkg/fsexec`: + +- All paths must be absolute, clean, and under an allowed root (passed as `--root` flags at startup) +- Linux: `os.Root` (Go 1.24+) wraps allowed-root directories, uses `openat2(RESOLVE_BENEATH)` under the hood +- macOS/Windows: `os.Root` with userspace-resolution fallback + symlink rejection +- Destructive ops refuse to operate on the allowed-root itself (must be a strict descendant) +- `..` rejected at command validation time +- Device-ID guard: startup records `dev` of each allowed root, rejects ops whose resolved path is on a different device (catches unmounted roots) +- TOCTOU contract: `ResolveSafe()` returns an `*os.Root` handle; all subsequent ops use that same handle via `*at` syscalls + +**Audit log:** `~/.local/state/qui-helper/audit.log` records every destructive op as JSON lines: `{ts, op, path, request_id, qui_session_id, outcome, error}`. In-process size-based rotation (50 MB, 5 archives). + +## SSH Credentials & Deployment + +### Credential Model + +Per instance, the user provides: +- **Host** (defaults port 22) +- **Username** +- **Auth**: private key (OpenSSH format, optional passphrase) or password + +All credential material encrypted at rest with `sessionSecret` (AES-GCM, same pattern as qBittorrent passwords). Host key captured on first connect (TOFU), verified on every subsequent connect. + +### Deploy Flow + +1. qui opens SSH connection +2. Detects remote arch: `uname -m && uname -s` -> maps to cross-compiled binary +3. Downloads matching binary from GitHub releases to qui host, verifies SHA256 against embedded constant +4. SCPs to `~/.config/qui-helper/qui-helper` (mode 0700, atomic via temp + rename) +5. Runs `qui-helper version --json` to confirm deployment +6. Caches version/capabilities/reflink-roots on instance record + +Auto-redeploy on version mismatch at connect time. + +### Hardening + +Optional `authorized_keys` restriction to lock SSH key to helper-only access: +``` +command="/home/user/.config/qui-helper/qui-helper serve --stdio --root /home/user/data --root /home/user/seed",no-port-forwarding,no-X11-forwarding,no-agent-forwarding ssh-ed25519 AAAA... +``` + +## Schema + +Next available migration numbers: 077 (sqlite) / 078 (postgres). + +New columns on `instances`: + +```sql +ALTER TABLE instances ADD COLUMN ssh_host TEXT NOT NULL DEFAULT ''; +ALTER TABLE instances ADD COLUMN ssh_port INTEGER NOT NULL DEFAULT 22; +ALTER TABLE instances ADD COLUMN ssh_username TEXT NOT NULL DEFAULT ''; +ALTER TABLE instances ADD COLUMN ssh_auth_type TEXT NOT NULL DEFAULT '' CHECK (ssh_auth_type IN ('', 'key', 'password')); +ALTER TABLE instances ADD COLUMN ssh_key_encrypted TEXT NOT NULL DEFAULT ''; +ALTER TABLE instances ADD COLUMN ssh_key_passphrase_encrypted TEXT NOT NULL DEFAULT ''; +ALTER TABLE instances ADD COLUMN ssh_password_encrypted TEXT NOT NULL DEFAULT ''; +ALTER TABLE instances ADD COLUMN ssh_host_key TEXT NOT NULL DEFAULT ''; +ALTER TABLE instances ADD COLUMN helper_path TEXT NOT NULL DEFAULT '~/.config/qui-helper/qui-helper'; +ALTER TABLE instances ADD COLUMN helper_version TEXT NOT NULL DEFAULT ''; +ALTER TABLE instances ADD COLUMN helper_capabilities TEXT NOT NULL DEFAULT '[]'; +ALTER TABLE instances ADD COLUMN helper_allowed_roots TEXT NOT NULL DEFAULT '[]'; +ALTER TABLE instances ADD COLUMN helper_reflink_roots TEXT NOT NULL DEFAULT '[]'; +ALTER TABLE instances ADD COLUMN helper_platform TEXT NOT NULL DEFAULT ''; +ALTER TABLE instances ADD COLUMN helper_deployed_at DATETIME; +ALTER TABLE instances ADD COLUMN helper_last_activity_at DATETIME; +``` + +`HasFilesystemAccess(instance) -> (FilesystemMode, bool)` returns true if `has_local_filesystem_access || (ssh_host != '' && helper_deployed_at IS NOT NULL)`. + +## Helper Binary + +**Location:** `cmd/qui-helper/main.go`. Zero `internal/` imports (CI-enforced). + +**Import graph:** `pkg/agent/proto`, `pkg/fsexec`, `pkg/hardlinktree`, `pkg/reflinktree`, `pkg/hardlink`, `pkg/fsutil`. Single-digit-MB binary. + +**Subcommands:** +- `serve --stdio --root /path1 --root /path2` -- long-running mode, NDJSON loop +- `version --json` -- prints HelloBanner and exits +- `version` -- human-readable output + +**Cross-compile:** linux/{amd64,arm64}, darwin/{amd64,arm64}. Windows/amd64 best-effort. + +**No config file.** Everything via flags. Stateless. Cleanup: `rm -rf ~/.config/qui-helper/ ~/.local/state/qui-helper/`. + +## SSH Pool + +`internal/sshpool` manages per-instance SSH connections. + +Key behaviors: +- Lazy connect on first FS op +- Concurrent ops multiplexed via NDJSON requestIDs on single stdio channel +- Pending results map with 5-min TTL +- Reconnect backoff: exponential 5s -> 60s with +/-20% jitter +- SSH auth failure: 60s pause, 3 sequential failures -> "credentials invalid" state +- Sweeper goroutine: connection health (30s), pending TTL (30s), reconnect scheduler (5s) +- `max_inflight_per_instance = 32` + +## API Endpoints + +``` +POST /api/instances/{id}/ssh-test -- test SSH credentials, return host key fingerprint +POST /api/instances/{id}/helper/deploy -- push binary, return parsed HelloBanner +POST /api/instances/{id}/helper/redeploy -- re-push binary (upgrade) +DELETE /api/instances/{id}/helper -- disconnect + remove binary + clear helper fields +DELETE /api/instances/{id}/ssh-credentials -- clear SSH credential fields +GET /api/instances/{id}/helper -- return helper status (version, capabilities, etc.) +``` + +## Frontend + +Instance form replaces `hasLocalFilesystemAccess` toggle with a RadioGroup: +- None / Local / Remote helper + +"Remote helper" reveals SSH credential fields, test connection button (TOFU host key confirmation), deploy button with progress UI, and helper status card. + +## Implementation Plan + +### PR 1: Design doc + proto types + +- `documentation/design/remote-helper.md` (this document) +- `pkg/agent/proto/proto.go` -- Command, Result, HelloBanner envelopes +- `pkg/agent/proto/ops.go` -- all op request/response types +- `pkg/agent/proto/proto_test.go` -- JSON round-trip tests + +Leaf package, zero `internal/` imports. + +### PR 2: Backend interface + local backend + pool + +- `internal/fsops/backend.go` -- Backend interface (17 methods) +- `internal/fsops/types.go` -- value types (FileInfo, LstatInfo, WalkEntry, etc.) +- `internal/fsops/errors.go` -- sentinel errors +- `internal/fsops/noop.go` -- noop backend for instances without FS access +- `internal/fsops/pool.go` -- Pool resolver (instance ID -> Backend) +- `internal/fsops/local/` -- local backend implementation + platform-specific Statfs +- Tests for all of the above + +No callsite changes yet. Services still use `os.*` directly. + +### PR 3: Service callsite refactors + +Migrate all `os.*`/`unix.*` callsites to `backend.*`. Zero behavioral change. Could split by service area if too large: + +- `automations/missing_files.go` -- `os.Stat` -> `backend.Stat` +- `automations/free_space.go` -- `unix.Statfs` -> `backend.Statfs` +- `automations/hardlink_index.go` -- `os.Lstat` + FileID -> `backend.Lstat` +- `qbittorrent/delete_cleanup.go` -- `os.Stat`/`os.Remove` -> `backend.Stat`/`backend.Remove` +- `dirscan/fileid_index.go` -- FileID index via backend +- `dirscan/inject.go` + `crossseed/FindMatchingBaseDir` -- link-tree ops via backend +- `dirscan/scanner.go` -- `filepath.WalkDir` callback -> `backend.WalkDir` channel +- `orphanscan/walker.go` + `delete.go` -- full walker + delete via backend + +All existing tests must pass with zero behavioral diff. + +### PR 4: Schema + models + +- Migration 077/078: 16 SSH/helper columns on `instances` +- `internal/models/instance.go` -- extend Instance struct with SSH/helper fields +- `internal/models/filesystem_access.go` -- `HasFilesystemAccess` helper +- `internal/models/filesystem_access_test.go` +- Update InstanceStore Create/Update to handle new fields + +### PR 5: Path safety + SSH pool + helper binary + +- `pkg/fsexec/` -- SafeRoot, Roots, ResolveSafe with os.Root wrapping, property tests +- `internal/sshpool/` -- pool, transport (TOFU), deploy, sweeper +- `cmd/qui-helper/` -- main.go, server (NDJSON loop, diag.echo), executor stub +- `Makefile` -- `make helper` target +- CI workflow for helper (import-graph guard, cross-compile) + +### PR 6: API endpoints + wiring + +- `internal/api/handlers/helper.go` -- 6 endpoints +- `internal/api/server.go` -- route registration +- Wire sshpool into Dependencies struct +- Wire fsops.Pool into services via `cmd/qui/main.go` + +### PR 7: First real op (fs.statfs) end-to-end + +- `internal/fsops/remote/remote.go` -- Remote backend backed by SSH pool +- Wire `sshpool.Pool.Submit`/`Cancel` with real SSH session dispatch +- Implement `fs.statfs` in helper executor +- Pool resolver returns Remote backend for helper-configured instances +- Dual-backend integration test + +### PR 8+: Remaining ops (one PR each) + +Read ops first, destructive ops later: + +1. `fs.stat` -- missing-files condition +2. `fs.lstat` -- hardlink index, FileID index +3. `fs.samefs` -- hardlink/reflink precondition +4. `fs.readdir` -- disc-layout detection, orphanscan +5. `fs.walk` (streaming) -- dirscan, orphanscan +6. `fs.mkdir` -- link-tree materialization +7. `fs.remove` -- managed delete cleanup, orphanscan +8. `fs.removeall` -- orphanscan directory deletion +9. `tree.hardlink` -- cross-seed inject +10. `tree.reflink` -- cross-seed inject (capability-gated) +11. `tree.remove` -- link-tree rollback + +Each PR adds: helper executor case + Remote backend method + dual-backend integration test. + +### Frontend PR (parallel with PR 7+) + +- Instance form RadioGroup (None / Local / Remote helper) +- SSH credential fields + host key confirmation modal +- Deploy button + progress UI +- Helper status card +- `authorized_keys` hardening snippet with copy button + +## Salvageable Code from PR #1820 + +The following packages from the original `feat/fsops-backend-scaffold` branch are production-quality and can be cherry-picked onto fresh branches with minimal changes: + +| Package | Verdict | Tests | +|---|---|---| +| `pkg/agent/proto` | Use as-is | 38 tests | +| `internal/fsops` (interface, pool, noop, types, errors) | Use as-is | 4 tests | +| `internal/fsops/local` | Use as-is | 35+ tests | +| `pkg/fsexec` | Use as-is | 17 tests | +| `cmd/qui-helper` | Use as-is | 5 tests | +| `internal/models/filesystem_access` | Use as-is | 6 tests | +| `internal/sshpool` | Needs test coverage for deploy.go and sweeper.go | 12 tests | + +All packages have zero coupling to the unrelated changes in PR #1820 and will apply cleanly on develop. diff --git a/pkg/agent/proto/ops.go b/pkg/agent/proto/ops.go new file mode 100644 index 000000000..42061671a --- /dev/null +++ b/pkg/agent/proto/ops.go @@ -0,0 +1,181 @@ +// Copyright (c) 2025-2026, s0up and the autobrr contributors. +// SPDX-License-Identifier: GPL-2.0-or-later + +package proto + +import "github.com/autobrr/qui/pkg/hardlinktree" + +// All paths in requests are absolute. The helper rejects any path not under an allowed root. +// Command.RequestID is the canonical idempotency token — it is not duplicated inside payloads. + +// --- fs.stat --- + +type StatRequest struct { + Paths []string `json:"paths"` +} + +type StatEntry struct { + Path string `json:"path"` + Exists bool `json:"exists"` + Size int64 `json:"size,omitempty"` + ModTime string `json:"modTime,omitempty"` // RFC 3339 Nano + IsDir bool `json:"isDir,omitempty"` + Mode uint32 `json:"mode,omitempty"` + Err string `json:"err,omitempty"` // "not_found", "permission", etc. +} + +type StatResponse struct { + Entries []StatEntry `json:"entries"` +} + +// --- fs.lstat --- + +type LstatRequest struct { + Paths []string `json:"paths"` + WantFileID bool `json:"wantFileID,omitempty"` + WantNlinks bool `json:"wantNlinks,omitempty"` +} + +type LstatEntry struct { + StatEntry + IsSymlink bool `json:"isSymlink,omitempty"` + FileID []byte `json:"fileID,omitempty"` // hardlink.FileID.Bytes() + Nlinks uint64 `json:"nlinks,omitempty"` +} + +type LstatResponse struct { + Entries []LstatEntry `json:"entries"` +} + +// --- fs.walk (streaming) --- + +type WalkRequest struct { + Root string `json:"root"` + SkipHidden bool `json:"skipHidden,omitempty"` + IgnoreDirNames []string `json:"ignoreDirNames,omitempty"` + IgnorePaths []string `json:"ignorePaths,omitempty"` + WantFileID bool `json:"wantFileID,omitempty"` + WantNlinks bool `json:"wantNlinks,omitempty"` + MaxEntries int `json:"maxEntries,omitempty"` // 0 = unlimited +} + +// WalkEntry is sent as one Result.Payload frame per entry. +// The final frame carries Done:true on the Result envelope. +type WalkEntry struct { + Path string `json:"path,omitempty"` + RelPath string `json:"relPath,omitempty"` + IsDir bool `json:"isDir,omitempty"` + IsSymlink bool `json:"isSymlink,omitempty"` + Size int64 `json:"size,omitempty"` + ModTime string `json:"modTime,omitempty"` + Mode uint32 `json:"mode,omitempty"` + FileID []byte `json:"fileID,omitempty"` + Nlinks uint64 `json:"nlinks,omitempty"` + Err string `json:"err,omitempty"` + Truncated bool `json:"truncated,omitempty"` // set on last frame if MaxEntries was hit +} + +// --- fs.statfs --- + +type StatfsRequest struct { + Path string `json:"path"` +} + +type StatfsResponse struct { + BytesAvailable int64 `json:"bytesAvailable"` + BytesTotal int64 `json:"bytesTotal"` + Filesystem string `json:"filesystem,omitempty"` // best-effort +} + +// --- fs.readdir --- + +type ReadDirRequest struct { + Path string `json:"path"` + MaxEntries int `json:"maxEntries,omitempty"` // 0 = no cap (subject to per-op cap) +} + +type DirEntry struct { + Name string `json:"name"` + IsDir bool `json:"isDir,omitempty"` + IsSymlink bool `json:"isSymlink,omitempty"` + Size int64 `json:"size,omitempty"` + ModTime string `json:"modTime,omitempty"` + Mode uint32 `json:"mode,omitempty"` +} + +type ReadDirResponse struct { + Entries []DirEntry `json:"entries"` + Truncated bool `json:"truncated,omitempty"` +} + +// --- fs.samefs --- + +type SameFSRequest struct { + Path1 string `json:"path1"` + Path2 string `json:"path2"` +} + +type SameFSResponse struct { + Same bool `json:"same"` +} + +// --- fs.mkdir --- + +type MkdirRequest struct { + Path string `json:"path"` + Perm uint32 `json:"perm"` +} + +// --- fs.remove / fs.removeall --- + +type RemoveRequest struct { + Path string `json:"path"` + Recursive bool `json:"recursive,omitempty"` + IgnorePaths []string `json:"ignorePaths,omitempty"` // server-side ignore list (orphanscan) +} + +type RemoveResponse struct { + Removed bool `json:"removed"` + Disposition string `json:"disposition,omitempty"` // "deleted" | "skipped_missing" | "skipped_ignored" + RemovedBytes int64 `json:"removedBytes,omitempty"` +} + +// --- tree.hardlink / tree.reflink --- + +// TreeCreateRequest carries the entire TreePlan. The helper executes it atomically +// and rolls back on partial failure. +type TreeCreateRequest struct { + Plan hardlinktree.TreePlan `json:"plan"` + Mode string `json:"mode"` // "hardlink" or "reflink" + SourceFS string `json:"sourceFS,omitempty"` // hint for pre-flight checks +} + +type TreeCreateResponse struct { + Created int `json:"created"` + SkippedExists int `json:"skippedExists"` + RolledBack bool `json:"rolledBack"` + Err string `json:"err,omitempty"` + DiagFiles []string `json:"diagFiles,omitempty"` // truncated debug, opt-in +} + +// --- tree.remove --- + +type TreeRemoveRequest struct { + Plan hardlinktree.TreePlan `json:"plan"` +} + +// --- control.cancel --- + +type CancelRequest struct { + RequestIDs []string `json:"requestIDs"` // ops to abort +} + +// --- diag.echo --- + +type DiagEchoRequest struct { + Message string `json:"message"` +} + +type DiagEchoResponse struct { + Message string `json:"message"` +} diff --git a/pkg/agent/proto/proto.go b/pkg/agent/proto/proto.go new file mode 100644 index 000000000..cb674dbb7 --- /dev/null +++ b/pkg/agent/proto/proto.go @@ -0,0 +1,85 @@ +// Copyright (c) 2025-2026, s0up and the autobrr contributors. +// SPDX-License-Identifier: GPL-2.0-or-later + +// Package proto defines the NDJSON wire types shared between qui and qui-helper. +// Every line on the helper's stdin is a Command; every line on stdout is a Result +// (or a HelloBanner on the very first line). Types here are pure data — no +// business logic, no internal/ imports. +package proto + +import "encoding/json" + +// Command is written by qui to the helper's stdin (one NDJSON line per command). +type Command struct { + RequestID string `json:"requestID"` // UUID, qui-generated + Op string `json:"op"` // "fs.stat", "tree.hardlink", "control.cancel", … + Args json.RawMessage `json:"args"` // op-specific payload + Deadline string `json:"deadline,omitempty"` // RFC 3339; helper aborts if exceeded +} + +// Result is written by the helper to stdout (one NDJSON line per result frame). +type Result struct { + RequestID string `json:"requestID"` + OK bool `json:"ok"` + Code string `json:"code,omitempty"` // stable error code + Error string `json:"error,omitempty"` // human-readable error detail + Payload json.RawMessage `json:"payload,omitempty"` // op-specific response + Done bool `json:"done,omitempty"` // true on last frame (always true for non-streaming ops) + Frame int `json:"frame,omitempty"` // monotonic per-requestID counter (streaming ops only) +} + +// HelloBanner is the first line the helper writes to stdout on session startup. +// qui parses it before sending any commands. +type HelloBanner struct { + HelperVersion string `json:"helperVersion"` + ProtoVersion string `json:"protoVersion"` // "1" + Capabilities []string `json:"capabilities"` + AllowedRoots []string `json:"allowedRoots"` + ReflinkRoots []string `json:"reflinkRoots"` // subset of AllowedRoots whose FS supports CoW reflinks + Platform string `json:"platform"` // "linux", "darwin", "windows" + Hostname string `json:"hostname"` + PID int `json:"pid"` + StartedAt string `json:"startedAt"` // RFC 3339 +} + +// Op constants. Whether an op streams is determined solely by Op via IsStreamingOp. +const ( + OpStat = "fs.stat" + OpLstat = "fs.lstat" + OpReadDir = "fs.readdir" + OpWalk = "fs.walk" + OpStatfs = "fs.statfs" + OpSameFS = "fs.samefs" + OpMkdir = "fs.mkdir" + OpRemove = "fs.remove" + OpRemoveAll = "fs.removeall" + + OpTreeHardlink = "tree.hardlink" + OpTreeReflink = "tree.reflink" + OpTreeRemove = "tree.remove" + + OpControlCancel = "control.cancel" + OpControlShutdown = "control.shutdown" + + OpDiagEcho = "diag.echo" +) + +// IsStreamingOp returns true if the op produces multiple Result frames before Done. +func IsStreamingOp(op string) bool { + return op == OpWalk +} + +// Stable error codes returned in Result.Code. +const ( + CodePathNotAllowed = "path_not_allowed" + CodePathNotFound = "path_not_found" + CodePermissionDenied = "permission_denied" + CodeCrossDevice = "cross_device" + CodeTreePartialRollback = "tree_partial_rollback" + CodeVersionSkew = "version_skew" + CodeRequestTooLarge = "request_too_large" + CodeInternal = "internal" + CodeConnectionLost = "connection_lost" + CodeCancelled = "cancelled" + CodeDeadlineExceeded = "deadline_exceeded" +) diff --git a/pkg/agent/proto/proto_test.go b/pkg/agent/proto/proto_test.go new file mode 100644 index 000000000..1e6e5d88b --- /dev/null +++ b/pkg/agent/proto/proto_test.go @@ -0,0 +1,389 @@ +// Copyright (c) 2025-2026, s0up and the autobrr contributors. +// SPDX-License-Identifier: GPL-2.0-or-later + +package proto + +import ( + "encoding/json" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/autobrr/qui/pkg/hardlinktree" +) + +// roundTrip marshals v to JSON and unmarshals it into a new value of the same type. +// Fails the test on any error and returns the round-tripped value. +func roundTrip[T any](t *testing.T, v T) T { + t.Helper() + data, err := json.Marshal(v) + require.NoError(t, err, "marshal") + var out T + require.NoError(t, json.Unmarshal(data, &out), "unmarshal") + return out +} + +func TestCommand_RoundTrip(t *testing.T) { + args, _ := json.Marshal(StatRequest{Paths: []string{"/data/file.mkv"}}) + cmd := Command{ + RequestID: "abc-123", + Op: OpStat, + Args: args, + Deadline: "2026-04-28T12:00:00Z", + } + got := roundTrip(t, cmd) + assert.Equal(t, cmd.RequestID, got.RequestID) + assert.Equal(t, cmd.Op, got.Op) + assert.JSONEq(t, string(cmd.Args), string(got.Args)) + assert.Equal(t, cmd.Deadline, got.Deadline) +} + +func TestCommand_OmitsEmptyDeadline(t *testing.T) { + cmd := Command{RequestID: "x", Op: OpStat, Args: json.RawMessage(`{}`)} + data, err := json.Marshal(cmd) + require.NoError(t, err) + assert.NotContains(t, string(data), "deadline") +} + +func TestResult_RoundTrip(t *testing.T) { + payload, _ := json.Marshal(StatResponse{Entries: []StatEntry{{Path: "/a", Exists: true}}}) + r := Result{ + RequestID: "abc-123", + OK: true, + Payload: payload, + Done: true, + } + got := roundTrip(t, r) + assert.Equal(t, r.RequestID, got.RequestID) + assert.True(t, got.OK) + assert.True(t, got.Done) + assert.JSONEq(t, string(r.Payload), string(got.Payload)) +} + +func TestResult_ErrorFields(t *testing.T) { + r := Result{ + RequestID: "err-1", + OK: false, + Code: CodePathNotAllowed, + Error: "path /etc is not under any allowed root", + } + got := roundTrip(t, r) + assert.False(t, got.OK) + assert.Equal(t, CodePathNotAllowed, got.Code) + assert.Equal(t, r.Error, got.Error) +} + +func TestResult_StreamingFrame(t *testing.T) { + r := Result{ + RequestID: "walk-1", + OK: true, + Payload: json.RawMessage(`{"path":"/data/file.mkv"}`), + Frame: 42, + } + got := roundTrip(t, r) + assert.Equal(t, 42, got.Frame) + assert.False(t, got.Done) +} + +func TestHelloBanner_RoundTrip(t *testing.T) { + banner := HelloBanner{ + HelperVersion: "1.0.0", + ProtoVersion: "1", + Capabilities: []string{"fs.stat", "fs.walk", "tree.hardlink"}, + AllowedRoots: []string{"/home/user/data", "/home/user/seed"}, + ReflinkRoots: []string{"/home/user/data"}, + Platform: "linux", + Hostname: "seedbox01", + PID: 12345, + StartedAt: "2026-04-28T10:00:00Z", + } + got := roundTrip(t, banner) + assert.Equal(t, banner.HelperVersion, got.HelperVersion) + assert.Equal(t, banner.ProtoVersion, got.ProtoVersion) + assert.Equal(t, banner.Capabilities, got.Capabilities) + assert.Equal(t, banner.AllowedRoots, got.AllowedRoots) + assert.Equal(t, banner.ReflinkRoots, got.ReflinkRoots) + assert.Equal(t, banner.Platform, got.Platform) + assert.Equal(t, banner.Hostname, got.Hostname) + assert.Equal(t, banner.PID, got.PID) + assert.Equal(t, banner.StartedAt, got.StartedAt) +} + +func TestStatRequest_RoundTrip(t *testing.T) { + req := StatRequest{Paths: []string{"/data/a.mkv", "/data/b.mkv"}} + got := roundTrip(t, req) + assert.Equal(t, req.Paths, got.Paths) +} + +func TestStatResponse_RoundTrip(t *testing.T) { + resp := StatResponse{Entries: []StatEntry{ + {Path: "/data/a.mkv", Exists: true, Size: 1024, ModTime: "2026-01-01T00:00:00Z", IsDir: false, Mode: 0o644}, + {Path: "/data/gone.mkv", Exists: false, Err: "not_found"}, + }} + got := roundTrip(t, resp) + require.Len(t, got.Entries, 2) + assert.Equal(t, int64(1024), got.Entries[0].Size) + assert.True(t, got.Entries[0].Exists) + assert.Equal(t, "not_found", got.Entries[1].Err) + assert.False(t, got.Entries[1].Exists) +} + +func TestLstatRequest_RoundTrip(t *testing.T) { + req := LstatRequest{Paths: []string{"/data/link"}, WantFileID: true, WantNlinks: true} + got := roundTrip(t, req) + assert.Equal(t, req.Paths, got.Paths) + assert.True(t, got.WantFileID) + assert.True(t, got.WantNlinks) +} + +func TestLstatEntry_RoundTrip(t *testing.T) { + entry := LstatEntry{ + StatEntry: StatEntry{Path: "/data/file", Exists: true, Size: 512, Mode: 0o755}, + IsSymlink: true, + FileID: []byte{0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f}, + Nlinks: 3, + } + got := roundTrip(t, entry) + assert.Equal(t, "/data/file", got.Path) + assert.True(t, got.IsSymlink) + assert.Equal(t, entry.FileID, got.FileID) + assert.Equal(t, uint64(3), got.Nlinks) +} + +func TestWalkRequest_RoundTrip(t *testing.T) { + req := WalkRequest{ + Root: "/data", + SkipHidden: true, + IgnoreDirNames: []string{".git", "node_modules"}, + IgnorePaths: []string{"/data/.cache"}, + WantFileID: true, + WantNlinks: false, + MaxEntries: 10000, + } + got := roundTrip(t, req) + assert.Equal(t, req.Root, got.Root) + assert.True(t, got.SkipHidden) + assert.Equal(t, req.IgnoreDirNames, got.IgnoreDirNames) + assert.Equal(t, req.IgnorePaths, got.IgnorePaths) + assert.True(t, got.WantFileID) + assert.False(t, got.WantNlinks) + assert.Equal(t, 10000, got.MaxEntries) +} + +func TestWalkEntry_RoundTrip(t *testing.T) { + entry := WalkEntry{ + Path: "/data/movies/file.mkv", + RelPath: "movies/file.mkv", + IsDir: false, + Size: 1_000_000, + ModTime: "2026-01-15T14:30:00.123456789Z", + Mode: 0o644, + FileID: []byte{0x01, 0x02}, + Nlinks: 2, + } + got := roundTrip(t, entry) + assert.Equal(t, entry.Path, got.Path) + assert.Equal(t, entry.RelPath, got.RelPath) + assert.Equal(t, entry.Size, got.Size) + assert.Equal(t, entry.FileID, got.FileID) + assert.Equal(t, entry.Nlinks, got.Nlinks) +} + +func TestWalkEntry_Truncated(t *testing.T) { + entry := WalkEntry{Truncated: true} + got := roundTrip(t, entry) + assert.True(t, got.Truncated) +} + +func TestWalkEntry_Error(t *testing.T) { + entry := WalkEntry{Path: "/data/secret", Err: "permission"} + got := roundTrip(t, entry) + assert.Equal(t, "permission", got.Err) +} + +func TestStatfsRequest_RoundTrip(t *testing.T) { + req := StatfsRequest{Path: "/data"} + got := roundTrip(t, req) + assert.Equal(t, "/data", got.Path) +} + +func TestStatfsResponse_RoundTrip(t *testing.T) { + resp := StatfsResponse{BytesAvailable: 1_000_000_000, BytesTotal: 2_000_000_000, Filesystem: "ext4"} + got := roundTrip(t, resp) + assert.Equal(t, int64(1_000_000_000), got.BytesAvailable) + assert.Equal(t, int64(2_000_000_000), got.BytesTotal) + assert.Equal(t, "ext4", got.Filesystem) +} + +func TestReadDirRequest_RoundTrip(t *testing.T) { + req := ReadDirRequest{Path: "/data/movies", MaxEntries: 100} + got := roundTrip(t, req) + assert.Equal(t, "/data/movies", got.Path) + assert.Equal(t, 100, got.MaxEntries) +} + +func TestReadDirResponse_RoundTrip(t *testing.T) { + resp := ReadDirResponse{ + Entries: []DirEntry{ + {Name: "movie.mkv", Size: 5000, Mode: 0o644}, + {Name: "subs", IsDir: true, Mode: 0o755}, + {Name: "link", IsSymlink: true}, + }, + Truncated: false, + } + got := roundTrip(t, resp) + require.Len(t, got.Entries, 3) + assert.Equal(t, "movie.mkv", got.Entries[0].Name) + assert.True(t, got.Entries[1].IsDir) + assert.True(t, got.Entries[2].IsSymlink) +} + +func TestSameFSRequest_RoundTrip(t *testing.T) { + req := SameFSRequest{Path1: "/data/a", Path2: "/data/b"} + got := roundTrip(t, req) + assert.Equal(t, "/data/a", got.Path1) + assert.Equal(t, "/data/b", got.Path2) +} + +func TestSameFSResponse_RoundTrip(t *testing.T) { + resp := SameFSResponse{Same: true} + got := roundTrip(t, resp) + assert.True(t, got.Same) +} + +func TestMkdirRequest_RoundTrip(t *testing.T) { + req := MkdirRequest{Path: "/data/new-dir", Perm: 0o755} + got := roundTrip(t, req) + assert.Equal(t, "/data/new-dir", got.Path) + assert.Equal(t, uint32(0o755), got.Perm) +} + +func TestRemoveRequest_RoundTrip(t *testing.T) { + req := RemoveRequest{ + Path: "/data/old-file", + Recursive: true, + IgnorePaths: []string{"/data/old-file/.keep"}, + } + got := roundTrip(t, req) + assert.Equal(t, "/data/old-file", got.Path) + assert.True(t, got.Recursive) + assert.Equal(t, []string{"/data/old-file/.keep"}, got.IgnorePaths) +} + +func TestRemoveResponse_RoundTrip(t *testing.T) { + resp := RemoveResponse{Removed: true, Disposition: "deleted", RemovedBytes: 4096} + got := roundTrip(t, resp) + assert.True(t, got.Removed) + assert.Equal(t, "deleted", got.Disposition) + assert.Equal(t, int64(4096), got.RemovedBytes) +} + +func TestTreeCreateRequest_RoundTrip(t *testing.T) { + req := TreeCreateRequest{ + Plan: hardlinktree.TreePlan{ + RootDir: "/data/cross-seed/torrent-abc", + Files: []hardlinktree.FilePlan{ + {SourcePath: "/data/movies/file.mkv", TargetPath: "/data/cross-seed/torrent-abc/file.mkv"}, + {SourcePath: "/data/movies/subs.srt", TargetPath: "/data/cross-seed/torrent-abc/subs.srt"}, + }, + }, + Mode: "hardlink", + SourceFS: "ext4", + } + got := roundTrip(t, req) + assert.Equal(t, "hardlink", got.Mode) + assert.Equal(t, "ext4", got.SourceFS) + assert.Equal(t, req.Plan.RootDir, got.Plan.RootDir) + require.Len(t, got.Plan.Files, 2) + assert.Equal(t, req.Plan.Files[0].SourcePath, got.Plan.Files[0].SourcePath) + assert.Equal(t, req.Plan.Files[1].TargetPath, got.Plan.Files[1].TargetPath) +} + +func TestTreeCreateResponse_RoundTrip(t *testing.T) { + resp := TreeCreateResponse{ + Created: 5, + SkippedExists: 2, + RolledBack: false, + } + got := roundTrip(t, resp) + assert.Equal(t, 5, got.Created) + assert.Equal(t, 2, got.SkippedExists) + assert.False(t, got.RolledBack) +} + +func TestTreeCreateResponse_WithRollback(t *testing.T) { + resp := TreeCreateResponse{ + Created: 3, + RolledBack: true, + Err: "partial failure at file 4", + DiagFiles: []string{"file1.mkv", "file2.mkv", "file3.mkv"}, + } + got := roundTrip(t, resp) + assert.True(t, got.RolledBack) + assert.Equal(t, "partial failure at file 4", got.Err) + assert.Equal(t, []string{"file1.mkv", "file2.mkv", "file3.mkv"}, got.DiagFiles) +} + +func TestTreeRemoveRequest_RoundTrip(t *testing.T) { + req := TreeRemoveRequest{ + Plan: hardlinktree.TreePlan{ + RootDir: "/data/cross-seed/torrent-abc", + Files: []hardlinktree.FilePlan{ + {SourcePath: "/data/movies/file.mkv", TargetPath: "/data/cross-seed/torrent-abc/file.mkv"}, + }, + }, + } + got := roundTrip(t, req) + assert.Equal(t, req.Plan.RootDir, got.Plan.RootDir) + require.Len(t, got.Plan.Files, 1) +} + +func TestCancelRequest_RoundTrip(t *testing.T) { + req := CancelRequest{RequestIDs: []string{"req-1", "req-2", "req-3"}} + got := roundTrip(t, req) + assert.Equal(t, []string{"req-1", "req-2", "req-3"}, got.RequestIDs) +} + +func TestDiagEchoRequest_RoundTrip(t *testing.T) { + req := DiagEchoRequest{Message: "hello helper"} + got := roundTrip(t, req) + assert.Equal(t, "hello helper", got.Message) +} + +func TestDiagEchoResponse_RoundTrip(t *testing.T) { + resp := DiagEchoResponse{Message: "hello helper"} + got := roundTrip(t, resp) + assert.Equal(t, "hello helper", got.Message) +} + +func TestIsStreamingOp(t *testing.T) { + assert.True(t, IsStreamingOp(OpWalk)) + assert.False(t, IsStreamingOp(OpStat)) + assert.False(t, IsStreamingOp(OpLstat)) + assert.False(t, IsStreamingOp(OpTreeHardlink)) + assert.False(t, IsStreamingOp(OpDiagEcho)) + assert.False(t, IsStreamingOp(OpControlCancel)) +} + +func TestOmitsZeroValues(t *testing.T) { + // Verify omitempty fields are not present in JSON for zero values. + entry := StatEntry{Path: "/data/file", Exists: true} + data, err := json.Marshal(entry) + require.NoError(t, err) + s := string(data) + assert.NotContains(t, s, "size") + assert.NotContains(t, s, "modTime") + assert.NotContains(t, s, "isDir") + assert.NotContains(t, s, "mode") + assert.NotContains(t, s, "err") +} + +func TestCommand_NDJSONLine(t *testing.T) { + // Verify a Command can be serialized as a single NDJSON line (no embedded newlines). + args, _ := json.Marshal(WalkRequest{Root: "/data", MaxEntries: 100}) + cmd := Command{RequestID: "walk-1", Op: OpWalk, Args: args} + data, err := json.Marshal(cmd) + require.NoError(t, err) + assert.NotContains(t, string(data), "\n") +} From fddd84092edbf84a8d2b5de12b8f1ffa08275567 Mon Sep 17 00:00:00 2001 From: nitrobass24 Date: Tue, 19 May 2026 13:33:26 -0500 Subject: [PATCH 2/5] docs(design): revise implementation plan to feature-scoped PRs Replace monolithic callsite refactor with incremental feature-by-feature migration ordered by user-visible functionality. Each PR proves one feature works through the Backend interface before moving to the next. --- documentation/design/remote-helper.md | 115 +++++++------------------- 1 file changed, 29 insertions(+), 86 deletions(-) diff --git a/documentation/design/remote-helper.md b/documentation/design/remote-helper.md index 3afc73f4a..7b6750cf1 100644 --- a/documentation/design/remote-helper.md +++ b/documentation/design/remote-helper.md @@ -318,92 +318,49 @@ Instance form replaces `hasLocalFilesystemAccess` toggle with a RadioGroup: ## Implementation Plan -### PR 1: Design doc + proto types +### Phase 1: Foundation +#### PR 1: Design doc + proto types - `documentation/design/remote-helper.md` (this document) -- `pkg/agent/proto/proto.go` -- Command, Result, HelloBanner envelopes -- `pkg/agent/proto/ops.go` -- all op request/response types -- `pkg/agent/proto/proto_test.go` -- JSON round-trip tests +- `pkg/agent/proto/` -- shared NDJSON wire types (Command, Result, HelloBanner, op payloads) -Leaf package, zero `internal/` imports. - -### PR 2: Backend interface + local backend + pool - -- `internal/fsops/backend.go` -- Backend interface (17 methods) -- `internal/fsops/types.go` -- value types (FileInfo, LstatInfo, WalkEntry, etc.) -- `internal/fsops/errors.go` -- sentinel errors -- `internal/fsops/noop.go` -- noop backend for instances without FS access -- `internal/fsops/pool.go` -- Pool resolver (instance ID -> Backend) +#### PR 2: Backend interface + local backend + pool +- `internal/fsops/` -- Backend interface, value types, sentinel errors, noop backend, pool resolver - `internal/fsops/local/` -- local backend implementation + platform-specific Statfs -- Tests for all of the above - -No callsite changes yet. Services still use `os.*` directly. - -### PR 3: Service callsite refactors - -Migrate all `os.*`/`unix.*` callsites to `backend.*`. Zero behavioral change. Could split by service area if too large: - -- `automations/missing_files.go` -- `os.Stat` -> `backend.Stat` -- `automations/free_space.go` -- `unix.Statfs` -> `backend.Statfs` -- `automations/hardlink_index.go` -- `os.Lstat` + FileID -> `backend.Lstat` -- `qbittorrent/delete_cleanup.go` -- `os.Stat`/`os.Remove` -> `backend.Stat`/`backend.Remove` -- `dirscan/fileid_index.go` -- FileID index via backend -- `dirscan/inject.go` + `crossseed/FindMatchingBaseDir` -- link-tree ops via backend -- `dirscan/scanner.go` -- `filepath.WalkDir` callback -> `backend.WalkDir` channel -- `orphanscan/walker.go` + `delete.go` -- full walker + delete via backend - -All existing tests must pass with zero behavioral diff. +- No callsite changes. Services still use `os.*` directly. -### PR 4: Schema + models +### Phase 2: Feature-by-feature callsite migration -- Migration 077/078: 16 SSH/helper columns on `instances` -- `internal/models/instance.go` -- extend Instance struct with SSH/helper fields -- `internal/models/filesystem_access.go` -- `HasFilesystemAccess` helper -- `internal/models/filesystem_access_test.go` -- Update InstanceStore Create/Update to handle new fields +Each PR migrates one user-visible feature to use the Backend interface. Zero behavioral change -- the local backend delegates to the same `os.*` calls as before. Ordered from simplest to most complex. -### PR 5: Path safety + SSH pool + helper binary +| PR | Feature | What users see | Backend ops | Files changed | +|---|---|---|---|---| +| 3 | Missing files detection | Automation condition: "torrent has missing files" | Stat | missing_files.go | +| 4 | Free space monitoring | Automation condition: "path has < X GB free" | Statfs | free_space.go | +| 5 | Managed delete cleanup | Empty parent dirs pruned after torrent deletion | Stat, Remove | delete_cleanup.go, sync_manager.go | +| 6 | Hardlink scope detection | Automation condition: "files are hardlinked elsewhere" | Lstat, FileID | hardlink_index.go | +| 7 | Orphan scan | Find and delete files no torrent claims | WalkDir, Lstat, FileID, Remove, ReadDir | walker.go, delete.go, service.go | +| 8 | Cross-seed inject | Full pipeline: scan, index, match, link trees | WalkDir, ReadDir, SameFilesystem, MkdirAll, HardlinkTree, ReflinkTree, RemoveTree, SupportsReflink | scanner.go, fileid_index.go, inject.go, service.go | -- `pkg/fsexec/` -- SafeRoot, Roots, ResolveSafe with os.Root wrapping, property tests -- `internal/sshpool/` -- pool, transport (TOFU), deploy, sweeper -- `cmd/qui-helper/` -- main.go, server (NDJSON loop, diag.echo), executor stub -- `Makefile` -- `make helper` target -- CI workflow for helper (import-graph guard, cross-compile) +PR 8 is developed as separate sub-PRs for reviewability (8a: dirscan walking + FileID, 8b: hardlink inject, 8c: reflink inject) but merged to develop as an atomic group. -### PR 6: API endpoints + wiring +Each PR includes its main.go wiring changes. -- `internal/api/handlers/helper.go` -- 6 endpoints -- `internal/api/server.go` -- route registration -- Wire sshpool into Dependencies struct -- Wire fsops.Pool into services via `cmd/qui/main.go` +### Phase 3: Remote helper infrastructure -### PR 7: First real op (fs.statfs) end-to-end +After all local callsites are migrated: -- `internal/fsops/remote/remote.go` -- Remote backend backed by SSH pool -- Wire `sshpool.Pool.Submit`/`Cancel` with real SSH session dispatch -- Implement `fs.statfs` in helper executor -- Pool resolver returns Remote backend for helper-configured instances -- Dual-backend integration test +- Schema + models -- migration adding SSH/helper columns to `instances` +- `pkg/fsexec` -- path safety with `os.Root` wrapping +- `internal/sshpool` -- SSH connection pool, transport, deploy, sweeper +- `cmd/qui-helper` -- helper binary (NDJSON loop, executor) +- API endpoints -- SSH test, helper deploy/redeploy/remove/status +- `internal/fsops/remote` -- Remote backend backed by SSH pool +- Wire first real op end-to-end, then remaining ops iteratively -### PR 8+: Remaining ops (one PR each) +### Phase 4: Frontend -Read ops first, destructive ops later: - -1. `fs.stat` -- missing-files condition -2. `fs.lstat` -- hardlink index, FileID index -3. `fs.samefs` -- hardlink/reflink precondition -4. `fs.readdir` -- disc-layout detection, orphanscan -5. `fs.walk` (streaming) -- dirscan, orphanscan -6. `fs.mkdir` -- link-tree materialization -7. `fs.remove` -- managed delete cleanup, orphanscan -8. `fs.removeall` -- orphanscan directory deletion -9. `tree.hardlink` -- cross-seed inject -10. `tree.reflink` -- cross-seed inject (capability-gated) -11. `tree.remove` -- link-tree rollback - -Each PR adds: helper executor case + Remote backend method + dual-backend integration test. - -### Frontend PR (parallel with PR 7+) +Held on a feature branch until UA tested: - Instance form RadioGroup (None / Local / Remote helper) - SSH credential fields + host key confirmation modal @@ -411,18 +368,4 @@ Each PR adds: helper executor case + Remote backend method + dual-backend integr - Helper status card - `authorized_keys` hardening snippet with copy button -## Salvageable Code from PR #1820 - -The following packages from the original `feat/fsops-backend-scaffold` branch are production-quality and can be cherry-picked onto fresh branches with minimal changes: - -| Package | Verdict | Tests | -|---|---|---| -| `pkg/agent/proto` | Use as-is | 38 tests | -| `internal/fsops` (interface, pool, noop, types, errors) | Use as-is | 4 tests | -| `internal/fsops/local` | Use as-is | 35+ tests | -| `pkg/fsexec` | Use as-is | 17 tests | -| `cmd/qui-helper` | Use as-is | 5 tests | -| `internal/models/filesystem_access` | Use as-is | 6 tests | -| `internal/sshpool` | Needs test coverage for deploy.go and sweeper.go | 12 tests | - All packages have zero coupling to the unrelated changes in PR #1820 and will apply cleanly on develop. From 5849113c71f0a6624d1bbe1b35111b711a9ab159 Mon Sep 17 00:00:00 2001 From: nitrobass24 Date: Tue, 19 May 2026 13:40:39 -0500 Subject: [PATCH 3/5] =?UTF-8?q?docs(design):=20correct=20phasing=20?= =?UTF-8?q?=E2=80=94=20remote=20helper=20before=20feature=20rollout?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 2 is just missing files detection (proves the pattern). Phase 3 builds the remote helper infrastructure. Phase 5 iteratively adds remaining filesystem features after the helper is released, so each new feature works for both local and remote instances. --- documentation/design/remote-helper.md | 46 ++++++++++++--------------- 1 file changed, 21 insertions(+), 25 deletions(-) diff --git a/documentation/design/remote-helper.md b/documentation/design/remote-helper.md index 7b6750cf1..ef5141d11 100644 --- a/documentation/design/remote-helper.md +++ b/documentation/design/remote-helper.md @@ -320,43 +320,27 @@ Instance form replaces `hasLocalFilesystemAccess` toggle with a RadioGroup: ### Phase 1: Foundation -#### PR 1: Design doc + proto types -- `documentation/design/remote-helper.md` (this document) -- `pkg/agent/proto/` -- shared NDJSON wire types (Command, Result, HelloBanner, op payloads) - -#### PR 2: Backend interface + local backend + pool -- `internal/fsops/` -- Backend interface, value types, sentinel errors, noop backend, pool resolver -- `internal/fsops/local/` -- local backend implementation + platform-specific Statfs +- Design doc + `pkg/agent/proto/` (shared NDJSON wire types) +- `internal/fsops/` (Backend interface, local backend, pool resolver) - No callsite changes. Services still use `os.*` directly. -### Phase 2: Feature-by-feature callsite migration - -Each PR migrates one user-visible feature to use the Backend interface. Zero behavioral change -- the local backend delegates to the same `os.*` calls as before. Ordered from simplest to most complex. - -| PR | Feature | What users see | Backend ops | Files changed | -|---|---|---|---|---| -| 3 | Missing files detection | Automation condition: "torrent has missing files" | Stat | missing_files.go | -| 4 | Free space monitoring | Automation condition: "path has < X GB free" | Statfs | free_space.go | -| 5 | Managed delete cleanup | Empty parent dirs pruned after torrent deletion | Stat, Remove | delete_cleanup.go, sync_manager.go | -| 6 | Hardlink scope detection | Automation condition: "files are hardlinked elsewhere" | Lstat, FileID | hardlink_index.go | -| 7 | Orphan scan | Find and delete files no torrent claims | WalkDir, Lstat, FileID, Remove, ReadDir | walker.go, delete.go, service.go | -| 8 | Cross-seed inject | Full pipeline: scan, index, match, link trees | WalkDir, ReadDir, SameFilesystem, MkdirAll, HardlinkTree, ReflinkTree, RemoveTree, SupportsReflink | scanner.go, fileid_index.go, inject.go, service.go | +### Phase 2: First feature migration -PR 8 is developed as separate sub-PRs for reviewability (8a: dirscan walking + FileID, 8b: hardlink inject, 8c: reflink inject) but merged to develop as an atomic group. +Migrate missing files detection to prove the Backend pattern end-to-end with one user-visible feature. Zero behavioral change for local installs. -Each PR includes its main.go wiring changes. +| Feature | What users see | Backend ops | Files changed | +|---|---|---|---| +| Missing files detection | Automation condition: "torrent has missing files" | Stat | missing_files.go, service.go, main.go | ### Phase 3: Remote helper infrastructure -After all local callsites are migrated: - - Schema + models -- migration adding SSH/helper columns to `instances` - `pkg/fsexec` -- path safety with `os.Root` wrapping - `internal/sshpool` -- SSH connection pool, transport, deploy, sweeper - `cmd/qui-helper` -- helper binary (NDJSON loop, executor) - API endpoints -- SSH test, helper deploy/redeploy/remove/status - `internal/fsops/remote` -- Remote backend backed by SSH pool -- Wire first real op end-to-end, then remaining ops iteratively +- Wire `Stat` op end-to-end so missing files works on remote instances ### Phase 4: Frontend @@ -368,4 +352,16 @@ Held on a feature branch until UA tested: - Helper status card - `authorized_keys` hardening snippet with copy button -All packages have zero coupling to the unrelated changes in PR #1820 and will apply cleanly on develop. +### Phase 5: Iterative feature rollout + +After the remote helper is released, migrate remaining features one at a time. Each adds new Backend ops and wires them through both local and remote backends. Ordered by user value and complexity: + +| Feature | What users see | Backend ops | +|---|---|---| +| Free space monitoring | Automation condition: "path has < X GB free" | Statfs | +| Managed delete cleanup | Empty parent dirs pruned after torrent deletion | Stat, Remove | +| Hardlink scope detection | Automation condition: "files are hardlinked elsewhere" | Lstat, FileID | +| Orphan scan | Find and delete files no torrent claims | WalkDir, Lstat, FileID, Remove, ReadDir | +| Cross-seed inject | Full pipeline: scan, index, match, hardlink/reflink trees | WalkDir, ReadDir, SameFilesystem, MkdirAll, HardlinkTree, ReflinkTree, RemoveTree, SupportsReflink | + +Cross-seed inject is developed as separate sub-PRs for reviewability (dirscan walking + FileID, hardlink inject, reflink inject) but merged as an atomic group. From f133b77c788ea44662e73ee4e67170a6d06f4f34 Mon Sep 17 00:00:00 2001 From: nitrobass24 Date: Tue, 19 May 2026 16:42:46 -0500 Subject: [PATCH 4/5] docs(design): detail Phase 3 PR splits and dependencies --- documentation/design/remote-helper.md | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/documentation/design/remote-helper.md b/documentation/design/remote-helper.md index ef5141d11..30c7f8f1e 100644 --- a/documentation/design/remote-helper.md +++ b/documentation/design/remote-helper.md @@ -334,13 +334,12 @@ Migrate missing files detection to prove the Backend pattern end-to-end with one ### Phase 3: Remote helper infrastructure -- Schema + models -- migration adding SSH/helper columns to `instances` -- `pkg/fsexec` -- path safety with `os.Root` wrapping -- `internal/sshpool` -- SSH connection pool, transport, deploy, sweeper -- `cmd/qui-helper` -- helper binary (NDJSON loop, executor) -- API endpoints -- SSH test, helper deploy/redeploy/remove/status -- `internal/fsops/remote` -- Remote backend backed by SSH pool -- Wire `Stat` op end-to-end so missing files works on remote instances +Four PRs, ordered by dependency chain: + +1. **Schema + models** (~300 lines, independent) -- migration adding 16 SSH/helper columns to `instances`, `HasFilesystemAccess` helper, `scanInstance` shared query helper +2. **pkg/fsexec** (~420 lines, independent) -- path safety with `os.Root` wrapping, allowed-roots validation, device-ID guard, property tests +3. **sshpool + helper binary** (~1,120 lines, depends on #1 and #2) -- `internal/sshpool` (SSH connection pool, TOFU transport, deploy, sweeper) + `cmd/qui-helper` (NDJSON stdio loop, executor stub, `diag.echo`) + Makefile/CI for cross-compile +4. **API + remote backend + wiring** (~575 lines, depends on #3) -- 6 API endpoints (SSH test, deploy, redeploy, remove, status, clear credentials) + `internal/fsops/remote` (Remote backend backed by SSH pool) + wire `Stat` end-to-end so missing files works on remote instances ### Phase 4: Frontend From 54b2b1e4fcf560f862c4b547465d2b15ccf69c07 Mon Sep 17 00:00:00 2001 From: nitrobass24 Date: Tue, 19 May 2026 21:02:13 -0500 Subject: [PATCH 5/5] docs(design): add merge strategy section --- documentation/design/remote-helper.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/documentation/design/remote-helper.md b/documentation/design/remote-helper.md index 30c7f8f1e..d73693878 100644 --- a/documentation/design/remote-helper.md +++ b/documentation/design/remote-helper.md @@ -316,6 +316,12 @@ Instance form replaces `hasLocalFilesystemAccess` toggle with a RadioGroup: "Remote helper" reveals SSH credential fields, test connection button (TOFU host key confirmation), deploy button with progress UI, and helper status card. +## Merge Strategy + +Backend PRs merge to develop as they're reviewed and approved — no user-visible changes until the frontend ships. Frontend is held on a feature branch until UA tested against the full backend stack. + +An integration branch (`integration/remote-helper-ua`) stacks all in-progress PRs for build/test verification before individual PRs merge. + ## Implementation Plan ### Phase 1: Foundation