Skip to content

inbox: exit once its home is deleted, and with the test that started it - #80

Merged
krishhgg merged 3 commits into
mainfrom
fix/test-inboxes-exit
Oct 6, 2026
Merged

krishhgg merged 3 commits into
mainfrom
fix/test-inboxes-exit

Conversation

@krishhgg

@krishhgg krishhgg commented Oct 6, 2026 •

Copy link
Copy Markdown
Owner

What leaked

After a day of cargo test and scripts/leak-test.sh runs, this machine had 365 target/debug/tokenstash inbox processes running, every one with TOKENSTASH_HOME under /tmp. On main, one cargo test --workspace run leaves 11 of them: 7 from human_only.rs, 2 from mcp_surface.rs, 1 from mcp_bind.rs and 1 from init_mode.rs.

Why

A need that files a card calls notify::ensure_inbox, which starts tokenstash inbox in the background. The inbox exits only after 30 idle minutes with no open cards. Tests file cards and do not answer them, so those inboxes never exit, and many of their temp homes stay on disk too.

What changed

  • Once a second, whether requests arrive or not, the inbox checks that its home still exists and that its database is the file it opened (same device and inode). It exits when the home is gone or the database is gone or replaced. This holds with --keep too. A real user loses nothing, since there is nothing left to serve. The port also comes free: the old inbox answers /verify with the proof key it read at start, so a need in a recreated home would find it foreign and hand out no links.
  • Debug builds read TOKENSTASH_EXIT_WITH=<pid> and exit once that process has ended. On Linux the check reads /proc/<pid>/stat and counts a zombie as ended. Elsewhere it runs kill -0. Release builds never read the variable, the same way they ignore TOKENSTASH_TAILSCALE.
  • ensure_inbox passes its environment to the inbox it starts. Every integration test now starts the binary through tests/common/mod.rs::tokenstash(), which sets the variable to the test process's pid. The pty run in init_mode.rs clears the environment, so it sets the variable itself. leak-test.sh and agent-conformance.sh export it as $$.
  • The tests that start an inbox themselves (the Inbox guards in inbox_scope.rs, inbox_deadline.rs and remote_tailscale.rs) still kill and reap it on drop. They are unchanged apart from going through the helper.
  • The --keep help text and the uninstall steps in the README now say that an inbox exits once its data directory is deleted.

How it was verified

  • New crates/cli/tests/inbox_exit.rs, 2.5 s. Each test files a card with need in a scratch home and checks the inbox still answers 1.5 s later. It then removes one thing and waits up to 5 s for the port to close, sending a /verify every 100 ms meanwhile so the inbox is never idle for a second. The things removed are a stand-in parent (sleep 30, named in TOKENSTASH_EXIT_WITH), the home, and the database. A fourth test replaces the home with a new one where another need has run, expects the old inbox to exit, and expects the next need to print a card link. All four tests fail against the inbox on main.
  • A unit test in cmd/inbox.rs checks process_alive against a child process: running, a zombie (Linux), and reaped.
  • cargo clippy --workspace --all-targets --locked -- -D warnings passes in debug and release, and cargo test --workspace --locked --release --no-run builds without warnings. cargo test --workspace --locked passes, and so does bash scripts/leak-test.sh, on this branch and merged with the current main.
  • Process counts for this worktree's target/debug/tokenstash, from pgrep -a tokenstash:
    • On main, 0 before cargo test --workspace and 11 inboxes after.
    • On this branch, 0 before and 0 after, and 0 after the leak test. While four of the test files ran, up to 8 inboxes were up at once. All of them were gone 1.25 s after the run ended.

🤖 Generated with Claude Code

RetriggerConfidence Score: 5/5

The PR appears safe to merge; the exit tests no longer accept stalled inboxes.

What we checked:

  • Stalled replies cannot pass: inbox_exits returns success only after a refused connection. A failed or stalled reply does not satisfy that check.
Summary

The inbox exits when its home or database disappears, when the database is replaced on Unix, or when a named process ends in debug builds. Test helpers and scripts tie background inboxes to their test run.

  • The latest change requires a refused connection before an exit test passes.
  • The numbered previous finding is fixed. No new actionable issues were found.

Reviews (4) · Last reviewed commit: "tests: an inbox counts as gone only once..."

@krishhgg
krishhgg force-pushed the fix/test-inboxes-exit branch from 7bfdc4c to 40afc1e Compare October 6, 2026 05:02
Comment thread crates/cli/src/cmd/inbox.rs Outdated
Comment thread crates/cli/src/cmd/inbox.rs Outdated
Comment thread crates/cli/tests/inbox_exit.rs Outdated
krishhgg and others added 2 commits October 6, 2026 05:27
A `need` that files a card starts an inbox in the background, and an inbox
with an open card never reaches its idle exit. Tests file cards and leave
them, so one `cargo test` run left 11 inboxes running for good.

The inbox now checks on each idle tick that its home and database still
exist, and exits when either is gone, with or without --keep. Debug builds
also read TOKENSTASH_EXIT_WITH=<pid> and exit once that process has ended;
release builds ignore it. The integration tests start the binary through
one helper that sets it to the test's own pid, and leak-test.sh and
agent-conformance.sh set it to the script's.

inbox_exit.rs files a card in a scratch home and checks the inbox exits
once a stand-in parent process ends, once the home is deleted, and once
the database is deleted.

Co-Authored-By: Claude <[email protected]>
… remade one

The exit checks ran only after a second with no request, so a client
asking more often than that kept a deleted home's inbox up. They now run
once a second whether requests arrive or not.

The inbox records its database's device and inode at start and exits when
they change, so a home deleted and made again by another `need` before the
next check no longer leaves the old inbox holding the port with the old
proof key.

The exit tests in inbox_exit.rs now send a /verify every 100 ms while they
wait, and the_inbox_exits_once_its_home_is_made_again checks that the old
inbox goes and the next `need` gets a link from a new one. The debug-only
helpers there are compiled only in debug builds, and init_mode.rs no
longer imports `Command`, which only its Linux test used.

Co-Authored-By: Claude <[email protected]>
@krishhgg
krishhgg force-pushed the fix/test-inboxes-exit branch from ced4313 to c89774f Compare October 6, 2026 05:29
Comment thread crates/cli/tests/inbox_exit.rs Outdated
A reply that failed or stalled used to count as an exit, so a stuck inbox
holding the port could pass. The test now keeps asking until the
connection is refused.

Co-Authored-By: Claude <[email protected]>
@krishhgg
krishhgg merged commit adb3342 into main Oct 6, 2026
4 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