chore(cmd): consolidate boolean params into options structs

[boneskull]

Summary

Implements #26. Functions like doCascadeWithState and doSubmitPushAndPR had accumulated so many boolean parameters that call sites were unreadable without counting argument positions.

• CascadeOptions — replaces 8 positional params on doCascadeWithState (DryRun, Operation, UpdateOnly, Web, PushOnly, AllBranches, StashRef, Worktrees)
• SubmitOptions — replaces 4 boolean params on doSubmitPushAndPR and doSubmitPRs (DryRun, UpdateOnly, OpenWeb, PushOnly)
• prContext — bundles the 6 shared read-only params threaded through createPRForBranch, adoptExistingPR, and adoptExistingPRDirect

All callers updated across cascade.go, submit.go, sync.go, and continue.go.

Closes #26

Test plan

• All existing unit and e2e tests pass (make test)
• Linter clean (make lint)

[claude]

Claude finished @boneskull's task in 1m 37s —— View job

---

Code Review

• Gather context and read changed files
• Analyze diff and review code changes
• Post review feedback

---

This is a solid readability improvement — the call sites for doCascadeWithState are dramatically cleaner, and the documentation in CascadeOptions does a good job flagging which fields are submit-specific. A few observations below.

---

Minor Issues

1. Web vs OpenWeb naming inconsistency

CascadeOptions uses Web for the flag, while SubmitOptions uses OpenWeb. They represent the same --web flag. The asymmetry becomes visible at the mapping site in continue.go:149:

OpenWeb: st.Web,  // CascadeOptions.Web → SubmitOptions.OpenWeb

This will trip up anyone reading the field side-by-side. Pick one name and use it in both structs. Since OpenWeb is more descriptive (it clarifies what opens in the web), I'd suggest renaming CascadeOptions.Web → CascadeOptions.OpenWeb. Fix this →

---

2. Partial adoption of prContext in doSubmitPRs

pCtx is constructed and passed to createPRForBranch, but the update-existing-PR branch in doSubmitPRs still uses raw locals directly (e.g. submit.go:316–331):

// uses ghClient, root, trunk, remoteBranches, s — not pCtx.*
if err := ghClient.UpdatePRBase(existingPR, parent); err != nil { ... }
if err := ghClient.GenerateAndPostStackComment(root, b.Name, trunk, existingPR, remoteBranches); err != nil { ... }
maybeMarkPRReady(ghClient, existingPR, b.Name, parent, trunk, s)

This is inconsistent: the create path uses pCtx, the update path doesn't. Since the raw variables and pCtx.* refer to the same data it's not a correctness problem, but it undermines the point of prContext. Either go through pCtx consistently, or consider restructuring the update-existing-PR section into a helper that also accepts prContext.

---

3. ctx parameter name conflicts with Go context.Context convention

func createPRForBranch(g *git.Git, ctx prContext, branch, base string) (int, bool, error) {
func adoptExistingPR(ctx prContext, branch, base string) (int, error) {
func adoptExistingPRDirect(ctx prContext, branch, base string, existingPR *github.PR) (int, error) {

ctx is the universal convention for context.Context in Go. Readers will do a double-take every time they see ctx.ghClient. The call site already uses pCtx — consider using that as the parameter name inside the functions too.

---

Nits

• CascadeOptions.AllBranches []string is stored in state as st.Branches, so the struct field name is slightly inconsistent with the state field it shadows. Not a blocker, but worth noting.
• maybeMarkPRReady / promptMarkPRReady still take raw params rather than prContext. Not wrong, but it means those helpers are one step behind the refactoring.

---

Summary

The core refactoring is clean and clearly motivated. The CascadeOptions doc block is particularly nice — calling out the submit-specific fields explicitly prevents future confusion. The items above are all minor and none blocks merging, but the Web/OpenWeb rename (item 1) and consistent pCtx usage (item 2) would make the code cleaner if addressed before merge.

[claude]

Claude finished @boneskull's task in 1m 32s —— View job

---

Code Review

• Gather context
• Read changed files
• Analyze diff against previous review feedback
• Post review

---

All three items from the previous review have been addressed in this update. Here's the rundown:

1. Web → OpenWeb rename ✅

CascadeOptions.Web is now CascadeOptions.OpenWeb, matching SubmitOptions.OpenWeb. The state round-trip (opts.OpenWeb → st.Web → CascadeOptions.OpenWeb) is correct, and the asymmetry at continue.go:150 is now just the expected name mismatch between prContext and the legacy state.CascadeState struct field, which is clearly annotated.

2. Consistent pCtx usage in the update-existing-PR branch ✅

doSubmitPRs now goes through pCtx.* uniformly for both the create and update paths (submit.go:315–331). The one call to s.WarningIcon() that remains before pCtx is constructed (submit.go:284) is correct — pCtx doesn't exist yet at that point.

3. ctx → pCtx parameter rename ✅

All three helpers (createPRForBranch, adoptExistingPR, adoptExistingPRDirect) now use pCtx as the parameter name, eliminating the context.Context collision.

---

One remaining minor observation

maybeMarkPRReady and promptMarkPRReady still take raw params rather than prContext — e.g. submit.go:331:

maybeMarkPRReady(pCtx.ghClient, existingPR, b.Name, parent, pCtx.trunk, pCtx.s)

Not a blocker. These are small leaf helpers and widening their signatures to accept prContext would be a separate step. Worth a follow-up issue if you want the refactoring to be exhaustive, but it doesn't need to hold up this PR.

---

Summary

All substantive feedback from the first review has been incorporated cleanly. The PR is ready to merge from a code quality standpoint.