refactor(op): move init and update code into op crate by twitchyliquid64 · Pull Request #592 · gominimal/minimal

twitchyliquid64 · 2026-06-29T04:54:37Z

Moves the logic for minimal init and minimal update into the op crate, generalized by the ProjectOp trait.

This enables:

Tests
Being able to do these operations in minimald

Summary by CodeRabbit

New Features
- Streamlined project setup for creating and updating minimal.toml.
- Initialization now auto-detects the project stack and generates the configuration and output path.
- Updates now refresh upstream/sideload pins and return a structured report of changes.
Refactor
- Initialization and update commands were refactored to share centralized project setup and operations.
- Init confirmation preview now reflects the generated TOML content; accepts yes in addition to y.
Bug Fixes
- Improved error handling for project operations with clearer error mapping.

coderabbitai · 2026-06-29T04:54:51Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 067d2dbb-03b1-4375-8340-350f7cc1641a

📥 Commits

Reviewing files that changed from the base of the PR and between 370b5f1 and 5b81b6e.

⛔ Files ignored due to path filters (1)

Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (11)

crates/decode/src/lib.rs
crates/mctx/src/error.rs
crates/mctx/src/lib.rs
crates/mctx/src/project_setup.rs
crates/minimal/src/cmd_init.rs
crates/minimal/src/cmd_update.rs
crates/op/Cargo.toml
crates/op/src/lib.rs
crates/op/src/project/init.rs
crates/op/src/project/mod.rs
crates/op/src/project/update.rs

✅ Files skipped from review due to trivial changes (1)

crates/op/Cargo.toml

🚧 Files skipped from review as they are similar to previous changes (10)

crates/mctx/src/error.rs
crates/decode/src/lib.rs
crates/op/src/lib.rs
crates/minimal/src/cmd_update.rs
crates/mctx/src/project_setup.rs
crates/mctx/src/lib.rs
crates/op/src/project/mod.rs
crates/minimal/src/cmd_init.rs
crates/op/src/project/init.rs
crates/op/src/project/update.rs

📝 Walkthrough

Walkthrough

Adds shared project-operation traits and a ProjectSetup environment, then refactors init/update commands to use InitProject and UpdateProject. The op crate exports the new operation types, mctx wires the environment and error conversion, and decode re-exports an additional stack predicate.

Changes

Project Init/Update Abstraction

Layer / File(s)	Summary
Project operation contracts `crates/op/src/project/mod.rs`, `crates/op/src/lib.rs`, `crates/op/Cargo.toml`, `crates/decode/src/lib.rs`	Defines `ProjectEnv` and `ProjectOp`, wires `project` module exports, adds `toml_edit`, and re-exports `PackageMatcherPredicate`.
InitProject operation `crates/op/src/project/init.rs`	Implements `InitProject::run`, stack matching, TOML generation, and init tests for matching and rendering.
UpdateProject operation `crates/op/src/project/update.rs`	Implements `UpdateProject::run`, `RevChange`, `UpdateReport`, pin refresh, TOML rewriting, and return handling.
ProjectSetup in mctx `crates/mctx/src/project_setup.rs`, `crates/mctx/src/lib.rs`, `crates/mctx/src/error.rs`	Adds `ProjectSetup`, repo-dir resolution, `ProjectEnv` methods, `Context::project_setup`, and `From<op::Error> for Error`.
Command delegation `crates/minimal/src/cmd_init.rs`, `crates/minimal/src/cmd_update.rs`	Replaces inline init/update logic with `ProjectSetup` plus `InitProject`/`UpdateProject`, and updates confirmation/output handling.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

gominimal/minimal#525: Touches the same mctx error-conversion surface and another From<...> for Error mapping pattern.

Suggested labels

needs-spec

Suggested reviewers

norrietaylor

Poem

🐇 I hopped through traits and tidy code,
New project paths now find their road.
Init and update share one tune,
With plans and reports beneath the moon,
minimal.toml twinkles bright,
And bunny ears applaud tonight. 🥕

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main refactor of moving init and update logic into the op crate.
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.

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

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

coderabbitai

Actionable comments posted: 3

🧹 Nitpick comments (1)

crates/mctx/src/error.rs (1)

222-225: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Preserve the inner op::Error::Other error instead of stringifying it.

The fallback converts the error into a new formatted anyhow::Error, which loses the original source chain. Match op::Error::Other(e) explicitly and pass e through.

Suggested change

         match value {
             op::Error::IO(e) => Self::IO("op", PathBuf::new(), e),
             op::Error::Plan(graph, e) => Self::Plan(Box::new((graph, e))),
+            op::Error::Other(e) => Self::Other(e),
             other => Self::Other(anyhow::anyhow!("{}", other)),
         }

🤖 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/mctx/src/error.rs` around lines 222 - 225, The fallback arm in the
error conversion currently stringifies the inner error and discards its source
chain; update the match in the error mapping logic to handle op::Error::Other
explicitly instead of using the generic other case. Preserve the original error
by passing the inner value through directly in the same conversion path used by
the other op::Error variants, alongside the existing IO and Plan handling.

🤖 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.

Inline comments:
In `@crates/minimal/src/cmd_init.rs`:
- Around line 33-36: The confirmation check in cmd_init should accept "yes" as a
valid affirmative input in addition to "y" and empty input. Update the input
validation logic in the init command flow (the block using input.trim() and
eprintln!("Aborted.")) so that any case-insensitive "yes" value is treated as
confirmation and does not trigger the abort path.

In `@crates/op/src/project/init.rs`:
- Around line 53-61: The re-init path in init() is rebuilding the graph from
env.repo_dir() instead of reusing the upstream already stored in minimal.toml,
which can cause graph_from_chain() to resolve the wrong source. Update the
existing branch in crates/op/src/project/init.rs so it preserves and reuses the
upstream link from the current configuration rather than forcing a new
LinkConfig::Dir from the repo directory, and keep the existing origin/file_path
flow intact.

In `@crates/op/src/project/update.rs`:
- Around line 126-129: The migration in update.rs unconditionally overwrites any
existing stack entry when moving data from harness to stack. Update the project
update logic in the update path so the harness migration only populates stack
when stack is absent, or otherwise merges without replacing existing stack
contents. Use the existing update flow around doc.remove("harness") and
report.migrated_harness to preserve current stack config during minimal update.

---

Nitpick comments:
In `@crates/mctx/src/error.rs`:
- Around line 222-225: The fallback arm in the error conversion currently
stringifies the inner error and discards its source chain; update the match in
the error mapping logic to handle op::Error::Other explicitly instead of using
the generic other case. Preserve the original error by passing the inner value
through directly in the same conversion path used by the other op::Error
variants, alongside the existing IO and Plan handling.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f1ae8588-2165-4ca3-ad1a-bf7bf2994429

📥 Commits

Reviewing files that changed from the base of the PR and between 98b7ae9 and f4cbef0.

⛔ Files ignored due to path filters (1)

Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (11)

crates/decode/src/lib.rs
crates/mctx/src/error.rs
crates/mctx/src/lib.rs
crates/mctx/src/project_setup.rs
crates/minimal/src/cmd_init.rs
crates/minimal/src/cmd_update.rs
crates/op/Cargo.toml
crates/op/src/lib.rs
crates/op/src/project/init.rs
crates/op/src/project/mod.rs
crates/op/src/project/update.rs

coderabbitai

Actionable comments posted: 1

🤖 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.

Inline comments:
In `@crates/op/src/project/update.rs`:
- Around line 83-85: The update flow in `update()` advances checkout state too
early by calling `env.update_checkouts()` before the pin rewrite in the
read/parse/write path is guaranteed. Change the logic so
`update_checkouts()`/`checkout_branch()` only resolve the new heads without
mutating state, or add rollback handling if the persistence step fails after
`minimal.toml` is updated. Keep the checkout mutation and the pin rewrite in
`update()` consistent so a failed write does not leave the project half-updated.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b86638d5-750c-47a9-a4d9-c5e403acc9f4

📥 Commits

Reviewing files that changed from the base of the PR and between f4cbef0 and 370b5f1.

⛔ Files ignored due to path filters (1)

Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (11)

crates/decode/src/lib.rs
crates/mctx/src/error.rs
crates/mctx/src/lib.rs
crates/mctx/src/project_setup.rs
crates/minimal/src/cmd_init.rs
crates/minimal/src/cmd_update.rs
crates/op/Cargo.toml
crates/op/src/lib.rs
crates/op/src/project/init.rs
crates/op/src/project/mod.rs
crates/op/src/project/update.rs

🚧 Files skipped from review as they are similar to previous changes (10)

crates/decode/src/lib.rs
crates/op/src/project/mod.rs
crates/mctx/src/error.rs
crates/op/src/lib.rs
crates/mctx/src/lib.rs
crates/op/Cargo.toml
crates/mctx/src/project_setup.rs
crates/minimal/src/cmd_update.rs
crates/minimal/src/cmd_init.rs
crates/op/src/project/init.rs

coderabbitai · 2026-06-29T16:16:18Z

+        // best effort, yeet any cached remote index so a fresh fetch occurs
+        env.invalidate_remote_index();
+        env.update_checkouts()?;


🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Avoid mutating checkouts before the pin rewrite is guaranteed.

update_checkouts() / checkout_branch() run before the read/parse/write path at Lines 115-149. If that persistence step fails, the checkout state has already advanced while minimal.toml still points at the old commits, leaving the project half-updated. This needs either a non-mutating way to resolve the new heads first, or a rollback path when persisting the new pins fails.

Also applies to: 115-149

🤖 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/op/src/project/update.rs` around lines 83 - 85, The update flow in `update()` advances checkout state too early by calling `env.update_checkouts()` before the pin rewrite in the read/parse/write path is guaranteed. Change the logic so `update_checkouts()`/`checkout_branch()` only resolve the new heads without mutating state, or add rollback handling if the persistence step fails after `minimal.toml` is updated. Keep the checkout mutation and the pin rewrite in `update()` consistent so a failed write does not leave the project half-updated.

twitchyliquid64 requested review from 0chroma, evanspearman, jtnkminimal and norrietaylor June 29, 2026 04:54

coderabbitai Bot reviewed Jun 29, 2026

View reviewed changes

Comment thread crates/minimal/src/cmd_init.rs Outdated

Comment thread crates/op/src/project/init.rs

Comment thread crates/op/src/project/update.rs Outdated

twitchyliquid64 enabled auto-merge (squash) June 29, 2026 05:39

twitchyliquid64 force-pushed the tom/init-op branch from f4cbef0 to 370b5f1 Compare June 29, 2026 16:13

coderabbitai Bot reviewed Jun 29, 2026

View reviewed changes

twitchyliquid64 added 2 commits June 29, 2026 09:38

refactor(op): move init and update code into op crate

61f31bf

chore(update): rip out old harness->stack migration logic

5b81b6e

twitchyliquid64 force-pushed the tom/init-op branch from 370b5f1 to 5b81b6e Compare June 29, 2026 16:38

evanspearman approved these changes Jun 29, 2026

View reviewed changes

twitchyliquid64 merged commit 229f655 into main Jun 29, 2026
23 checks passed

twitchyliquid64 deleted the tom/init-op branch June 29, 2026 17:38

Navigation Menu

Uh oh!

refactor(op): move init and update code into op crate - #592

refactor(op): move init and update code into op crate#592
twitchyliquid64 merged 2 commits into
mainfrom
tom/init-op

twitchyliquid64 commented Jun 29, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jun 29, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested labels

Suggested reviewers

Poem

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

coderabbitai Bot Jun 29, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

twitchyliquid64 commented Jun 29, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jun 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested labels

Suggested reviewers

Poem

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot Jun 29, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

twitchyliquid64 commented Jun 29, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jun 29, 2026 •

edited

Loading