This entire file is a revert, and the changes that were previously made here have been moved to the tag-and-release.yaml workflow so we can continue to use both workflows as we transition.

P2 Idempotency check breaks when targetRef is a short SHA (Contract Auditor P1, downgraded)

resolveCommit in calculate.go accepts any hex string >= 7 chars via isHexSHA and returns it unchanged. createAndPushTag then compares existing == targetRef where existing comes from git rev-parse --verify refs/tags/<tag>^{}, which always returns a full 40-character SHA. If a user passes a 7-12 character short SHA via the commit_sha workflow input, the comparison fails even when both refer to the same commit.

The same issue affects createAndPushBranch where remoteSHA (full, from ls-remote) is compared against targetRef (possibly short).

Downgraded from P1 because commit_sha defaults to empty (which falls through to git rev-parse origin/<ref>, producing a full SHA). Short SHAs only happen when explicitly provided. But when they are, the consequence is a confusing error that breaks the idempotency promise: "tag already exists at , expected ".

Fix: resolve targetRef to a full SHA with git rev-parse in resolveCommit before returning it, or do a strings.HasPrefix comparison in the idempotency checks.

P2 publishRelease validation will fail in dry-run mode (Go Architect)

publishRelease performs validation checks (asset existence, git describe match) before the mutation. If the dry-run executor is used with publishRelease, the git describe check will fail because the checkout won't match the expected tag (since the tag was never actually created in dry-run). The function will error out before reaching the gh release create mutation.

In release.yaml, the publish step calls ./scripts/release/publish.sh (shell script), not go run ./scripts/release-action publish, so this is not a live bug in CI. But publishRelease in Go now accepts a CommandExecutor with --dry-run wired up in main.go, creating an API contract that won't actually work in dry-run mode. Worth either documenting that publish --dry-run is not expected to work standalone, or guarding the git describe check.

P3 RunOutput swallows stderr, making command failures opaque (Contract Auditor P2, Go Architect Obs)

RunOutput calls cmd.Output() and on error returns only the *exec.ExitError. The caller gets an opaque exit code with no stderr context. Failures in git fetch, git rev-parse, git ls-remote, etc. will produce errors like "exit status 128" with no indication of why.

This is pre-existing behavior (same code was in git.go before the refactoring), so not a regression. But the new abstraction is a good opportunity to attach stderr to the error. cmd.Output() already populates ExitError.Stderr in Go; consider including it in the error message.

P3 Mock comment says "returns ("", error)" but implementation returns ("", nil) (Test Auditor, Edge Case Analyst, Style Reviewer, Modernization Reviewer)

Four reviewers independently flagged this. The comment on RunOutputFunc says If nil, returns ("", error) but the fallback on line 29-30 is return "", nil. Anyone writing new tests that rely on the default mock behavior would be misled.

P3 TestShelljoin "quote" case has a dead want field (Test Auditor, Style Reviewer, Modernization Reviewer)

The "quote" row has want: "\"it's\"" set, but the assertion short-circuits to assert.Contains(t, got, "it") with comment "just checks it doesn't panic and wraps." The want field is never checked for this row, so the test would pass even if shelljoin returned "it" with no quoting at all.

shelljoin produces 'it'"'"'s' (POSIX single-quote escaping), not double-quoted "it's". Either fix want to match the actual output and assert with Equal, or remove the want field for this case.

Nit generate-notes subcommand has --dry-run option but it's a no-op (Structural Analyst)

dryRunOption is added to generate-notes in main.go, but generateReleaseNotes only uses RunOutput/Run (read-only methods). The dry-run executor delegates these to the real executor unchanged, so the flag has no observable effect.

Either remove dryRunOption from generate-notes or add a comment explaining it's a no-op for consistency.

Nit The parameter name exec shadows the os/exec stdlib package (Style Reviewer)

Used consistently across every file (func gitOutput(exec CommandExecutor, ...), etc.), so this is a deliberate pattern choice rather than an accident. In cmdexec.go itself, os/exec is imported, so exec the parameter and exec the package coexist in the same file. A name like ce or cmdExec would avoid the ambiguity. Low priority since the pattern is consistent.