Skip to content

feat(daemon): enable private Windows named-pipe service - #16

Merged
jlongster merged 2 commits into
masterfrom
windows-daemon
Oct 8, 2026
Merged

jlongster merged 2 commits into
masterfrom
windows-daemon

Conversation

@jlongster

@jlongster jlongster commented Sep 3, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • Enable the shared Windows daemon with protocol 7 and the unchanged exclusive-owner/handoff state machine.
  • Store registration and locking state in LOCALAPPDATA/opencode-pty or an absolute OPENCODE_PTY_RUNTIME_DIR override. Enforce current-user protected ACLs, ownership/reparse checks, non-delete-shared directory/lock handles, and instance-scoped cleanup.
  • Publish atomically using FileRenameInfoEx replace/POSIX flags on the held private source. The full destination comes from GetFinalPathNameByHandleW on the held directory; RootDirectory is NULL for the supported Win32 wrapper. Open readers retain the old complete file; new readers see the new complete file. No delete gap, source reopen, ACL relaxation, or non-atomic fallback.
  • Retain pipe namespace ownership through cleanup; cancel partial requests and blocked subscribers before service shutdown; keep the cleanup watchdog on listener-error paths too.
  • Expose only a minimal PipeConnection byte stream with optional I/O timeouts, not Windows TerminalClient/interactive CLI parity.
  • Add five real daemon tests and two private/atomic storage regressions. Reuse P2's shared console fixture without divergent setup.

Dependency correction

The original standalone T3 ancestry exposed an ARM64 owner-loss cleanup watchdog failure under blocked subscriber output. This PR was rebased onto the full P2 runtime cleanup series, not a partial fix. The strict failing daemon regression is unchanged and now passes on both native architectures. Runtime, service, and shared-fixture bytes are identical to the frozen P2 base.

Verification — head 040087b

  • Windows run 33787060709: native x64 and ARM64 each pass 34 library + 5 callback-effects + 12 real runtime + 1 retained-pointer + 5 real daemon tests = 57 tests, plus the standalone 34-library-test rerun. The two ignored tests are subprocess fixture entrypoints, not skipped coverage. Native Clippy, formatting, architecture checks, and executable smoke tests pass.
  • The daemon suite verifies authentication/live-owner exclusion/handoff, lock and stale-instance handling, unclaimed partial-request cancellation, real ConPTY create/input/output/resize/shutdown, and owner-loss cancellation of a blocked subscriber plus partial request with the child process signalled.
  • Core run 33787060669: Linux 61 tests, macOS 56 tests, and Miri 5 callback + 1 retained-pointer test under each borrow model pass; formatting and locked Clippy pass.
  • Local quick checks: 48 selected tests, locked all-target/all-feature Clippy, formatting, actionlint, both retained-pointer Miri modes, and range-diff inspection pass.

Unmerged stack

master -> windows-transport (#13) -> windows-pipes (#14) -> conpty-runtime (#12) -> conpty-cleanup (#15) -> windows-daemon (this PR).

Base: conpty-cleanup at c92abf2e6587dac215a2f73e0c0861c162507884.

No merges or publication. Source is frozen pending coordinator-led verification. Basic coverage does not claim exhaustive lifecycle coverage, repair the documented Linux blocked-master-write limitation, or add Windows interactive CLI parity.

@jlongster
jlongster changed the base branch from windows-pipes to conpty-cleanup September 3, 2026 17:51
@jlongster
jlongster marked this pull request as ready for review September 3, 2026 18:01
@jlongster

Copy link
Copy Markdown
Collaborator Author

Restacked onto current master and ported, folding the two follow-up rename fixes into one commit:

  • Windows now uses the shared --name/--runtime-dir layout instead of the fixed LOCALAPPDATA path / OPENCODE_PTY_RUNTIME_DIR override. Private ACL, reparse-point, lock and atomic FileRenameInfoEx publication checks are unchanged; clients still verify the registration owner/ACL.
  • master's abandoned-sibling sweep moved to a shared daemon/sweep.rs; platforms supply only process liveness (Windows: process object) and stale endpoint removal (Windows: none).
  • After registration cleanup the daemon releases its protected handles and removes the lock file and runtime directory, matching Unix.
  • The server uses fix(pty): allow ticket-authorized ownership takeover #10's generation-based ownership; the Windows handoff test now covers takeover while the old owner is still connected.

Rebased onto named runtime directories and ticket takeover:
- Windows uses the shared `--name`/`--runtime-dir` layout instead of a fixed
  LOCALAPPDATA path, keeping private ACL, reparse, lock and atomic
  FileRenameInfoEx publication checks (folded from the follow-up rename fixes).
- The stale-sibling sweep is shared by Unix and Windows; platforms supply only
  process liveness and stale endpoint removal.
- The shared server uses generation-based ownership, and the Windows handoff
  test covers takeover while the previous owner is connected.
@jlongster
jlongster changed the base branch from conpty-cleanup to master October 8, 2026 19:51
A crashed daemon leaves its registration behind, and its pipe name is then
free for another local user to create. PipeConnection::connect_to checks
GetNamedPipeServerProcessId against the registered PID before a client sends
its token or input, and the docs require integrations to do the same.

Also treat a failed liveness wait as live in the stale-runtime sweep, read the
registration through the owner/ACL-checked reader in daemon tests, document
that connections must be discarded after a timeout, and describe Windows
storage and its NTFS/POSIX-rename requirement in README and SPEC.
@jlongster

Copy link
Copy Markdown
Collaborator Author

Review follow-up (4453740):

  • Pipe-name squatting after a crash: PipeConnection::connect_to(socket, pid) now checks GetNamedPipeServerProcessId against the registered PID before a client sends its token or input. The README and SPEC require integrations in other languages to make the same check. A new transport test covers acceptance and rejection, and the daemon tests connect through it.
  • The stale-runtime sweep treats a failed liveness wait as live.
  • Daemon tests read the registration through the owner/ACL-checked reader.
  • Docs: discard a connection after a timeout, Windows storage in SPEC, the NTFS/POSIX-rename requirement, and a stale release note.
    test(windows): verify daemon lifecycle and isolation #17–refactor(windows): simplify metadata reads and test setup #19 were restacked onto this.

@jlongster
jlongster merged commit 10f64cb into master Oct 8, 2026
6 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant