diff --git a/openspec/changes/agent-claude-phase5-public-cli-dedup-base-detection-s-2026-06-03-14-28/.openspec.yaml b/openspec/changes/agent-claude-phase5-public-cli-dedup-base-detection-s-2026-06-03-14-28/.openspec.yaml new file mode 100644 index 0000000..0ba725f --- /dev/null +++ b/openspec/changes/agent-claude-phase5-public-cli-dedup-base-detection-s-2026-06-03-14-28/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-06-03 diff --git a/openspec/changes/agent-claude-phase5-public-cli-dedup-base-detection-s-2026-06-03-14-28/proposal.md b/openspec/changes/agent-claude-phase5-public-cli-dedup-base-detection-s-2026-06-03-14-28/proposal.md new file mode 100644 index 0000000..2e7fc7c --- /dev/null +++ b/openspec/changes/agent-claude-phase5-public-cli-dedup-base-detection-s-2026-06-03-14-28/proposal.md @@ -0,0 +1,14 @@ +## Why + +- After #618 landed the canonical `detectDefaultBaseBranch` in `src/git/index.js`, `src/pr.js` still carried its own drifted `detectBaseBranch` (different candidate list, only-origin ref scope, different fallback). Two base-detectors can return different bases for the same repo. Also, `src/git/index.js` kept a dead `rawRun` import + `void rawRun;` scaffolding deferred from Phase 1. + +## What Changes + +- Remove `pr.js` `detectBaseBranch`; route PR base resolution through git's canonical `detectDefaultBaseBranch` (imported from `./git`). Drop the now-redundant pr-module test (the detector is covered by `test/git-base-branch.test.js`). +- Remove the dead `rawRun` import and `void rawRun;` from `git/index.js` (and the stale JSDoc reference). + +## Impact + +- Surface: PR base detection (`gx pr`). Unifies on the more robust detector (also checks local heads). Common cases unchanged; edge-case differences (`develop` candidate, no-base fallback) converge to the canonical behavior. +- Risk: low. Zero new test failures vs base (33→33). Detector behavior is spec'd + tested in #618. +- Deferred (NOT in this PR): A3 sandbox-helper consolidation (3 of 8 helpers DRIFTED between `sandbox/index.js` and `cli/shared/sandbox.js` — needs drift reconciliation, not a partial dedup); A7 launch.js registry-shape (inferred/not-verified defensive code); B5 finish base-branch hoist (marginal perf, finish is not a hot loop). diff --git a/openspec/changes/agent-claude-phase5-public-cli-dedup-base-detection-s-2026-06-03-14-28/specs/phase5-public-cli-dedup-base-detection-sandbox-launch-rawrun/spec.md b/openspec/changes/agent-claude-phase5-public-cli-dedup-base-detection-s-2026-06-03-14-28/specs/phase5-public-cli-dedup-base-detection-sandbox-launch-rawrun/spec.md new file mode 100644 index 0000000..1ed39d3 --- /dev/null +++ b/openspec/changes/agent-claude-phase5-public-cli-dedup-base-detection-s-2026-06-03-14-28/specs/phase5-public-cli-dedup-base-detection-sandbox-launch-rawrun/spec.md @@ -0,0 +1,12 @@ +## ADDED Requirements + +### Requirement: Single canonical base-branch detector +Base-branch detection SHALL have one implementation. The PR flow SHALL resolve the base branch via the canonical `detectDefaultBaseBranch` (in `src/git/index.js`) rather than a separate, drift-prone copy. + +#### Scenario: PR base uses the canonical detector +- **WHEN** the PR flow needs a base branch and no explicit `--base` is given +- **THEN** it resolves the base via `detectDefaultBaseBranch` (origin/HEAD → main/master/dev → DEFAULT_BASE_BRANCH) + +#### Scenario: No duplicate detector remains +- **WHEN** the codebase is searched for base-branch detection +- **THEN** `src/pr.js` exposes no separate `detectBaseBranch` implementation diff --git a/openspec/changes/agent-claude-phase5-public-cli-dedup-base-detection-s-2026-06-03-14-28/tasks.md b/openspec/changes/agent-claude-phase5-public-cli-dedup-base-detection-s-2026-06-03-14-28/tasks.md new file mode 100644 index 0000000..490eb97 --- /dev/null +++ b/openspec/changes/agent-claude-phase5-public-cli-dedup-base-detection-s-2026-06-03-14-28/tasks.md @@ -0,0 +1,34 @@ +## Definition of Done + +This change is complete only when **all** of the following are true: + +- Every checkbox below is checked. +- The agent branch reaches `MERGED` state on `origin` and the PR URL + state are recorded in the completion handoff. +- If any step blocks (test failure, conflict, ambiguous result), append a `BLOCKED:` line under section 4 explaining the blocker and **STOP**. Do not tick remaining cleanup boxes; do not silently skip the cleanup pipeline. + +## Handoff + +- Handoff: change=`agent-claude-phase5-public-cli-dedup-base-detection-s-2026-06-03-14-28`; branch=`agent//`; scope=`TODO`; action=`continue this sandbox or finish cleanup after a usage-limit/manual takeover`. +- Copy prompt: Continue `agent-claude-phase5-public-cli-dedup-base-detection-s-2026-06-03-14-28` on branch `agent//`. Work inside the existing sandbox, review `openspec/changes/agent-claude-phase5-public-cli-dedup-base-detection-s-2026-06-03-14-28/tasks.md`, continue from the current state instead of creating a new sandbox, and when the work is done run `gx branch finish --branch agent// --base dev --via-pr --wait-for-merge --cleanup`. + +## 1. Specification + +- [x] 1.1 Finalize proposal scope and acceptance criteria for `agent-claude-phase5-public-cli-dedup-base-detection-s-2026-06-03-14-28`. +- [x] 1.2 Define normative requirements in `specs/phase5-public-cli-dedup-base-detection-sandbox-launch-rawrun/spec.md`. + +## 2. Implementation + +- [x] 2.1 Implement scoped behavior changes. +- [x] 2.2 Add/update focused regression coverage. + +## 3. Verification + +- [x] 3.1 Run targeted project verification commands. +- [x] 3.2 Run `openspec validate agent-claude-phase5-public-cli-dedup-base-detection-s-2026-06-03-14-28 --type change --strict`. +- [x] 3.3 Run `openspec validate --specs`. + +## 4. Cleanup (mandatory; run before claiming completion) + +- [ ] 4.1 Run the cleanup pipeline: `gx branch finish --branch agent// --base dev --via-pr --wait-for-merge --cleanup`. This handles commit -> push -> PR create -> merge wait -> worktree prune in one invocation. +- [ ] 4.2 Record the PR URL and final merge state (`MERGED`) in the completion handoff. +- [ ] 4.3 Confirm the sandbox worktree is gone (`git worktree list` no longer shows the agent path; `git branch -a` shows no surviving local/remote refs for the branch). diff --git a/src/git/index.js b/src/git/index.js index feffd27..9237e39 100644 --- a/src/git/index.js +++ b/src/git/index.js @@ -13,7 +13,6 @@ const { COMPOSE_HINT_FILES, LOCK_FILE_RELATIVE, } = require('../context'); -const { run: rawRun } = require('../core/runtime'); /** * Result of a synchronous child-process spawn. @@ -90,8 +89,8 @@ const { run: rawRun } = require('../core/runtime'); * answer from cache. `cachedSpawn` falls through to `cp.spawnSync` for any * command not on its read-only allowlist (writes like `git commit`, * `git push`, `git checkout`, `git worktree add/remove`), so this is a - * strict perf optimization that preserves `rawRun`'s signature - * `(cmd, args, opts) -> spawnSync result`. + * strict perf optimization preserving the `(cmd, args, opts) -> spawnSync + * result` signature. * * @param {string} cmd Executable to invoke. * @param {ReadonlyArray} args Argument vector. @@ -107,9 +106,6 @@ function run(cmd, args, options = {}) { timeout: options.timeout, }); } -// Keep rawRun referenced so a future write that intentionally bypasses the -// cache stays trivial. (Currently nothing in this module needs to bypass.) -void rawRun; /** * Run `git -C ` and throw unless `allowFailure` is set. diff --git a/src/pr.js b/src/pr.js index fe7873c..4d91f73 100644 --- a/src/pr.js +++ b/src/pr.js @@ -13,6 +13,7 @@ const { resolveRepoRoot, currentBranchName, hasOriginRemote, + detectDefaultBaseBranch, } = require('./git'); const DEFAULT_POLL_INTERVAL_MS = 5_000; @@ -105,26 +106,6 @@ function pushBranch(repoRoot, branch, { setUpstream = true } = {}) { }; } -function detectBaseBranch(repoRoot, fallback = 'main') { - // Prefer remote HEAD; fall back to common base names; finally `fallback`. - const remoteHead = run('git', ['symbolic-ref', '--short', 'refs/remotes/origin/HEAD'], { - cwd: repoRoot, allowFailure: true, - }); - if (remoteHead.status === 0) { - const value = (remoteHead.stdout || '').trim(); - if (value.startsWith('origin/')) { - return value.slice('origin/'.length); - } - } - for (const candidate of ['main', 'dev', 'develop', 'master']) { - const exists = run('git', ['rev-parse', '--verify', `refs/remotes/origin/${candidate}`], { - cwd: repoRoot, allowFailure: true, - }); - if (exists.status === 0) return candidate; - } - return fallback; -} - function defaultPrTitleFromCommit(repoRoot, branch) { const result = run('git', ['log', '-1', '--pretty=%s', branch], { cwd: repoRoot, allowFailure: true, @@ -201,7 +182,7 @@ function openPullRequest(options) { } } - const base = options.base || detectBaseBranch(repoRoot); + const base = options.base || detectDefaultBaseBranch(repoRoot); const title = options.title || defaultPrTitleFromCommit(repoRoot, branch); const body = options.body || defaultPrBodyFromCommits(repoRoot, branch, base); @@ -367,7 +348,6 @@ module.exports = { findOpenPrForBranch, findLatestPrForBranch, pushBranch, - detectBaseBranch, openPullRequest, getPullRequestStatus, enableAutoMerge, diff --git a/test/pr-module.test.js b/test/pr-module.test.js index 1ae50ca..6a642f8 100644 --- a/test/pr-module.test.js +++ b/test/pr-module.test.js @@ -32,16 +32,6 @@ function makeRepo() { return dir; } -test('detectBaseBranch falls back to main when no origin is configured', () => { - const repoRoot = makeRepo(); - try { - const base = prModule.detectBaseBranch(repoRoot); - assert.equal(base, 'main'); - } finally { - fs.rmSync(repoRoot, { recursive: true, force: true }); - } -}); - test('defaultPrTitleFromCommit returns latest commit subject', () => { const repoRoot = makeRepo(); try {