fix(ship-it): set git identity so lane commits succeed - #32
Conversation
Back-port of the coding-scenarios fix. GitHub runners have no default git identity, so the real-work lane's Candidate `git commit` can die with 'Author identity unknown' (exit 128), crashing the lane -> RESIDUALS_READY with no PR. It only survived here before by luck (the LLM agent committed on its own before Candidate ran). Set a global git identity before the batch runs so every lane worktree commit works deterministically. 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
CHANGES_REQUESTED — git identity fix lacks a regression guard (Tests lane HIGH) and places a cross-cutting infrastructure concern inside a business-logic step with a --global side-effect leak (Architecture lane, elevated to HIGH by multi-lane agreement).
Two independent lanes (Tests and Architecture) flag the same four-line block, elevating the combined severity to HIGH. The Tests lane finds no regression guard: the identity lines can be silently deleted with no CI signal. The Architecture lane finds a separation-of-concerns violation and a --global side-effect leak on shared runners. One MEDIUM finding notes that --global scope may be ineffective if a repo-level or system-level override is present. Three lanes (Correctness, Patterns, Pedantic) pass with no issues.
Full details in the inline comments on Files Changed.
VERDICT: CHANGES_REQUESTED
⚠️ Automated exhaustive review — 5 independent lanes (correctness, architecture, patterns, tests, pedantic) each with fresh context. Thorough but not definitive. Human maintainer should give final approval before merging.
| # Lane worktrees run `git commit`; GitHub runners have no git identity by | ||
| # default, so a lane's Candidate commit dies with "Author identity unknown". | ||
| git config --global user.email "amplifier-bot@users.noreply.github.com" | ||
| git config --global user.name "Amplifier Bot" |
There was a problem hiding this comment.
[HIGH — TESTS + ARCHITECTURE] No regression guard exists for this identity block, and it is placed inside the business-logic lane-execution step rather than a dedicated infrastructure step.
From the Tests lane: these four lines fix the 'Author identity unknown' crash but can be silently deleted in a future refactor with no CI signal. Add a subsequent verification step (e.g. ) so CI fails immediately if the identity block is ever removed.
From the Architecture lane: git identity is a cross-cutting runner infrastructure concern and should not live inside the lane-execution step. If any step inserted before this one also calls , the identity will be absent. The flag also mutates the shared runner gitconfig, creating a side-effect leak if jobs share a runner. Extract this into a named step that runs before any git-touching work, or into a reusable composite action.
| # Lane worktrees run `git commit`; GitHub runners have no git identity by | ||
| # default, so a lane's Candidate commit dies with "Author identity unknown". | ||
| git config --global user.email "amplifier-bot@users.noreply.github.com" | ||
| git config --global user.name "Amplifier Bot" |
There was a problem hiding this comment.
[MEDIUM — TESTS] The scope may be ineffective if the lane worktree runs under a different user context or has a repo-level/system-level git identity override. There is no test that exercises the full worktree-commit path to confirm the identity is actually consumed by the that was previously failing.
2 participants
Back-port of coding-scenarios PR #5. GitHub runners have no default git identity, so the real-work lane's Candidate
git commitcan fail with 'Author identity unknown' (exit 128) → RESIDUALS_READY, no PR. It only survived here by luck (the LLM agent happened to commit before Candidate ran). Sets a global git identity before the batch runs. actionlint OK.