Skip to content

fix(minimald): correct command over ssh exec for package build - #984

Merged
twitchyliquid64 merged 1 commit into
mainfrom
tom/completions
Jul 27, 2026
Merged

fix(minimald): correct command over ssh exec for package build#984
twitchyliquid64 merged 1 commit into
mainfrom
tom/completions

Conversation

@twitchyliquid64

@twitchyliquid64 twitchyliquid64 commented Jul 27, 2026 •
edited by macroscopeapp Bot
Loading

Copy link
Copy Markdown
Member

Forgot the ssh exec path in #976

Note

Fix SSH exec routing in minimald to accept min package build instead of min build

Updates handle_exec in exec.rs to match the package build subcommand form rather than the bare build form. Unknown or malformed package subcommands are now rejected with a warning before the channel is acknowledged. Risk: any SSH exec callers still using the old min build form will be rejected.

Macroscope summarized 047a335.

@coderabbitai

coderabbitai Bot commented Jul 27, 2026 •
edited
Loading

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The SSH exec handler now accepts min package build requests, rejects unsupported package forms before channel acknowledgment, forwards build arguments to the existing runner, and updates related documentation and logging text.

Changes

Package build exec routing

Layer / File(s) Summary
Validate and route package build requests
crates/minimald/src/exec.rs		 handle_exec recognizes only the min package build form, parses its arguments, and routes them to run_build_exec; related documentation and channel-write warnings use the new command name.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: bryan-minimal

Poem

A rabbit hops where build commands flow,
“Package build!” makes the channels glow.
Wrong paths stop before success is spun,
While args race onward, one by one.
The logs now name the work they’ve done!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description covers the summary but omits the required Testing and Checklist sections from the template. Add a Testing section with commands/results and complete the Checklist, including docs updates and any BREAKING CHANGE footer if needed.
✅ Passed checks (4 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title is clear, concise, and matches the main SSH exec routing fix for package build.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

Comment @coderabbitai help to get the list of available commands.

bryan-minimal
bryan-minimal approved these changes Jul 27, 2026
View reviewed changes

@bryan-minimal bryan-minimal left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@twitchyliquid64
twitchyliquid64 enabled auto-merge (squash) July 27, 2026 19:53
coderabbitai[bot]
coderabbitai Bot reviewed Jul 27, 2026
View reviewed changes

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
crates/minimald/src/exec.rs (1)

859-875: 🩺 Stability & Availability | 🔵 Trivial

Run the daemon integration coverage for this SSH routing change.

Please run just e2e and/or just test-vm, including the applicable root-integration harnesses; do not rely only on unit tests for channel acknowledgment and VM/networking behavior.

As per coding guidelines, daemon/VM changes require the repository’s relevant integration coverage and root-integration harnesses.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/minimald/src/exec.rs` around lines 859 - 875, Run the applicable
daemon integration coverage for the SSH routing change, including just e2e
and/or just test-vm and the relevant root-integration harnesses. Verify channel
acknowledgment and VM/networking behavior through these integration tests rather
than relying solely on unit tests.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@crates/minimald/src/exec.rs`:
- Around line 859-875: Run the applicable daemon integration coverage for the
SSH routing change, including just e2e and/or just test-vm and the relevant
root-integration harnesses. Verify channel acknowledgment and VM/networking
behavior through these integration tests rather than relying solely on unit
tests.
ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 777df59a-b599-4f8d-9e19-6cd5103229e3

📥 Commits

Reviewing files that changed from the base of the PR and between b1cde76 and 047a335.

📒 Files selected for processing (1)
  • crates/minimald/src/exec.rs

@twitchyliquid64
twitchyliquid64 merged commit 09a4697 into main Jul 27, 2026
29 checks passed
@twitchyliquid64
twitchyliquid64 deleted the tom/completions branch July 27, 2026 20:35
@coderabbitai coderabbitai Bot mentioned this pull request Jul 28, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@coderabbitai coderabbitai[bot] coderabbitai[bot] left review comments
@bryan-minimal bryan-minimal bryan-minimal approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@twitchyliquid64 @bryan-minimal