Comment the chat approval construction site to say the full argv is attached downstream
Maintainers usually reply within 2 days
Nobody has claimed this yet.
Assessment
- Difficulty
- 1/5
- Estimated time
- Under an hour
- Newbie friendliness
- 88/100
Research direction
Start at the ChatRocmCommandAction::Approval construction site and inspect how display_command is passed to the approval body. Add the requested comment explaining that the full argv is attached downstream via display_command and rendered in the approval body, without changing behavior.
Written by the indexing model from the issue text.
Description
Small readability fix, raised during review of #224 as a recurrence note after the reviewer reached a wrong conclusion from this code and had to withdraw it.
Problem
ChatRocmCommandAction::Approval is constructed with a short pending_title (for example "Install driver"). Read at that site alone, it looks as though the human approving a privileged command sees only that title.
That is not what happens — the same construction attaches display_command: Some(format_structured_tool_call("rocm", &args)), and the approval body renders the full argv. But the code that consumes it and renders the approval sits several hundred lines away, so following the value takes real effort.
A reviewer did read it that way during #224, filed a blocking finding on the strength of it, and had to withdraw the claim after tracing the value properly. That is a cheap mistake to make and it will be made again.
Suggested fix
One line at the construction site saying the full command line travels with the approval via display_command and is rendered in the approval body. No behaviour change.
Out of scope
The separate, genuine observation from that review still stands and is not covered here: the download URL and the fact that the package is installed as root are not surfaced in the approval body. That is existing behaviour and deserves its own issue if anyone wants to change it.
- Dominant language
- Rust
- Stars
- 41
- Forks
- 10
- Avg merge
- 5d 14h
- Merged PRs (30d)
- 87
Getting set up
- No Dockerfile or Docker Compose file
- Has a pull request template
- Read the contributing guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from ROCm/rocm-cli
-
Move the remaining `scripts/` tooling to Rust (`cargo xtask` / e2e scenarios)Possibly taken A pull request linked to this issue is open or already merged. Open
Difficulty 2/5 Under an hour Newbie friendliness 85/100
Maintainers usually reply within 2 days
-
serve: the post-launch smoke test spins forever at 100% CPU if the engine closes the connection mid-responsePossibly taken @rominf claimed this 3 days ago. Openbug
Difficulty 2/5 1-3 hours Newbie friendliness 85/100
ROCm/rocm-cli#514 · 1 comment ·
Maintainers usually reply within 2 days
-
examine: lspci cannot name a GPU that pci.ids does not know, though the device id is on the lineOpenbug
Difficulty 2/5 1-3 hours Newbie friendliness 68/100
Maintainers usually reply within 2 days
-
diagnose: a line reading "blacklistamdgpu" is treated as blacklisting amdgpuPossibly taken A pull request linked to this issue is open or already merged. Openbug
Difficulty 2/5 1-3 hours Newbie friendliness 76/100
Maintainers usually reply within 2 days
-
bug
Difficulty 2/5 1-3 hours Newbie friendliness 84/100
Maintainers usually reply within 2 days
Similar issues
-
Difficulty 1/5 Under an hour Newbie friendliness 75/100
ajeetraina/awesome-docker-sbx#220 ·
-
`helios / deploy`: switch zone wait in `deploy.sh` has almost no headroom over healthy startup timesOpenTest Flake
Difficulty 2/5 1-3 hours Newbie friendliness 74/100
oxidecomputer/omicron#11453 ·
Maintainers usually reply within 1 day
-
Difficulty 2/5 1-3 hours Newbie friendliness 88/100
Maintainers usually reply within 1 day
-
[Feature] 设置里面的同步功能Openenhancement user-priority/P2
Difficulty 2/5 1-3 hours Newbie friendliness 65/100
Maintainers usually reply within 1 day
-
agent:triaged bug bughunt pm:npm priority:p1
Difficulty 2/5 1-3 hours Newbie friendliness 85/100
SocketDev/socket-patch#1127 · 1 comment ·
Maintainers usually reply within 1 day