Skip to content

feat: let ordinary users watch tracked commands - #137

Merged
jcreedcmu merged 2 commits into
mainfrom
jcreed/verso-pub-1
Sep 14, 2026
Merged

jcreedcmu merged 2 commits into
mainfrom
jcreed/verso-pub-1

Conversation

@jcreedcmu

Copy link
Copy Markdown
Collaborator

Change tracked commands from admin-only to available to arbitrary logged in users.

A tracked command now records a TrackedCommandOwner, and a second route streams user-owned commands to the user who started them. The two routes share the server-sent-event body, which moves to trackedCommandStream.ts. Components now have a scope prop which can be 'admin' or 'user'.

Change tracked commands from admin-only to available to arbitrary logged in users.

A tracked command now records a TrackedCommandOwner, and a second
route streams user-owned commands to the user who started them. The
two routes share the server-sent-event body, which moves to
trackedCommandStream.ts. Components now have a `scope` prop which can
be 'admin' or 'user'.

@robsimmons robsimmons left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think the interface here makes sense, but I'm worried about the implementation — if user command keys don't incorporate the user's id, then the assumption that you can still view the stream for a given key if you could view that key's stream at some point in the past before breaks down.

My proposal would be to change the implementation so that each user has their own user-level key -> commandstate mapping, and have the admin commands exist in their own administrative key -> commandstate map. (Two users should be able to run a publish command simultaneously, but I think we'd rather not let two administrators install a toolchain simultaneously.)

(Aside: the robots that pester me about my own PRs are generally very vocal about untrusted users being able to create unbounded logs, and that's maybe also a concern here — user tracked command streams may need to get truncated or cleaned up somehow, that doesn't need to happen right away but seemed worth noting.)

@jcreedcmu

Copy link
Copy Markdown
Collaborator Author

This proposal sounds good to me. Out of band you were questioning whether the user/admin distinction should exist as command intent vs. it being autodetected. Do you still have feelings about that?

My default sense is to prefer capturing "this is a command that I mean to run as admin, [and with your proposal therefore] in the admin running-task-name-space, do confirm that I have permission to do this" rather than silently, like, escalate commands to admin-ness just because I have admin rights at the moment. I think there may exist situations where the admin user wants to take actions that don't necessarily carry all admin capabilities with them.

@robsimmons

robsimmons commented Sep 14, 2026

Copy link
Copy Markdown
Collaborator

My default sense is to prefer capturing "this is a command that I mean to run as admin, [and with your proposal therefore] in the admin running-task-name-space, do confirm that I have permission to do this" rather than silently, like, escalate commands to admin-ness just because I have admin rights at the moment.

I agree with your default stance — you are correct I briefly disagreed with this stance, but then I talked myself out of that disagreement in favor of the "request to put stuff in admin namespace" interface. If there's a separate "admin namespace" of tracked commands, it makes sense to say "I'm requesting to put this task in the admin namespace". Users can generally only put stuff in their own namespace. That's all very in line with the design of project namespacing which is nice.

@robsimmons robsimmons left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks great!

@jcreedcmu
jcreedcmu merged commit 7239ee5 into main Sep 14, 2026
1 check 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.

2 participants