[GeoffChurch]

Issue Link

Discussion #440

AI Policy Ack

ack

Drafted with vibes, edited and tested with meat

This PR was:

• mostly or completely vibe coded
• mostly or completely meat coded
• bit of both

Description

Adds window_title to the output of shpool list --json:

{
  "sessions": [
    {
      "name": "djt-miner",
      "started_at_unix_ms": 1790885744638,
      "last_connected_at_unix_ms": 1791385820043,
      "last_disconnected_at_unix_ms": 1791383990390,
      "status": "Disconnected",
      "shell_pid": 51725,
      "attachments": [],
      "window_title": "Coins mined: 23.203 ($0.00032). Level: patriot"
    }
  ]
}

The use case is letting downstream tools show the user the window titles of their sessions. Many processes (shells, coding assistants, etc) send status information to the window title, which is traditionally visible in a terminal window/tab but in shpool is inaccessible without attaching to the session.

This change exposes the current title through shpool list --json without attaching.

[ethanpailes]

I think you'll need to pull in shpool-vterm 0.2.7 (shell-pool/shpool-vterm#12)

[ethanpailes]

I'd like to take a different approach to avoid double-storing the same data.

[ethanpailes]

I just bumped the shpool-vterm version to 0.2.8, so you should be able to rebase and impl

[ethanpailes]

Draining other incoming requests immediately seems overly complicated. Let's just handled one at a time and use the top level dispatch loops for all incoming requests.

[GeoffChurch]

Done.

Or, maybe we could at least continue; after shell.rs:541 (like maybe_switch does on shell.rs:523), which would let it handle bursts without the hacky inner loop. Burstiness is the typical case in my workflow since I have k running instances of the TUI app, and they all call shpool list --json at the same time to refresh their view whenever the events channel sends anything.

[ethanpailes]

I still the see the requests chan being passed in, which is confusing. It looks like it is only being used to hang up on error. Let's instead accept a hang_up closure or just bubble up the error and have the caller handle it. Right now you have to read the implementation to see that we are only handling one request because the args still hint at more being handled.

[GeoffChurch]

Done - 6d374b1 lifts the handling to the caller.

Re adding a continue; to reduce latency in case of bursts, is that fine?

[ethanpailes]

I'm not sure what you mean about adding continue. I don't see one in that patch. I'd rather not overcomplicate it. Let's just handle one request per loop until we observe a problem with that approach.

[GeoffChurch]

That was in reference to my comment from earlier in the thread.

I didn't include the continue in the patch, but I was asking because, given my workflow with K TUI app instances, each event on the events channel triggers K shpool list --json calls, which will lead to titles being omitted whenever the 500ms RPC timeout is exceeded (each request takes ~50-60ms and the timeout is 500ms, so I can only scale to K≈10)

[ethanpailes]

Oh, I see, you're saying add a continue after handle_fetch_metadata. Yeah that seems fine.

[GeoffChurch]

Done (a028fe7). Thank you🙏

[ethanpailes]

Looks like you have fmt issues.

[GeoffChurch]

Looks like you have fmt issues.

Done (ded4fce) - my nightly toolchain was old.

[ethanpailes]

fmt is still broken, can you fix that?

[GeoffChurch]

fmt is still broken, can you fix that?

Done (1128b16). These comments were already on master so the CI runner probably just updated its rustfmt

[ethanpailes]

FYI: for this project, with very rare exceptions PRs should be 1-1 with commits, so please just edit your commit in place rather than making a chain I'll need to manually squash.

Looks like fmt is still broken. You probably need to make sure you are matching your fmt to the one running in CI, which you can figure out from the CI config files. You may want to run rustup update nightly or something like that.

[GeoffChurch]

FYI: for this project, with very rare exceptions PRs should be 1-1 with commits, so please just edit your commit in place rather than making a chain I'll need to manually squash.

Looks like fmt is still broken. You probably need to make sure you are matching your fmt to the one running in CI, which you can figure out from the CI config files. You may want to run rustup update nightly or something like that.

Okay, I squashed the commits. The CI failed on config_watcher::test::debounce, which looks like it's just flaky since it failed after comment changes.