remotefs operations cannot be bounded: no way to pass a context to filesystem commands
- Dominant language
- Go
- Stars
- 53
- Forks
- 33
- Avg merge
- 1d 9h
- Merged PRs (30d)
- 29
Description
## What
`remotefs` filesystem operations run their commands with `context.Background()`, so there is no way for a caller to bound them. On a host that has stopped answering, a single `Stat` or `Sha256` blocks for as long as the connection takes to notice — which, for WinRM with no deadline, is forever.
61 call sites across `remotefs/posixfs.go` (35), `remotefs/winfs.go` (21) and `remotefs/posixfile.go` (5) run a command with no caller-supplied context.
## Why now
Came out of #473, which bounded the rigrcp session behind a Windows file so that a session dying *mid-transfer* fails instead of hanging. Copilot rightly pointed out during review that this does not make the public call bounded end to end:
- `WinFS.OpenFile` starts with a `Stat` probe, which runs `ExecOutput` on a background context. An `Upload` against a host that went away before the transfer starts still hangs there, before the new watchdog is ever armed.
- `Upload`'s cleanup calls `RemoveAll`, which starts with a `Stat` on the same connection. #473 works around that specific one by skipping cleanup after a session timeout, but that is a patch on one path, not a fix.
The `protocol/winrm` contract after #473 is explicit: a background context stays unbounded, and a caller wanting a bound sets a deadline. `remotefs` has no way to express that.
## The constraint
This is an API shape problem, not an oversight. `remotefs.FS` embeds `fs.FS`, `fs.StatFS`, `fs.ReadFileFS` and `fs.ReadDirFS`, whose signatures are fixed by the standard library — `Open(name string)`, `Stat(name string)` and friends have nowhere to put a `context.Context`, ever.
Rig's own methods are not stuck that way, and the interface is already inconsistent about it: `remotefs.OS` has `Follow(ctx, path, w)` and `Reboot(ctx)` taking contexts, while `Remove`, `Mkdir`, `WriteFile`, `Rename`, `Sha256` and the rest do not. `cmd.Runner` already embeds `ContextRunner`, so the plumbing exists below this layer.
## Possible directions
Not asking for a specific design, but the options seem to be:
1. **A context-carrying derived FS** — something like `fsys.WithContext(ctx) FS`, returning an FS whose commands all run under `ctx`. Additive, and the only option that also covers the `fs.FS`-shaped methods.
2. **Context variants on rig's own methods** — `RemoveContext`, `Sha256Context` and so on. Additive but partial: it cannot reach `Open`/`Stat`/`ReadFile`/`ReadDir`, which are where the `Upload` hangs above actually happen.
3. **A default command timeout on the connection or runner**, so that every command is bounded even when nobody passes a context. Cheapest, but it changes behaviour for long-running commands, which is exactly why #473 deliberately left background contexts unbounded.
(1) looks like the one worth exploring first. It is worth deciding before more callers grow their own per-path workarounds like the one #473 added to `Upload`.
Contributor guide
No contributing guide indexed for this repository
Research direction
Start by tracing the 61 command call sites in remotefs/posixfs.go, remotefs/winfs.go, and remotefs/posixfile.go, then inspect remotefs.FS, remotefs.OS, and cmd.Runner's ContextRunner plumbing. Compare the existing context handling from #473 with the proposed derived-FS direction; done means a decided API can bound both rig-specific operations and fs.FS-shaped methods without leaving background-context calls.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- backend-api-design
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100