Skip to content

Estate Consolidation P0 — cli installs the CLI the repository pins, and the estates' infra-diff becomes one reusable workflow - #11

Closed
igorlamos wants to merge 2 commits into
mainfrom
feature/estate-consolidation-p0-foundations
Closed

igorlamos wants to merge 2 commits into
mainfrom
feature/estate-consolidation-p0-foundations

Conversation

@igorlamos

Copy link
Copy Markdown
Contributor

This PR does two things. First, the cli action now installs the beplus CLI version a repository pins in beplus.estate.json (via be auto) instead of floating on latest, so a CLI release can no longer change what a repository's CI does without a commit in that repository. Second, the 186-line infra-diff.yml that each of the three estates carries becomes one reusable workflow here, with no inputs. It cannot be made to deploy, and a script proves step by step that it reproduces each copy it replaces.

Part of Linear initiative I-34 "Estate Consolidation — One Copy of Every Helper", project P0 — Foundations & Guardrails (https://linear.app/bepluscloud/project/estate-consolidation-p0-foundations-and-guardrails-44e0009310cd).
I-34 uses one branch per project per repository and one commit per issue. Please review commit by commit, then squash-merge.

Commit Issue
808f3a2 feat(cli): installs the beplus CLI the repository pins, via be auto (BE-213) BE-213
cd32810 feat(infra-diff): the estates' pull-request cdk diff, as one reusable workflow (BE-218) BE-218

Base: main @ 61a06d2. Both commits are signed.

Merge order: this PR comes after the beplus/be be auto PR (BE-213). It must be merged, and the v2 tag moved to the merge commit, before any estate merges its BE-218 infra-diff caller. The full list is under "Waiting on Igor" at the end.


BE-213 — Pin the beplus CLI per repository (Linear)

A CLI release must never change what a repository's CI does unless that repository has a commit that says so. Until now, cli defaulted to latest.

What changed

  • cli/action.yml
    • BE_CLI_VERSION now defaults to empty instead of latest.
    • A new "Choose" step finds the nearest beplus.estate.json at or above BE_WORKING_DIRECTORY, walking up the same way be auto does. It reads cli.version only to decide whether a pin exists. Resolving and validating the pin stays be auto's job (PLAN principle 1).
    • With a pin, and no explicit version: the action runs be auto. A post-check then fails the run unless beplus --version is exactly the pin. An inexact pin, or a bug in be auto, can never fall back to latest.
    • With an explicit BE_CLI_VERSION: the action runs be <value>. The explicit value wins, with a ::warning:: when it differs from the pin.
    • With no pin: the action runs be latest, as today, and emits a ::warning:: that the repository floats. The warning shows the cli.version snippet to paste in.
    • A manifest that cannot be read fails the run. It does not count as "no pin".
    • The be argument now reaches the script through env and is quoted, not interpolated.
    • New input BE_WORKING_DIRECTORY (default: the workspace) says where the manifest search starts.
  • cdk-package/action.yml installs the CLI through cli@v2. Its cli-version default changes from latest to empty, so it honours the pin too. I also removed its stale note that latest is v1; latest is 2.11.0.
  • Reusable workflows: docs-pages, library-ci, library-publish (two places) and provisioning no longer pass vars.BE_CLI_VERSION || 'latest'. An explicit latest would override a pin. They now pass vars.BE_CLI_VERSION, which still wins when it is set.
  • .github/workflows/test-cli.yml runs this branch's ./cli in a six-leg matrix against fixtures in test/cli/{pinned,unpinned,inexact} and asserts what beplus --version reports.
  • cli/README.md gains a "Pinning the CLI" section, the bump procedure (one commit per repository, CI proves it, merged on its own) and troubleshooting entries for the two new messages. The root README.md examples no longer pass BE_CLI_VERSION: latest.

Gates

Command Result
node harness.cjs <worktree>: runs cli/action.yml's choose → install → verify steps and test-cli.yml's assert step locally, for every matrix leg, with a fake be that implements be auto as BE-213 specifies (FAKE_BE_MODE=spec) 6/6 pass. The pin installs 2.10.0, also from a package directory below the manifest. BE_CLI_VERSION=2.9.0 wins, with the override warning. No pin and no manifest both float on latest, with the float warning. The inexact pin "2" fails the action.
Same harness, FAKE_BE_MODE=lax (a buggy be auto that resolves "2" to 2.11.0) 6/6 pass. The action's post-check catches it: ::error … pins beplus CLI 2, but be auto installed 2.11.0
Same harness, FAKE_BE_MODE=old (today's beplus/be@main, whose be auto does not read beplus.estate.json yet) 4/6, as expected. Both pinned legs fail loudly, and none floats. This is the dependency on the beplus/be PR.
Review re-run of the harness against the real be auto from the beplus/be branch (4aa9ea2) 6/6 pass
shellcheck on every run: block of cli/action.yml and test-cli.yml (extracted, with ${{ }} substituted) clean, 0 findings
YAML parseDocument on cli/action.yml, cdk-package/action.yml, .github/workflows/*.yml 7/7 files parse with no errors or warnings
Reference check: every inputs.X declared, every steps.choose.outputs.X written, every in-repo caller of cli passes only declared inputs all references resolve (6 callers)
Edge cases of the choose step: invalid JSON, "cli" as a string, a version with whitespace, version: null The first three fail with ::error naming the file. null counts as no pin and floats with the warning.
node scripts/verify-callers.mjs ~/Git/github.com/beplus (the repo's old R2-1 library gate), at base and on the branch Crashes the same way in both: TypeError … reading 'map' on the same expression. It was already broken at main, because the HEAD~1 originals are gone, so it is unrelated to this change.

Deviations

  • The test fixture pins 2.10.0, not 2.11.0. 2.11.0 is the newest release, so a run that ignored the pin and installed latest would also report 2.11.0 and the test would prove nothing. 2.10.0 is a real release (it is in the S3 release index), so a run that floats cannot pass.
  • New input BE_WORKING_DIRECTORY. The test needs it to point the action at fixture manifests, and monorepos may want a manifest that is not at the root. The default keeps behaviour unchanged for every existing caller.
  • Small extension beyond cli and cdk-package: the four reusable workflows passed || 'latest', which would override any pin. They now pass only vars.BE_CLI_VERSION. For repositories without a pin, the only visible change is the new float warning.
  • The action reads cli.version itself, but only to detect a pin and to check beplus --version after the install. That post-check is deliberate defence in depth.
  • No real be/beplus binaries were downloaded or installed on the agent machine. The logic was proven with a fake be, plus the review re-run against the real be auto script. The live proof is this PR's "Test cli" run.
  • Known leftovers from review, deliberately not fixed here:
    • provisioning/README.md:106 still shows BE_CLI_VERSION: latest in an example.
    • cli.version set to empty, null or false, or "cli": {}, floats to latest with a warning instead of failing.
    • The inexact-pin leg passes on any failure of the action, not only on the pin refusal.
    • An explicit BE_CLI_VERSION: auto skips the post-check.

BE-218 — infra-diff.yml as a reusable workflow (Linear)

bepluscloud/ai, ironmountain-cz/monorepo and mrkithq/monorepo each carried a 186-line infra-diff.yml. Their jobs differ in three values only, and all three are now derived, so the shared workflow has no inputs.

What changed

  • .github/workflows/infra-diff.yml (on: workflow_call). I built it from the three estate copies: bepluscloud origin/dev 3065b04, Mountaineer origin/dev 3fd8e9893 and MRKIT dev 13220b751. The three derived values:
    1. Role. arn:aws:iam::${{ vars.BE_AWS_ACCOUNT_ID }}:role/github-actions-${{ github.repository_owner }}-${{ github.event.repository.name }}-${{ matrix.env }}-cdk-diff.
      • All three GitHubOIDCStack.ts build github-actions-${org}-${repo}-${env}-${name} from GITHUB_REPOSITORY.
      • Their trust policies check only aud, repository_owner_id and sub (pull_request / environment:<env>), with no workflow_ref condition, so the roles keep trusting a run that comes through a reusable workflow.
      • The account is still the environment's vars.BE_AWS_ACCOUNT_ID.
    2. Region. vars.AWS_REGION || needs.plan.outputs.region, where region comes from beplus.estate.json → defaultRegion. That gives us-east-1, eu-central-1 and us-east-1, each matching its copy's literal.
    3. Sticky-comment marker. This difference was not in the issue. The marker is naming.product lower-cased (be / mountaineer / mrkit), exactly what each copy used, so open PRs get their existing comment edited instead of a second one.
  • A new last step in the plan job (id: estate) reads and validates the region and the marker. The marker reaches github-script only through env, never pasted into the script.
  • Every other step body is byte-identical to the copies, except the CLI version: vars.BE_CLI_VERSION || '2' becomes vars.BE_CLI_VERSION, so the repository's pin is installed via be auto (BE-213).
  • It cannot deploy.
    • mode: diff and the -cdk-diff suffix are literals.
    • No input is declared, so GitHub refuses any with: a caller passes.
    • The …-cdk-diff role can only reach the CDK lookup role, and …-cdk-deploy does not trust pull_request.
  • scripts/verify-callers.mjs gains an infra-diff subcommand.
    • It resolves the shared jobs for each estate and compares them with the file the caller replaced: every job key, every step, every run/script byte.
    • The only differences it allows are the declared ones: the three derivations, the BE-213 pin, and the added step, outputs and env.
    • It also checks that the shared workflow can only diff.
    • The R2-1 library mode is unchanged.
  • README.md documents the workflow and the ~30-line caller. The trigger paths, the permissions and the concurrency group stay in each repository.

Gates

Command Result
node scripts/verify-callers.mjs infra-diff --repo ~/Git/github.com/bepluscloud/ai --ref origin/dev --caller <bepluscloud caller> --repo ~/Git/github.com/ironmountain-cz/monorepo --ref origin/dev --caller <mountaineer caller> --repo ~/Git/github.com/mrkithq/monorepo --ref dev --caller <mrkit caller> (Node 22.14) exit 0. The shared-workflow safety check passes. In all 3 estates, the caller's name, on, permissions and concurrency are identical to the original, with one job calling @v2 and secrets: inherit. Per estate, 10 steps: 6 identical, 1 identical once derived, 2 intended (BE-213 pin), 1 added, 0 divergent. Total: 18 identical, 3 derived, 6 intended, 3 added, 0 divergent.
The same gate re-run when this PR was prepared (bepluscloud 3065b04, Mountaineer origin/dev now d8d997d3c, MRKIT 1c28cd1d1) exit 0, same counts, 0 divergent. Each caller is byte-identical to the infra-diff.yml on that estate's P0 branch.
verify-callers infra-diff against nine mutations on scratch copies: shared mode: deploy; shared -cdk-deploy role; shared mode input wired to cdk-package; shared step body edited; shared step dropped; caller passes with: mode: deploy; caller loses a trigger path; caller adds a cdk-package deploy job; wrong --repository All 9 exit 1 with the expected failure. The unmutated copy exits 0.
beplus infra lint --path <scratch estate> (CLI v2.11.0 from cli_v2 source; scratch = .github + beplus.estate.json of each estate) For bepluscloud, Mountaineer and MRKIT, three forms pass: the original, the caller in place, and the caller header with the shared jobs inlined. The inlined form with mode: deploy fails (exit 2): the PR path would run the composite in deploy mode. The issue's acceptance check, that lint still fails a branch adding a deploy mode, holds.
Companion cli_v2 BE-218 commit eab15d0: lint now also checks a caller infra lint on each estate passes with the original and with the caller. It exits 2 with exactly one named failure for each of: a mode under with:, a role-arn under with:, another repository's workflow, and a missing id-token: write. Run on all 3 estates. plugin-infra 175/175, core/cli 601/601, golden-diff 0 divergent.

Deviations

  • A third derived value, the comment marker. The issue named only the role and the region. Without the marker, every open PR would get a second comment.
  • No inputs at all. The issue said the role comes from github.repository plus the manifest. It turned out to come entirely from the calling repository, the same way GitHubOIDCStack names it.
  • The callers are ~30 lines, not ~15. The trigger paths, the permissions and the concurrency group are per repository and stay there. Each estate's diff is +9 / −163.
  • CLI version. vars.BE_CLI_VERSION || '2' becomes vars.BE_CLI_VERSION, the BE-213 behaviour.
    • Until an estate commits cli.version (P3), it installs latest with a float warning.
    • Today that is 2.11.0, the same version be 2 resolves to.
  • Old copies. I did not delete the estates' old copies here. Each estate's P0 branch replaces its own infra-diff.yml with the caller.

Companion commits (same P0 branch name in each repository)

  • bepluscloud/ai fde4997, ironmountain-cz/monorepo f974307d5, mrkithq/monorepo ccbd2672d: ci(infra-diff): a pull request's cdk diff comes from setup-beplus's shared workflow (BE-218). In each, infra-diff.yml becomes the caller.
  • beplus/cli_v2 eab15d0 (BE-218: infra lint checks callers) and 5032316 (BE-213: manifest cli.version, plus a lint warning when it is missing).
  • beplus/be 4aa9ea2 (BE-213: be auto reads beplus.estate.json → cli.version).

Waiting on Igor (in order)

  1. Merge the beplus/be be auto PR first (BE-213, 4aa9ea2).
    • Run bin/bump minor on its branch, then scripts/release/verify-version.sh pr origin/main feature/estate-consolidation-p0-foundations. Expected: "Merging this PR releases @beplus/be@0.10.0".
    • Until it merges, this repository's Test cli pinned legs stay red, because cli installs be from beplus/be@main.
  2. Push this branch and open this PR. The agent App cannot push .github/workflows changes.
  3. Run "Test cli" on this PR (or re-run it after step 1). Expected: all 6 legs green.
    • pinned, and pinned from a package directory: "beplus CLI 2.10.0, as expected".
    • Explicit version: 2.9.0.
    • No pin, and no manifest: two latest legs.
    • Inexact pin: "The action refused the inexact pin".
  4. Squash-merge this PR, then move the v2 tag: git tag -f v2 <merge-sha> && git push -f origin v2.
    • After the move, cli@v2, cdk-package@v2 and .github/workflows/infra-diff.yml@v2 exist and honour the pin.
    • It is safe before the estates pin: no estate has a cli section yet, so they only gain the float warning.
  5. Only after the tag move, merge the estate infra-diff callers in estate order: bepluscloud → Mountaineer → MRKIT. MRKIT is not pushable by the agent; Igor pushes it.
    • On one real infra PR per estate, check that the sticky comment is edited in place (same marker) and its diff is identical to the one before the switch. This is BE-218's acceptance item.
  6. Later, P3: each estate adds "cli": {"version": "2.11.0"} to beplus.estate.json and drops explicit BE_CLI_VERSION: ${{ vars.BE_CLI_VERSION || '2' }} in infra.yml. Until then, today's explicit value wins over a pin, with a warning.

beplus-agent and others added 2 commits October 6, 2026 11:22
… (BE-213)

A CLI release must never change what a repository's CI does without a commit in that
repository, and `cli` defaulted to `latest`. Now, with no `BE_CLI_VERSION`, it installs the
version pinned in the nearest beplus.estate.json → cli.version by running `be auto` — the same
command a laptop, a Docker build stage or a buildspec runs, so the pin is resolved in one
place — and then fails unless `beplus --version` is exactly that pin. An inexact or unreadable
pin fails the run; it never falls back to `latest`.

An explicit `BE_CLI_VERSION` still wins (with a warning when it differs from the pin). With no
pin the action keeps today's behaviour, `be latest`, and says with a `::warning::` that the
repository floats. `BE_WORKING_DIRECTORY` says where the nearest manifest is looked up from
(default: the workspace).

`cdk-package` installs the CLI through `cli`; its `cli-version` default goes from `latest` to
empty so it honours the pin too (its note about `latest` being v1 is stale: latest is 2.11.0).
The four reusable workflows stop passing an explicit `latest`, which would override a pin;
`vars.BE_CLI_VERSION` still wins when it is set.

`test-cli.yml` runs this branch's action against fixtures in test/cli: a pin installs exactly
it (2.10.0 — not the newest release, so a run that floated cannot pass), also from a package
directory below the manifest; BE_CLI_VERSION wins; no pin and no manifest float on latest; an
inexact pin (`2`) fails the run. The pinned legs go green once `be auto` reads
beplus.estate.json on beplus/be@main.

Co-Authored-By: Igor Lamos <igor@be.plus>
… workflow (BE-218)

bepluscloud/ai, ironmountain-cz/monorepo and mrkithq/monorepo each carried a 186-line
infra-diff.yml. Their jobs differed in three values, and all three are derived now, so the
workflow has no inputs:

- the role github-actions-<owner>-<repo>-<env>-cdk-diff, from the calling repository
  (github.repository_owner + github.event.repository.name), as each estate's
  GitHubOIDCStack names it; the account is still vars.BE_AWS_ACCOUNT_ID of the environment;
- the region, from beplus.estate.json defaultRegion (vars.AWS_REGION still wins);
- the sticky-comment marker, from naming.product lower-cased: be / mountaineer / mrkit,
  exactly the markers the copies used, so the switch edits a PR's existing comment.

A plan step reads the last two from the manifest, validates them, and hands the marker to
the comment script through env, never pasted into it. Every other step body is
byte-identical to the copies, bar the CLI version: `vars.BE_CLI_VERSION || '2'` floated on
the newest 2.x; empty now installs the repository's pin through `be auto` (BE-213).

It cannot deploy: the composite always runs `mode: diff` as the read-only …-cdk-diff role,
both literals, and with no input declared a caller that passes `mode` is refused by GitHub.
The trigger paths, the permissions and the concurrency group stay in each repository's
~30-line caller.

scripts/verify-callers.mjs gains `infra-diff`: it compares the shared jobs, resolved for an
estate, with the file its caller replaced — every job key, every step, every run/script
byte — allowing only declared differences (three derivations, the BE-213 pin, the added
manifest step/outputs/env), and checks the shared workflow can only diff. The three callers
against bepluscloud origin/dev, Mountaineer origin/dev and MRKIT dev: 30 steps, 18
identical, 3 identical once derived, 6 the intended pin, 3 added, 0 divergent. Nine
mutations (deploy mode, deploy role, a mode input, an edited or dropped step, a caller
passing mode, a lost path, an extra job, the wrong repository) all go red. The R2-1
library mode is unchanged.

Co-Authored-By: Igor Lamos <igor@be.plus>
@beplus-agent-claude

Copy link
Copy Markdown
Contributor

Closing so the agent opens this PR under its own App identity (bea), as agreed. Same branch, title and body — the replacement links back here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants