Skip to content

fix(capsule/net): TcpStream::{read_bytes,peek} surface cancellation as Closed#760

Merged
joshuajbouw merged 2 commits into
mainfrom
fix/tcp-cancel-and-changelog-cleanup
May 25, 2026
Merged

fix(capsule/net): TcpStream::{read_bytes,peek} surface cancellation as Closed#760
joshuajbouw merged 2 commits into
mainfrom
fix/tcp-cancel-and-changelog-cleanup

Conversation

@joshuajbouw

@joshuajbouw joshuajbouw commented May 25, 2026

Copy link
Copy Markdown
Contributor

Linked Issue

Closes #759

Summary

Fix the TcpStream::read_bytes / peek cancellation hole flagged by Gemini on #758 (CHANGELOG.md:48 thread), plus a CHANGELOG [Unreleased] cleanup that consolidates duplicate Added/Changed blocks from the independent #752 + #746 roll-ups. Goes in before #758 so the 0.7.0 release inherits the corrected behaviour and cleaner CHANGELOG structure.

Changes

crates/astrid-capsule/src/engine/wasm/host/net/tcp_stream.rs

  • read_bytes: None => Ok(Vec::new())None => Err(ErrorCode::Closed)
  • peek: result.unwrap_or(Ok(Vec::new()))result.unwrap_or(Err(ErrorCode::Closed))

Both methods previously collapsed cancellation into an empty Vec<u8>, which is indistinguishable from a clean EOF in byte-stream reads (std::io::Read::read / tokio::io::AsyncReadExt::read convention). The framed read path (read_frame) and the write side (write_bytes, shutdown) already return Err(ErrorCode::Closed) on cancellation. read_bytes / peek were the outliers.

Capsules with EOF-triggered finalizers (write trailers, send last-message IPC, flush log, transition to "stream complete" state) would execute those finalizers under a forced unload when the cancel fires mid-read. The bug surface is narrow (the capsule has milliseconds to live) but it's a real correctness hole. Now Closed distinguishes cancellation from EOF; empty Vec keeps its "clean EOF" meaning (no behaviour change for the EOF case).

CHANGELOG.md ([Unreleased] consolidation)

Test Plan

Automated

  • cargo test -p astrid-capsule --lib — 272 passed, 0 failed
  • cargo clippy --workspace --all-features -- -D warnings clean

Manual

  • N/A — the cancellation path only fires on capsule unload, exercised in integration

Checklist

  • Linked to an issue
  • CHANGELOG.md updated under [Unreleased]

…s Closed

Closes #759

Both methods previously collapsed cancel-token fire into Ok(Vec::new()),
which is indistinguishable from a clean EOF in byte-stream reads. The
framed read path (read_frame) and write side (write_bytes, shutdown)
already return Err(ErrorCode::Closed) on cancellation — read_bytes /
peek were the outliers.

Capsules with EOF-triggered finalizers (write trailers, send last-message
IPC, flush log, transition to "stream complete") would execute those
finalizers under a forced unload when the cancel fires mid-read. Now
matches the rest of the TcpStream surface: empty Vec keeps its
"clean EOF" meaning; cancellation surfaces as its own error variant.

Reported by Gemini on PR #758.

Bundled CHANGELOG cleanup
- Consolidate duplicate Added/Changed blocks in [Unreleased] (created
  when #752 and #746 landed independently with their own roll-up
  entries). Now: single Added block, single Changed block, single
  Fixed block.
- Add Fixed entry for the read_bytes / peek change above.
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Summary of Changes

Hello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses a subtle correctness issue in the network stream handling where cancellation events were being conflated with clean EOF signals. By surfacing cancellation as ErrorCode::Closed, capsules can now distinguish between a graceful stream termination and a forced unload, ensuring that finalization logic is only executed when appropriate. Additionally, the changelog has been cleaned up to remove redundant entries.

Highlights

  • TCP Cancellation Handling: Updated TcpStream::read_bytes and peek to return ErrorCode::Closed instead of an empty Vec when cancellation occurs, preventing capsules from incorrectly triggering EOF finalizers during teardown.
  • CHANGELOG Cleanup: Consolidated duplicate entries in the [Unreleased] section of the changelog for better readability and structural consistency.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.


The stream was closed by sudden hand, But empty bytes were all they planned. Now Closed is sent to signal why, So final tasks don't go awry.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This PR introduces outbound TCP support for capsules via the net.connect-tcp host function and net_connect capability, alongside a refactor of the manifest and KV storage modules into submodules. It also updates the build toolchain for target-agnosticism and ensures that TCP stream cancellation returns a distinct ErrorCode::Closed instead of an empty vector. Feedback was provided to add missing audit logging to the peek implementation, ensuring consistency with other network operations and proper handling of logging failures.

Comment thread crates/astrid-capsule/src/engine/wasm/host/net/tcp_stream.rs Outdated
Reported by Gemini on #760. peek was the only data-transfer method on
TcpStream that didn't emit an astrid.audit.net event — read / write /
read_bytes / write_bytes / shutdown all do, so peek was the outlier.
Peek can leak observable bytes if the airlock/quota ever loosens, so
an operator needs the per-call audit signal even if it's a no-op
preview.

Gemini's suggested snippet wrapped audit_net in if let Err(...);
audit_net returns () (it just emits via tracing::debug!), so the
correct form is the unadorned call site used everywhere else.
@joshuajbouw joshuajbouw merged commit 5225a8d into main May 25, 2026
13 checks passed
@joshuajbouw joshuajbouw deleted the fix/tcp-cancel-and-changelog-cleanup branch May 25, 2026 13:02
joshuajbouw added a commit that referenced this pull request May 25, 2026
## Linked Issue

Closes #757

## Summary

Bump all workspace crates from 0.6.0 to 0.7.0. Big release rolling up the
per-domain WIT host ABI migration (#752 — wasi-elimination, every host
call routed through audited astrid:* interfaces), outbound TCP for
capsules (#746), wasmtime 43 → 45 (closes RUSTSEC-2026-0149), atomic
kv_cas, O(1) HostState quota counters, the Gemini review fixups for
#752, and the TcpStream cancellation hole closed in #760.

## Changes

- Workspace version 0.6.0 → 0.7.0
- All 20 workspace dependency versions updated to 0.7.0
- CHANGELOG [Unreleased] rolled into [0.7.0] (consolidation already
  done in #760 — Breaking, Added, Changed, Fixed all single-section)
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.

fix(capsule/net): TcpStream::{read_bytes,peek} return empty Vec on cancellation, indistinguishable from clean EOF

1 participant