Skip to content
sdlc

T-RNW9-task-close-out-verifies-pr-on-main

Status: closed/done · Impact: medium · Complexity: small

Auto-generated from a /sdlc:task-work post-mortem. Review and promote to open/ready before picking up.

Goal

Section titled “Goal”

/sdlc:task-close-out detects the merged PR via gh pr list --search "head:feat/<basename> is:merged" --limit 1. If a branch name was ever reused across PRs (e.g. a branch deleted then recreated under the same feat/<basename> slug), the query can match a stale PR — silently. The skill would then close the task referencing the wrong PR number. Verifying that the returned mergeCommit is reachable from origin/main (“the merge actually landed on the trunk we care about”) makes the detection robust to branch reuse and to weird repo states (force-pushed merges, re-targeted PRs) at near-zero cost. This hardens close-out enough that the orchestrator (T-FQCN-self-driving-orchestrator-loop) can dispatch it without the human supervising the PR-number match.

Today

Section titled “Today”

From the originating post-mortem:

gh pr list --search "head:feat/<basename> is:merged" can return false positives if the same feat/<basename> name was reused across PRs (rare but possible). The skill takes --limit 1 which is the most-recent — fine for now, but a sanity-check that the returned PR’s mergeCommit is actually reachable from origin/main would harden against this.

plugin/skills/task-close-out/SKILL.md Step 3 runs the gh pr list query, captures number,mergedAt,mergeCommit, and proceeds to Step 4 without verifying that mergeCommit is reachable from the current origin/main. If the most-recent matching PR was merged into a different branch (or the merge was later reverted), the close-out commit’s “Shipped via #N” claim is wrong.

Proposed

Section titled “Proposed”

After Step 3 returns a candidate PR, the skill runs a reachability check before proceeding:

git fetch origin main --quiet
git merge-base --is-ancestor <mergeCommit> origin/main

If the check fails, the skill emits STALE-PR pr=#<N> reason="mergeCommit not reachable from origin/main" and exits without making any edits or commits.

The invariants linter pins: (a) reachability is checked before any file edit / commit / push / worktree teardown / branch deletion happens; (b) the STALE-PR marker is used for this specific failure mode (distinct from NO-MERGED-PR).

Approach

Section titled “Approach”
  1. Add the git fetch + git merge-base --is-ancestor check to Step 3 of plugin/skills/task-close-out/SKILL.md, between “candidate PR resolved” and “proceed to Step 4.”
  2. Document the STALE-PR marker in the skill’s output contract alongside DONE, ALREADY-CLOSED, NOT-IN-PROGRESS, NO-MERGED-PR, PARTIAL.
  3. Add the two invariants above to plugin/skills/task-close-out/invariants.yaml.

Files to touch

Section titled “Files to touch”
  • plugin/skills/task-close-out/SKILL.md — Step 3 gains the reachability check; output contract gains STALE-PR.
  • plugin/skills/task-close-out/invariants.yaml — pin reachability-before-mutation and the STALE-PR marker prefix.

Acceptance criteria

Section titled “Acceptance criteria”
  • AC-1: A synthetic-fixture trace where gh pr list returns a PR whose mergeCommit is not reachable from origin/main results in STALE-PR pr=#<N> reason="mergeCommit not reachable from origin/main" and zero file edits / commits / branch deletions. (agent-manual via constructed fixture)
  • AC-2: The happy path (mergeCommit reachable) is unchanged end-to-end. (auto via the existing AC-2 synthetic trace from T-RDKI-extract-task-close-out-skill)
  • AC-3: lint_skill_prose.py passes for the new invariants. (auto)

Out of scope

Section titled “Out of scope”
  • Detecting reverted merges as a distinct failure mode. A revert leaves mergeCommit reachable but logically undone; addressing it requires a separate git log --grep="^Revert" check that’s out of scope here.
  • Hardening other gh pr callsites elsewhere in the plugin. Start with task-close-out where the failure is most expensive.

Dependencies

Section titled “Dependencies”
  • none

Discovery context

Section titled “Discovery context”

Spawned by /sdlc:task-work post-mortem of T-RDKI-extract-task-close-out-skill on 2026-05-20.

Post-mortem

Section titled “Post-mortem”

Captured by /sdlc:task-work on 2026-05-20. PR: pending.

Acceptance criteria coverage

Section titled “Acceptance criteria coverage”
  • AC-1: agent-manual — synthetic trace. Given gh pr list --search "head:feat/<basename> is:merged" returns {number: 42, mergeCommit: {oid: "abc123"}} and gh pr view 42 --json state returns MERGED, the new Step 3 reachability check runs git merge-base --is-ancestor abc123 origin/main. Exit code 1 (not reachable) drives the documented emit: STALE-PR pr=#42 reason="mergeCommit not reachable from origin/main" followed by exit. The check’s anchor sentence (“Before any file edit, commit, branch deletion, or worktree teardown, verify…”) and the bold “this check runs before Step 4, so no file edits, no commits, no branch deletions have happened when the STALE-PR marker is emitted” together guarantee zero file edits / commits / branch deletions on this path. The invariants linter pins both literals (the marker prefix and the “Before any file edit…” sentence) so the contract can’t drift silently.
  • AC-2: agent-manual — synthetic trace. On the happy path, git merge-base --is-ancestor <mergeCommit> origin/main exits 0; prose falls through unchanged to Step 4. No previous Step 4 prose, no commit-message format, no teardown order was edited. Structurally identical to the AC-2 trace in T-RDKI-extract-task-close-out-skill’s post-mortem, which exercised feat/2026-05-19-add-pr-check-skill against PR #39. Note: the task spec called this “auto via the existing AC-2 synthetic trace”, but that prior trace itself was agent-manual, so the realistic classification here is agent-manual — flagging the spec wording for future tightening.
  • AC-3: auto — python plugin/scripts/lint_skill_prose.py plugin/skills/task-close-out/SKILL.md passes (1 skill(s) checked, 1 with invariants.yaml, no violations.). The new invariants entries (STALE-PR marker, merge-base --is-ancestor literal, “Before any file edit…” anchor sentence) are all asserted against the SKILL.md and all matched.

What worked

Section titled “What worked”
  • The invariants linter caught a case-sensitivity drift between the invariants.yaml entry (“before any…”) and the SKILL.md prose (“Before any…”) immediately — fixed in one edit, no silent contract rot. Exactly the failure mode the linter exists for.
  • run_quality_checks.py --config sdlc.yaml ran all 6 gates in one shot (OK 6/6), no flake, no retries.
  • The reachability check is one git merge-base --is-ancestor invocation — the simplest possible mechanism for the property, and a well-known git primitive that won’t surprise readers.

Friction and automation gaps

Section titled “Friction and automation gaps”
  • The skill prose says to “run just full-check and just ci” in Step 7, but this repo has no justfile; the actual gate is python plugin/scripts/run_quality_checks.py --config sdlc.yaml. The task-work skill has hardcoded justfile verbs in its prose that don’t match this project’s actual quality runner. Either the skill should read sdlc.yaml’s quality_checks: directly (the convention is documented in plugin/conventions/sdlc-yaml.md), or it should shell out to run_quality_checks.py --config sdlc.yaml and not name the underlying runner at all.
  • task-work Step 6 (“delegate implementation to a sub-agent”) assumes the parent has the Agent tool. The resumed run did not have Agent available, so implementation was carried out inline by the parent. For a small prose-only task this was fine, but the skill prose doesn’t address the fallback — it should either declare Agent a hard dependency or describe the inline-implementation fallback explicitly. → T-PZW5-task-work-step-6-inline-impl-fallback
  • A previous sub-agent for this same task confused /sdlc:task-ensure-ready’s READY: marker for /sdlc:task-work’s final verdict and exited prematurely between Step 5a and Step 5b. The recovery here required a hand-crafted “resume at Step 5b” prompt from the orchestrator. task-work could detect this case automatically: at start, if the task is status: open/ready on main but the feat branch already exists with a docs(tasks): verify <basename> implementation-ready commit as HEAD, jump directly to Step 5b instead of re-running the gate. → T-1QO7-task-work-resumes-from-verify-stamp-commit

Spawned follow-up tasks

Section titled “Spawned follow-up tasks”

← Back to Tasks