Skip to content

docs(process): un-stub implemented id-keyed write-stdin/close-stdin#14

Merged
joshuajbouw merged 2 commits into
mainfrom
docs/persistent-stdin-untag
Jun 8, 2026
Merged

docs(process): un-stub implemented id-keyed write-stdin/close-stdin#14
joshuajbouw merged 2 commits into
mainfrom
docs/persistent-stdin-untag

Conversation

@joshuajbouw

Copy link
Copy Markdown
Contributor

The persistent-tier registry landed in astrid-runtime/astrid#867 — the id-keyed write-stdin/close-stdin are implemented (pipe retained host-side, survives pooled-instance reset, 1 MiB cap, backpressure, ownership-checked, audited). Drops the stale (NOT YET IMPLEMENTED) tags on those two funcs and corrects the two PERSISTENT TIER banners that claimed the whole tier was stubbed until the registry lands. Doc-only: signatures unchanged, no wire/binding impact. The ProcessHandle (ephemeral) write-stdin/close-stdin, attach, and watch/unwatch remain genuinely deferred and keep their notes.

The persistent-tier registry landed in astrid-runtime/astrid#867: the id-keyed
write-stdin/close-stdin are implemented (pipe retained host-side, survives
pooled-instance reset, 1 MiB cap, backpressure, ownership-checked, audited).
Drops the stale (NOT YET IMPLEMENTED) tags on those two funcs and corrects the
two PERSISTENT TIER banners that claimed the whole tier was stubbed 'until the
registry lands'. The ProcessHandle (ephemeral) write-stdin/close-stdin, attach,
and watch/unwatch remain genuinely deferred and keep their notes.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the documentation comments in host/process@1.0.0.wit to reflect that the persistent registry has landed and its shapes are frozen. It also details the behavior and constraints of the write-stdin and close-stdin functions. The review feedback points out minor inconsistencies in terminology (using '1 MiB' instead of '1 MB') and spelling ('re-attached' instead of 'reattached') compared to the rest of the file.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread host/process@1.0.0.wit Outdated
Comment on lines +509 to +511
/// Caps at 1 MiB/call (`too-large` above that); returns bytes written. The
/// retained pipe lives in the host registry, so a later instance that
/// re-attached keeps writing to the same child across pooled-instance

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

There are a couple of minor inconsistencies introduced in these new comments:

  1. Terminology (1 MiB vs 1 MB): The new comment uses 1 MiB/call, whereas other parts of the file (such as the too-large error on line 60 and the ephemeral write-stdin on line 382) use 1 MB. It would be best to standardize on either MiB or MB throughout the file depending on the exact byte limit enforced by the host.
  2. Spelling (re-attached vs reattached): The term reattach is used without a hyphen everywhere else in this file (e.g., lines 20, 203, 334, 466). Using reattached here maintains consistency.
    /// Caps at 1 MiB/call (too-large error above that); returns bytes written. The
    /// retained pipe lives in the host registry, so a later instance that
    /// reattached keeps writing to the same child across pooled-instance

The un-stub text claimed write-stdin is "all-or-nothing per call, so framed stdin is never torn". The host writes via write_all over a pipe, which can deliver a prefix before an I/O error and then marks the stream closed, so a failed write is NOT atomic and CAN tear a frame. Pipe writes are atomic only up to PIPE_BUF for a single write(), never for a 1 MiB write_all, so the contract cannot promise this. State the real semantics: a successful return wrote every byte; an error may have delivered a prefix and closes the stream.
joshuajbouw added a commit to astrid-runtime/astrid that referenced this pull request Jun 8, 2026
…e-stdin WIT (#871)

## Linked Issue

Closes #870

## Summary

The id-keyed persistent `write-stdin` / `close-stdin` were implemented
in #867 (the registry retains the child's stdin pipe host-side, survives
pooled-instance reset, 1 MiB-capped, backpressured, ownership-checked,
audited) but had **no test**, and the WIT still tagged them `(NOT YET
IMPLEMENTED)`. This adds the acceptance test and un-stubs the
now-accurate WIT docs. Doc-only on the contract — no signature change.

## Changes

- **Test** (`registry_tests.rs`): write-stdin delivers bytes to a
persistent child; a second by-id write (needing only registry + id —
exactly what a post-reset instance holds) still reaches the same child;
`close-stdin` → clean EOF exit; over-cap → `too-large`; wrong-owner →
`no-such-process`; post-close → `closed`.
- **WIT** (submodule bump → astrid-runtime/wit#14): drops the stale
`(NOT YET IMPLEMENTED)` tags on the id-keyed forms and corrects the two
`PERSISTENT TIER` "stubbed until the registry lands" banners. Restages
`wit-staging`. Signatures unchanged.
- The ephemeral `ProcessHandle` form, `attach`, and `watch` / `unwatch`
stay genuinely deferred and tagged.
- CHANGELOG updated under `[Unreleased]`.

## Test Plan

### Automated

- [x] `cargo test --workspace` passes
- [x] No new clippy warnings

Specifically: `cargo test -p astrid-capsule registry_tests` (the new
acceptance test + existing registry tests pass); build green (WIT parses
via bindgen).

## Checklist

- [x] Linked to an issue
- [x] CHANGELOG.md updated under `[Unreleased]`
@joshuajbouw joshuajbouw merged commit 83ebc6c into main Jun 8, 2026
1 check passed
@joshuajbouw joshuajbouw deleted the docs/persistent-stdin-untag branch June 8, 2026 10:57
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