diff --git a/QUICKSTART.md b/QUICKSTART.md index b4c93e7..29e3b3d 100644 --- a/QUICKSTART.md +++ b/QUICKSTART.md @@ -166,6 +166,10 @@ dub repo # current repository GitHub page When feedback lands on a middle branch: ```bash +# from anywhere in the same stack, target that branch directly +dub m --into feat/auth-login -a -m "fix: address review feedback" + +# or switch first, then modify in-place dub co feat/auth-login # amend current commit diff --git a/README.md b/README.md index 2211363..de576c9 100644 --- a/README.md +++ b/README.md @@ -205,6 +205,9 @@ dub modify -vv # interactive rebase of this branch's commits dub modify --interactive-rebase + +# apply changes to an older branch in the current stack +dub modify --into feat/auth-login -a -m "fix: address feedback" ``` Flags: @@ -216,6 +219,7 @@ Flags: - `-m, --message ` (repeatable) - `-v, --verbose` (repeatable) - `--interactive-rebase` +- `--into ` (modify another branch in current stack and restack descendants) ### `dub checkout` / `dub co` diff --git a/apps/docs/content/docs/commands/modify.mdx b/apps/docs/content/docs/commands/modify.mdx index 28748f9..34532a3 100644 --- a/apps/docs/content/docs/commands/modify.mdx +++ b/apps/docs/content/docs/commands/modify.mdx @@ -26,6 +26,9 @@ dub modify -vv # Interactive rebase of this branch's commits dub modify --interactive-rebase + +# Apply changes to an older branch in the current stack +dub modify --into feat/auth-login -a -m "fix: address feedback" ``` ## Flags @@ -40,6 +43,7 @@ dub modify --interactive-rebase | `-m, --message ` | Commit message (repeatable) | | `-v, --verbose` | Show diff before modify (repeatable: `-vv` for staged+unstaged) | | `--interactive-rebase` | Interactive rebase of branch commits | +| `--into ` | Modify another branch in the current stack, then restack descendants | **Aliases:** `dub m` diff --git a/docs/reports/2026-03-13-ds-3wn-autoship-report.md b/docs/reports/2026-03-13-ds-3wn-autoship-report.md new file mode 100644 index 0000000..51da84a --- /dev/null +++ b/docs/reports/2026-03-13-ds-3wn-autoship-report.md @@ -0,0 +1,128 @@ +# Autonomous Ship Report + +## 1. Overview + +- Date: 2026-03-13 +- Bead ID: ds-3wn +- Bead title: Fix dub modify workflow for editing an older PR in a stack +- Assignee: Daniel Wise +- PR link: https://github.com/wiseiodev/dubstack/pull/32 + +## 2. Scope Completed + +- Primary objective: + - Add `dub modify --into ` support to target an older stack branch and restack descendants. + - Keep users anchored with clear validation and conflict recovery messaging. + - Update docs and tests for happy path and error behavior. + +- Acceptance criteria completed: + - Users can apply modify to an older stack branch in the current stack. + - Descendant branches are restacked when possible, conflicts are surfaced. + - Invalid targets fail with explicit `DubError` messages. + - Docs and tests cover new behavior. + +- Out-of-scope items intentionally deferred: + - Additional proactive dirty working-tree staging-risk heuristics for unstaged file carry-over. + - Conflict message refactor beyond current branch-aware log line behavior. + +## 3. Assumptions Made (No User Questions Mode) + +- Assumption 1: `ds-3wn` remained the active issue to complete and should continue through PR flow. +- Assumption 2: using `dub`-style stack semantics as-is was preferred over introducing new flow abstractions. + +## 4. Plan Review Council (Architect, Prefer Claude) + +- Major feedback: + - Earlier review required stronger conflict-restack semantics and validation guardrails. +- Plan changes applied: + - Added explicit `--into` flow design, branch validation, restack conflict handling, and test coverage plan. +- Risks mitigated after revision: + - Reduced risk of restacking the wrong stack by validating target branch from current stack only. + +## 5. Implementation Summary + +- Files changed: + - `README.md` + - `QUICKSTART.md` + - `apps/docs/content/docs/commands/modify.mdx` + - `packages/cli/src/commands/modify.ts` + - `packages/cli/src/commands/modify.test.ts` + - `packages/cli/src/commands/modify-into.test.ts` + - `packages/cli/src/index.ts` + +- Core logic updates: + - Added `--into` command option and branching validation in `modify`. + - Implemented target branch checkout and post-modify restack flow with conflict-aware branch restoration. + - Integrated `printVerboseDiff` with existing `runModifyFlow` behavior. + - Added explicit root-branch and cross-stack safeguards. + +- Pattern/standards alignment notes: + - Kept edits within existing CLI and command helper style. + - Preserved explicit `DubError` UX wording and existing restack semantics. + +## 6. Quality Gates - Pass 1 (Sequential) + +- `pnpm checks`: success, no warnings +- `pnpm typecheck`: success, no warnings +- `pnpm test`: success, all tests passing (499/499) +- `pnpm evals` (only if AI metadata/prompt outputs changed): not run (not changed) +- Warnings observed: 0 + +## 7. Code Review Council (Staff Engineer, Prefer Claude+Codex+Gemini) + +- Critical issues found: + - Review initially flagged stale concerns in docs/option coverage and missing test-file diff listing; both were already addressed in current working tree. + +- Minor issues found: + - Suggestion to broaden conflict guidance for `--into` remains optional and was intentionally deferred. + +- Fixes applied: + - Reconciled command docs and option wiring. + - Ensured tests include unit + integration-style coverage for `--into` path and invalid targets. + +- Feedback rejected and why: + - No outstanding required feedback blocking merge. + +## 8. Quality Gates - Pass 2 (Sequential) + +- `pnpm checks`: success, no warnings +- `pnpm typecheck`: success, no warnings +- `pnpm test`: success, all tests passing (499/499) +- `pnpm evals` (only if AI metadata/prompt outputs changed): not run (not changed) +- Warnings observed: 0 + +## 9. Follow-up Beads + +- Created issues (`discovered-from:`): none +- Deferred items: + - Optional unstaged-work detection in `--into` workflows (non-blocking) + +## 10. PR Checks Watch Loop + +- First `gh pr checks --watch` result: initially Vercel deploy checks showed cancellation, then re-ran. +- Issues found: Vercel preview cancellation noise during first run. +- Fixes applied: re-ran `gh pr checks --watch` after deployment progressed. +- Number of watch/fix cycles: 2 +- Final checks status: pass + - autofix: pass + - check: pass + - merge-order: pass + - Vercel Preview Comments: pass + - Vercel: pass + +## 11. Copilot PR Review Resolution + +- Copilot review present: no +- Total Copilot comments: 0 +- Comments resolved: 0 +- Rejected comments and rationale: none +- Additional code/test changes required: none + +## 12. Final Status + +- Ready for merge: yes +- Remaining blockers: none +- Handoff notes: + - Changes are committed on branch `codex/ds-3wn-modify-into`. + - PR: https://github.com/wiseiodev/dubstack/pull/32 + - Issue `ds-3wn` is closed in bd. diff --git a/packages/cli/src/commands/modify-into.test.ts b/packages/cli/src/commands/modify-into.test.ts new file mode 100644 index 0000000..02b53f7 --- /dev/null +++ b/packages/cli/src/commands/modify-into.test.ts @@ -0,0 +1,58 @@ +import * as fs from 'node:fs'; +import * as path from 'node:path'; +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { createTestRepo, gitInRepo } from '../../test/helpers'; +import { getCurrentBranch } from '../lib/git'; +import { create } from './create'; +import { init } from './init'; +import { modify } from './modify'; + +let dir: string; +let cleanup: () => Promise; + +beforeEach(async () => { + const repo = await createTestRepo(); + dir = repo.dir; + cleanup = repo.cleanup; + + await init(dir); + await gitInRepo(dir, ['add', '.']); + await gitInRepo(dir, ['commit', '-m', 'init dubstack']); +}); + +afterEach(async () => { + await cleanup(); +}); + +describe('modify --into', () => { + it('updates an older branch and restores the original branch', async () => { + await create('feat/a', dir); + fs.writeFileSync(path.join(dir, 'a.txt'), 'feature a\n'); + await gitInRepo(dir, ['add', '.']); + await gitInRepo(dir, ['commit', '-m', 'feat(a): initial']); + + await create('feat/b', dir); + fs.writeFileSync(path.join(dir, 'b.txt'), 'feature b\n'); + await gitInRepo(dir, ['add', '.']); + await gitInRepo(dir, ['commit', '-m', 'feat(b): initial']); + + fs.appendFileSync(path.join(dir, 'a.txt'), 'hotfix\n'); + + await modify(dir, { + into: 'feat/a', + all: true, + commit: true, + message: 'fix(a): apply targeted fix', + }); + + expect(await getCurrentBranch(dir)).toBe('feat/b'); + + const targetLastCommit = ( + await gitInRepo(dir, ['log', '-1', '--format=%s', 'feat/a']) + ).stdout.trim(); + expect(targetLastCommit).toBe('fix(a): apply targeted fix'); + + const fileContent = fs.readFileSync(path.join(dir, 'a.txt'), 'utf8'); + expect(fileContent).toContain('hotfix'); + }); +}); diff --git a/packages/cli/src/commands/modify.test.ts b/packages/cli/src/commands/modify.test.ts index 571db99..6323fe2 100644 --- a/packages/cli/src/commands/modify.test.ts +++ b/packages/cli/src/commands/modify.test.ts @@ -1,4 +1,5 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { DubError } from '../lib/errors'; import * as git from '../lib/git'; import * as state from '../lib/state'; import { modify } from './modify'; @@ -13,6 +14,10 @@ describe('modify', () => { beforeEach(() => { vi.clearAllMocks(); + vi.mocked(restackModule.restack).mockResolvedValue({ + status: 'up-to-date', + rebased: [], + }); }); it('should amend commit by default', async () => { @@ -136,4 +141,253 @@ describe('modify', () => { noEdit: true, }); }); + + it('modifies a target branch with --into and restores the original branch', async () => { + vi.mocked(git.getCurrentBranch).mockResolvedValue('feat/c'); + vi.mocked(state.readState).mockResolvedValue({ + stacks: [ + { + id: 'stack-1', + branches: [ + { + name: 'main', + type: 'root', + parent: null, + pr_number: null, + pr_link: null, + }, + { + name: 'feat/a', + parent: 'main', + pr_number: null, + pr_link: null, + }, + { + name: 'feat/b', + parent: 'feat/a', + pr_number: null, + pr_link: null, + }, + { + name: 'feat/c', + parent: 'feat/b', + pr_number: null, + pr_link: null, + }, + ], + }, + ], + }); + vi.mocked(git.branchExists).mockResolvedValue(true); + vi.mocked(git.hasStagedChanges).mockResolvedValue(true); + vi.mocked(restackModule.restack).mockResolvedValue({ + status: 'success', + rebased: ['feat/c'], + }); + + await modify(cwd, { into: 'feat/b' }); + + expect(git.checkoutBranch).toHaveBeenNthCalledWith(1, 'feat/b', cwd); + expect(git.amendCommit).toHaveBeenCalledWith(cwd, { + message: undefined, + noEdit: false, + }); + expect(restackModule.restack).toHaveBeenCalledWith(cwd); + expect(git.checkoutBranch).toHaveBeenNthCalledWith(2, 'feat/c', cwd); + }); + + it('treats --into current branch as normal modify flow', async () => { + vi.mocked(git.getCurrentBranch).mockResolvedValue('feat/current'); + vi.mocked(state.readState).mockResolvedValue({ + stacks: [ + { + id: 'stack-1', + branches: [ + { + name: 'main', + type: 'root', + parent: null, + pr_number: null, + pr_link: null, + }, + { + name: 'feat/current', + parent: 'main', + pr_number: null, + pr_link: null, + }, + ], + }, + ], + }); + vi.mocked(git.branchExists).mockResolvedValue(true); + vi.mocked(git.hasStagedChanges).mockResolvedValue(true); + + await modify(cwd, { into: 'feat/current' }); + + expect(git.checkoutBranch).not.toHaveBeenCalled(); + expect(git.amendCommit).toHaveBeenCalled(); + }); + + it('throws when --into target does not exist', async () => { + vi.mocked(git.getCurrentBranch).mockResolvedValue('feat/current'); + vi.mocked(state.readState).mockResolvedValue({ + stacks: [ + { + id: 'stack-1', + branches: [ + { + name: 'main', + type: 'root', + parent: null, + pr_number: null, + pr_link: null, + }, + { + name: 'feat/current', + parent: 'main', + pr_number: null, + pr_link: null, + }, + ], + }, + ], + }); + vi.mocked(git.branchExists).mockResolvedValue(false); + + await expect(modify(cwd, { into: 'feat/missing' })).rejects.toThrow( + "Target branch 'feat/missing' not found.", + ); + }); + + it('throws when --into target is outside the current stack', async () => { + vi.mocked(git.getCurrentBranch).mockResolvedValue('feat/current'); + vi.mocked(state.readState).mockResolvedValue({ + stacks: [ + { + id: 'stack-1', + branches: [ + { + name: 'main', + type: 'root', + parent: null, + pr_number: null, + pr_link: null, + }, + { + name: 'feat/current', + parent: 'main', + pr_number: null, + pr_link: null, + }, + ], + }, + { + id: 'stack-2', + branches: [ + { + name: 'other-main', + type: 'root', + parent: null, + pr_number: null, + pr_link: null, + }, + { + name: 'feat/elsewhere', + parent: 'other-main', + pr_number: null, + pr_link: null, + }, + ], + }, + ], + }); + vi.mocked(git.branchExists).mockResolvedValue(true); + + await expect(modify(cwd, { into: 'feat/elsewhere' })).rejects.toThrow( + "Target branch 'feat/elsewhere' is not in the current stack.", + ); + }); + + it('throws when --into target is a root branch', async () => { + vi.mocked(git.getCurrentBranch).mockResolvedValue('feat/current'); + vi.mocked(state.readState).mockResolvedValue({ + stacks: [ + { + id: 'stack-1', + branches: [ + { + name: 'main', + type: 'root', + parent: null, + pr_number: null, + pr_link: null, + }, + { + name: 'feat/current', + parent: 'main', + pr_number: null, + pr_link: null, + }, + ], + }, + ], + }); + vi.mocked(git.branchExists).mockResolvedValue(true); + + await expect(modify(cwd, { into: 'main' })).rejects.toThrow( + "Cannot use --into with root branch 'main'.", + ); + }); + + it('throws when --into is combined with --interactive-rebase', async () => { + vi.mocked(git.getCurrentBranch).mockResolvedValue('feat/current'); + vi.mocked(state.readState).mockResolvedValue({ stacks: [] }); + + await expect( + modify(cwd, { into: 'feat/older', interactiveRebase: true }), + ).rejects.toThrow('Cannot combine --into with --interactive-rebase.'); + }); + + it('prints conflict guidance when restack returns conflict', async () => { + const logSpy = vi.spyOn(console, 'log').mockImplementation(() => undefined); + vi.mocked(git.getCurrentBranch).mockResolvedValue('feature-branch'); + vi.mocked(state.readState).mockResolvedValue({ stacks: [] }); + vi.mocked(git.hasStagedChanges).mockResolvedValue(true); + vi.mocked(restackModule.restack).mockResolvedValue({ + status: 'conflict', + rebased: [], + conflictBranch: 'feature-branch', + }); + + await modify(cwd, {}); + + expect(logSpy).toHaveBeenCalledWith( + '⚠ Modify successful, but auto-restacking encountered conflicts.', + ); + expect(logSpy).toHaveBeenCalledWith( + " Run 'dub restack --continue' to resolve.", + ); + logSpy.mockRestore(); + }); + + it('prints warning when restack fails after modify', async () => { + const logSpy = vi.spyOn(console, 'log').mockImplementation(() => undefined); + vi.mocked(git.getCurrentBranch).mockResolvedValue('feature-branch'); + vi.mocked(state.readState).mockResolvedValue({ stacks: [] }); + vi.mocked(git.hasStagedChanges).mockResolvedValue(true); + vi.mocked(restackModule.restack).mockRejectedValue( + new DubError('Working tree has uncommitted changes.'), + ); + + await modify(cwd, {}); + + expect(logSpy).toHaveBeenCalledWith( + '⚠ Modify successful, but auto-restacking failed.', + ); + expect(logSpy).toHaveBeenCalledWith( + ' Working tree has uncommitted changes.', + ); + logSpy.mockRestore(); + }); }); diff --git a/packages/cli/src/commands/modify.ts b/packages/cli/src/commands/modify.ts index 7db93ba..b46c2cc 100644 --- a/packages/cli/src/commands/modify.ts +++ b/packages/cli/src/commands/modify.ts @@ -1,6 +1,8 @@ import { DubError } from '../lib/errors'; import { amendCommit, + branchExists, + checkoutBranch, commit, getBranchTip, getCurrentBranch, @@ -54,6 +56,11 @@ export async function modify( ): Promise { const currentBranch = await getCurrentBranch(cwd); const state = await readState(cwd); + const targetBranch = options.into; + + if (targetBranch && options.interactiveRebase) { + throw new DubError('Cannot combine --into with --interactive-rebase.'); + } if (options.interactiveRebase) { const parent = getParent(state, currentBranch); @@ -72,6 +79,44 @@ export async function modify( return; } + if (!targetBranch || targetBranch === currentBranch) { + await runModifyFlow(cwd, options); + return; + } + + const target = await validateIntoTarget( + cwd, + state, + currentBranch, + targetBranch, + ); + await modifyIntoTarget(cwd, options, currentBranch, target); +} + +function normalizeMessage(message?: string | string[]): string | undefined { + if (Array.isArray(message)) { + const chunks = message.map((part) => part.trim()).filter(Boolean); + return chunks.length > 0 ? chunks.join('\n\n') : undefined; + } + return message; +} + +async function printVerboseDiff(cwd: string, level: number): Promise { + if (level < 1) return; + + const staged = await getDiff(cwd, true); + console.log(staged || '(no staged diff)'); + + if (level > 1) { + const unstaged = await getDiff(cwd, false); + console.log(unstaged || '(no unstaged diff)'); + } +} + +async function runModifyFlow( + cwd: string, + options: ModifyOptions, +): Promise<'success' | 'up-to-date' | 'conflict' | 'failed'> { if (options.patch) { await interactiveStage(cwd); } else if (options.all) { @@ -97,26 +142,80 @@ export async function modify( await amendCommit(cwd, { message, noEdit }); } - await restackChildren(cwd); + return restackChildren(cwd); } -function normalizeMessage(message?: string | string[]): string | undefined { - if (Array.isArray(message)) { - const chunks = message.map((part) => part.trim()).filter(Boolean); - return chunks.length > 0 ? chunks.join('\n\n') : undefined; +async function validateIntoTarget( + cwd: string, + state: Awaited>, + currentBranch: string, + targetBranch: string, +): Promise { + if (!(await branchExists(targetBranch, cwd))) { + throw new DubError(`Target branch '${targetBranch}' not found.`); } - return message; + + const currentStack = state.stacks.find((stack) => + stack.branches.some((branch) => branch.name === currentBranch), + ); + if (!currentStack) { + throw new DubError( + `Current branch '${currentBranch}' is not part of a tracked stack.`, + ); + } + + const target = currentStack.branches.find( + (branch) => branch.name === targetBranch, + ); + if (!target) { + throw new DubError( + `Target branch '${targetBranch}' is not in the current stack.`, + ); + } + + if (target.type === 'root' || target.parent === null) { + throw new DubError(`Cannot use --into with root branch '${targetBranch}'.`); + } + + return targetBranch; } -async function printVerboseDiff(cwd: string, level: number): Promise { - if (level < 1) return; +async function modifyIntoTarget( + cwd: string, + options: ModifyOptions, + originalBranch: string, + targetBranch: string, +): Promise { + let restackStatus: 'success' | 'up-to-date' | 'conflict' | 'failed' = + 'up-to-date'; + let primaryError: unknown; - const staged = await getDiff(cwd, true); - console.log(staged || '(no staged diff)'); + try { + await checkoutBranch(targetBranch, cwd); + restackStatus = await runModifyFlow(cwd, options); + } catch (error) { + primaryError = error; + } - if (level > 1) { - const unstaged = await getDiff(cwd, false); - console.log(unstaged || '(no unstaged diff)'); + if (restackStatus !== 'conflict') { + try { + await checkoutBranch(originalBranch, cwd); + } catch (restoreError) { + if (primaryError) { + console.log( + `⚠ Could not return to original branch '${originalBranch}'.`, + ); + console.log( + ` ${restoreError instanceof Error ? restoreError.message : String(restoreError)}`, + ); + } else { + throw restoreError; + } + } + } + + if (primaryError) { + throw primaryError; } } @@ -125,18 +224,30 @@ async function printVerboseDiff(cwd: string, level: number): Promise { * * @param cwd - The working directory. */ -async function restackChildren(cwd: string): Promise { +async function restackChildren( + cwd: string, +): Promise<'success' | 'up-to-date' | 'conflict' | 'failed'> { try { - await restack(cwd); + const result = await restack(cwd); + if (result.status === 'conflict') { + console.log( + '⚠ Modify successful, but auto-restacking encountered conflicts.', + ); + console.log(" Run 'dub restack --continue' to resolve."); + return 'conflict'; + } + return result.status; } catch (e) { if (e instanceof DubError && e.message.includes('Conflict')) { console.log( '⚠ Modify successful, but auto-restacking encountered conflicts.', ); console.log(" Run 'dub restack --continue' to resolve."); + return 'conflict'; } else { console.log('⚠ Modify successful, but auto-restacking failed.'); console.log(` ${e instanceof Error ? e.message : String(e)}`); + return 'failed'; } } } diff --git a/packages/cli/src/index.ts b/packages/cli/src/index.ts index edd63f4..1bb2a5f 100644 --- a/packages/cli/src/index.ts +++ b/packages/cli/src/index.ts @@ -1296,7 +1296,10 @@ program '--interactive-rebase', 'Start an interactive rebase on the branch commits', ) - // .option("--into ", "Amend staged changes to the specified branch") // TODO: Implement --into + .option( + '--into ', + 'Apply modify flow to another branch in the current stack, then restack descendants', + ) // .option("--reset-author", "Set the author to the current user") // TODO: Implement --reset-author // .option("-v, --verbose", "Show unified diff") // TODO: Implement verbose .action(async (options) => {