Conversation
Introduce bin/fm-scm-lib.sh: detect github/ado/unknown from a URL or origin remote, with normalized PR helpers (state, head, state+head, number-for-branch, ensure-commit-object, resolve-pr-head). Only a provably-ADO provider takes the az path; github and unknown keep the exact gh/gh-axi/git path, so GitHub behaviour is byte-for-byte preserved. Rewire fm-pr-check, fm-review-diff, fm-teardown, and fm-fleet-snapshot to the library; refuse ADO PR URLs in fm-pr-merge (captain completes in the ADO UI); add the az dependency and NEEDS_AZ_AUTH prompt for ADO-origin projects in fm-bootstrap.
fm-brief detects the project's PR host from its origin remote via fm-scm-lib.sh and renders GitHub or Azure DevOps tooling in the host-operations rule and the direct-PR done-line. GitHub briefs are unchanged.
Cover GitHub and Azure DevOps PR URL parsing, normalized pr_state/pr_head/ pr_state_head, and ado pr_number_for_branch against gh and az mocks.
Provider-neutral wording for pr_head recording, direct-PR host, review-diff PR head resolution, and teardown landed-work checks; note fm-pr-merge refuses ADO PRs; add NEEDS_AZ_AUTH handling. Point to docs/ado-backend.md.
fm-teardown.sh now sources fm-scm-lib.sh; the gotmp fixtures build an isolated bin/ that must include every sourced sibling.
Codify the content-readiness rule: firstmate waits only on content-influenced automation gates (build/test/e2e, coverage, Component Governance) and ignores human review/sign-off/attestation gates even when blocking. Expired content build gates are re-queued via PR-native policy evaluation, never treated as vacuously green.
…k release to prevent leak
The az-absent sub-case removed az from the fake toolchain but relied on az being absent from BASE_PATH, which fails on hosts that ship azure-cli (GitHub's ubuntu runners). Prepending a fakebin can add a stub but cannot hide a real tool further down PATH, so bootstrap found the system az and emitted NEEDS_AZ_AUTH instead of MISSING: az. Run that sub-case against a base PATH scrubbed of any real az via symlinks so the assertion is hermetic everywhere.
Two CI-surfaced issues on the ADO test additions:
- tests/fm-scm-lib.test.sh was committed mode 100644, so CI's direct
invocation ("$test_script") failed with Permission denied (exit 126).
Mark it executable to match every other test.
- The az-scrub helper used `[ -x ] && [ ! -d ] || continue`, which
shellcheck flags as SC2015 (A && B || C is not if-then-else). Split into
explicit guards.
Author
|
Superseded: landed via fork CI on davidkydd/firstmate#1 (green + merged). This upstream PR is no longer the delivery path. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Intent
Add first-party Azure DevOps (ADO) support to firstmate's own tracked material (Stream A: firstmate-core only), per an approved design blueprint and binding captain decisions. Introduce bin/fm-scm-lib.sh as a single sourced+CLI provider abstraction that detects the SCM host from the origin URL (github.com -> github; dev.azure.com/ssh.dev.azure.com/*.visualstudio.com -> ado; else unknown) and provides normalized PR helpers (state MERGED/OPEN/CLOSED, head sha, merged-PR-for-branch, ensure/resolve PR head commit) via gh for GitHub and az for ADO. Deliberate design decision: github AND unknown both take the exact pre-existing gh/git path so GitHub behavior is byte-for-byte preserved and non-github/non-ado remotes keep today's fallthrough - this is intentional, not a missed case. Rewire fm-pr-check, fm-review-diff, fm-teardown, and widen fm-fleet-snapshot's PR-URL extraction to also match ADO /_git//pullrequest/; teardown keeps the provider-agnostic content_in_default fallback and ALL refusal semantics intact. Captain decision #1: firstmate must NEVER merge/complete an ADO PR, so fm-pr-merge.sh refuses an ADO PR URL before any recording or host call and points the captain at the ADO UI (squash to match GitHub, never delete source branch); GitHub merge behavior is unchanged (--squash default). Captain decision #4: bootstrap adds az (+azure-devops extension) as a dependency for ADO-origin projects and emits NEEDS_AZ_AUTH prompting az login when unauthenticated, analogous to NEEDS_GH_AUTH, following detect->consent (never auto-install). Provider-aware crewmate brief wording (gh-axi for GitHub, az repos for ADO). AGENTS.md prose is reworded GitHub-specific contract lines to provider-neutral pointing at fm-scm-lib.sh/docs/ado-backend.md, deliberately NOT restating az command shapes inline (AGENTS.md size discipline / one-owner rule). docs/ado-backend.md is the human reference, and now also codifies which ADO gates firstmate gates on: firstmate judges content-readiness using only content-influenced automation gates (build/test/e2e, coverage, Component Governance) and ignores human review/sign-off/attestation gates even when blocking/rejected, re-queuing expired content build gates via PR-native policy evaluation rather than treating them as vacuously green. Scope is intentionally firstmate-core only (captain decision #5): no Python ado-pr CLI vendored, no new delivery mode, no explicit provider registry tag (provider auto-detected from origin, decision #2); fork-based ADO contribution is intentionally out of scope and firstmate refuses+escalates it (decision #6). Live end-to-end ADO run against a real repo is intentionally left as documented manual verification in docs/ado-backend.md (out of scope for automated CI). Tests: new tests/fm-scm-lib.test.sh (hermetic URL->provider table) plus extended pr-merge/review-diff/teardown/bootstrap/fleet-snapshot tests with an az mock alongside the gh-axi mock; the entire existing GitHub regression suite must stay green unchanged as proof the abstraction did not alter GitHub behavior. Prior pipeline rounds already added kept fix commits: a test fix trapping watcher signals so a checkpoint SIGTERM releases the singleton lock, and document-sync commits updating README/docs; all are intentionally on the branch. Follows firstmate coding guidelines: one sentence per line, plain dash, shellcheck-clean, colocated tests. NOTE: some existing watcher/timeout tests are timing-sensitive and can flake under load, and the fork push can hit transient network drops; treat a flaky-timing or transient-network failure as infra, not a code defect.
What Changed
bin/fm-scm-lib.sh, a sourced+CLI provider abstraction that detects the SCM host from the origin URL (github.com → github, dev.azure.com/ssh.dev.azure.com/*.visualstudio.com → ado, else unknown) and exposes normalized PR helpers (state, head sha, merged-PR-for-branch, resolve/ensure PR head) backed byghfor GitHub andazfor Azure DevOps; github and unknown both keep the exact pre-existing gh/git path so GitHub behavior is byte-for-byte preserved.fm-pr-check,fm-review-diff, andfm-teardownthrough the abstraction, widenedfm-fleet-snapshotPR-URL extraction to also match ADO/_git/<repo>/pullrequest/<n>, madefm-pr-mergerefuse ADO PR URLs before any recording or host call (pointing the captain at the ADO UI), addedNEEDS_AZ_AUTHbootstrap detection for ADO-origin projects, and made crewmate brief wording provider-aware; teardown keeps its content-in-default fallback and all refusal semantics intact.docs/ado-backend.md(including which ADO gates firstmate gates on) plus doc/index updates, and added hermeticfm-scm-libtests alongside extended pr-merge/review-diff/teardown/bootstrap/fleet-snapshot coverage with anazmock; a kept fix disarms terminating-signal traps during watcher lock release so a checkpoint SIGTERM releases the singleton lock.Risk Assessment
✅ Low: The change introduces a well-isolated provider abstraction that preserves the exact prior GitHub/git paths for github and unknown providers, keeps all teardown refusal semantics and the provider-agnostic content fallback intact, refuses ADO merges per captain policy, and is backed by hermetic tests plus the unchanged GitHub regression suite.
Testing
The configured baseline e2e suite already passed. I ran the smallest relevant set — every ADO-touched test plus the timing-sensitive watcher-lock and watch-checkpoint tests the final commit fixed — and all passed; the earlier full-suite failure is consistent with the author's flagged flaky timing/network tests rather than a code defect. I then exercised the two core captain decisions end-to-end as an operator would: provider auto-detection from origin URLs correctly maps GitHub/ADO/unknown, and fm-pr-merge refuses an Azure DevOps PR URL (exits non-zero, records no pr=, never invokes gh-axi, and points the captain to the ADO UI with squash/no-delete guidance) while the GitHub path stays byte-for-byte the same squash merge. Evidence captured as CLI transcripts.
Evidence: Provider detection from origin URLs
github.com -> github; dev.azure.com/ssh.dev.azure.com/*.visualstudio.com -> ado; gitlab.com -> unknownEvidence: ADO PR merge refusal (captain decision #1)
error: firstmate does not merge Azure DevOps PRs. Complete this PR yourself in the Azure DevOps UI ... Use squash to match GitHub, and do NOT delete the source branch. exit=1, pr= not recorded, gh-axi never calledEvidence: GitHub merge path preserved
gh-axi invoked with: pr merge 7 --repo owner/repo --squash ; pr=https://github.com/owner/repo/pull/7 recordedPipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
✅ **Review** - passed
✅ No issues found.
🔧 **Test** - 1 issue found → auto-fixed ✅
command -v tmux >/dev/null || { echo "tmux is required for e2e tests" >&2; exit 1; }; tmux -V; rc=0; for t in tests/*.test.sh; do echo "== $t =="; bash "$t" || rc=1; done; exit "$rc"🔧 Fix: disarm terminating-signal traps during watcher lock release to prevent leak
✅ Re-checked - no issues remain.
command -v tmux >/dev/null || { echo "tmux is required for e2e tests" >&2; exit 1; }; tmux -V; rc=0; for t in tests/*.test.sh; do echo "== $t =="; bash "$t" || rc=1; done; exit "$rc"bash tests/fm-scm-lib.test.sh(hermetic URL->provider + PR-op table)bash tests/fm-pr-merge.test.sh(ADO refusal + GitHub parity)bash tests/fm-review-diff.test.sh(ADO PR head via az)bash tests/fm-teardown.test.sh(ADO landed-work parity + refusal semantics)bash tests/fm-bootstrap.test.sh(NEEDS_AZ_AUTH for ADO-origin projects)bash tests/fm-fleet-snapshot-view.test.sh(ADO PR URL extraction)bash tests/fm-gotmp.test.sh(scm-lib symlinked into teardown fixtures)bash tests/fm-watcher-lock.test.shandbash tests/fm-watch-checkpoint.test.sh(signal-trap lock-release fix)Manual:bin/fm-scm-lib.sh provider-of-urlacross github/dev.azure.com/ssh.dev.azure.com/visualstudio.com/gitlabManual:bin/fm-pr-merge.shagainst an ADO PR URL (refuses, no pr= recorded, gh-axi never called) and a GitHub PR URL (squash merge, pr= recorded)✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.