Skip to content

use an enum to define job streams#101

Open
emilyalbini wants to merge 1 commit intoea-mvulumtytlotfrom
ea-ptqwnqpswsuv
Open

use an enum to define job streams#101
emilyalbini wants to merge 1 commit intoea-mvulumtytlotfrom
ea-ptqwnqpswsuv

Conversation

@emilyalbini
Copy link
Copy Markdown
Member

Note: This PR is stacked on top of #100.


This PR refactors how we handle job stream names, replacing the string handling we do all over the place with a common JobStream enum. My goal for this is that the compiler should guide you through its errors when adding a new job stream, rather than having to search through the codebase for places where the stream should be handled.

I chose for now to not have the server error out when an unknown stream is received, but that should be an easy change if we want to go for it.

@emilyalbini emilyalbini requested a review from jclulow April 20, 2026 08:55
Comment thread agent/src/exec.rs
Comment on lines -66 to +70
"error",
JobStream::Error,
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is the only occurrence of error as a job stream, and nothing else in the codebase handled it. Should this be panic instead?

Comment thread agent/src/main.rs
.worker_append()
.body(vec![WorkerAppend {
stream: "agent".into(),
stream: JobStream::Agent.to_string(),
Copy link
Copy Markdown
Member Author

@emilyalbini emilyalbini Apr 20, 2026

Choose a reason for hiding this comment

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

Found another stream we don't handle anywhere else: agent seems to be only used when spawning a job fails. Maybe we should use panic instead? Or task/diag.{name} depending on what failed to spawn?

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