Skip to content

feat(sessions): Add session command feature#38

Merged
pthurlow merged 2 commits intomainfrom
feat/sessions
Apr 10, 2026
Merged

feat(sessions): Add session command feature#38
pthurlow merged 2 commits intomainfrom
feat/sessions

Conversation

@pthurlow
Copy link
Copy Markdown
Collaborator

No description provided.

@sentry

This comment was marked as outdated.

Comment thread src/main.rs
Comment thread src/api.rs
Comment thread src/sessions.rs
claude[bot]

This comment was marked as resolved.

Comment thread src/sessions.rs
.env("HOTDATA_WORKSPACE", workspace_id)
.status();

match status {
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.

P2 — Signal exit code lost.

s.code() returns None when the child is killed by a signal (e.g. SIGINT from Ctrl-C), so the wrapper exits 1 instead of the conventional 128+signal. Scripts that inspect the exit code (e.g. if [ $? -eq 130 ]) will misbehave.

On Unix, the idiomatic fix is to re-raise the signal so the parent shell sees the process as signal-killed:

Suggested change
match status {
Ok(s) => {
#[cfg(unix)]
if let Some(sig) = std::os::unix::process::ExitStatusExt::signal(&s) {
// Re-raise so the parent shell sees a signal-killed exit
unsafe { libc::raise(sig) };
}
std::process::exit(s.code().unwrap_or(1))
}

If pulling in libc is undesirable, exiting with 128 + signal is the next-best option.

Comment thread src/sessions.rs
Comment on lines +107 to +110
let name = proc.name().to_string_lossy();
if name == "hotdata" {
if proc.cmd().iter().any(|a| a == "sessions")
&& proc.cmd().iter().any(|a| a == "run")
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.

P2 (theoretical) — Arg matching is position-independent.

any(|a| a == "sessions") && any(|a| a == "run") matches any hotdata process that has both tokens anywhere in its argv — e.g. hotdata sessions new --name run. In practice none of the other sessions subcommands spawn child hotdata processes, so this can't actually trigger a false positive today. But it's fragile: a future subcommand that accepts free-form string args and spawns children would silently break.

Prefer a positional check on the subcommand slots:

Suggested change
let name = proc.name().to_string_lossy();
if name == "hotdata" {
if proc.cmd().iter().any(|a| a == "sessions")
&& proc.cmd().iter().any(|a| a == "run")
if name == "hotdata" {
let args: Vec<_> = proc.cmd().iter().skip(1).collect(); // skip argv[0]
if args.first().map(|a| a.as_encoded_bytes()) == Some(b"sessions")
&& args.get(1).map(|a| a.as_encoded_bytes()) == Some(b"run")
{
return Some(pid);
}
}

@pthurlow pthurlow merged commit 39a3376 into main Apr 10, 2026
9 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