feat: Leiðangr Phase 2 Backstage skeleton (DevEx envelope, OpenBao-to-Gitea catalog, TDD/BDD)#1
Merged
Merged
Conversation
Adds the root platform envelope. The doctor logic (scripts/lib/doctor.ts) is pure and TDD'd (scripts/lib/doctor.test.ts) - it checks Node, Corepack (which provides Yarn in this setup), bao, and the dev ports, and never prints secret values. A thin run-doctor.mjs wires the real IO (PATH scan, port probe) and runs via Node's native type-stripping. The Makefile exposes doctor/deps/dev/test/test-app/lint/config-check/secrets/ci, invoking yarn through corepack so no global install is needed. Test infra: a dedicated jest.envelope.config.cjs (@swc/jest transform) runs the envelope tooling plus the later BDD specs, kept separate from backstage-cli repo test which only covers packages and plugins. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The scaffold's app-config.yaml is already the zero-secret stub/dev config (sqlite in-memory, guest auth, example catalog locations, and the only env ref ${GITHUB_TOKEN} is optional). Rather than add a dead app-config.development.yaml (backstage-cli start does not auto-load it, so create-app keeps dev defaults in the base), this adds the layering templates:
- app-config.local.yaml.example: template for personal NON-secret local overrides (the real app-config.local.yaml is gitignored). Secrets belong in .env.local, never here.
- .env.example: documents the ${VAR} references and notes stub mode needs none of them.
And points `make config-check` at app-config.yaml.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Wires jest-cucumber into the envelope jest config (adds the tests/ root and a 60s timeout for the CLI-spawning scenario) and lands the Checkpoint 1 acceptance feature: - config valid without secrets - runs the real `backstage-cli config:check` and asserts it loads with no .env.local and no GITHUB_TOKEN present - doctor reports the toolchain without leaking secret values - the stub catalog source declares the example component (a pragmatic catalog-source assertion; the full startTestBackend boot is deferred, and the live in-browser boot was verified via make dev) Checkpoint 1 (zero-secret stub boot) is now BDD-proven. 6/6 envelope tests green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The only supported local secret path. dev-secrets.ts is pure and TDD'd (7 tests): resolveTarget (BAO_ADDR direct URL vs kubectl port-forward, so the app never learns which OpenBao backs it), validateKeys, and renderEnvLocal (maps KV keys to env var names, reports presence, never logs values). The thin orchestrator scripts/dev-secrets resolves the target, logs in via OIDC (browser to the existing Keycloak), reads secret/leidangr/dev, and pipes the KV JSON to run-dev-secrets.mjs, which validates the required keys and writes the gitignored .env.local using the tested logic. The app stays secret-store-agnostic (it reads only env refs from .env.local). The orchestrator and renderer are IO glue (syntax-checked here); the live end-to-end run needs the bao CLI plus an unsealed OpenBao and is exercised in the @LiVe checkpoint (plan Task 8). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Wires the OpenBao to Gitea catalog loop:
- app-config.gitea.yaml: integrations.gitea (host gitea.localhost, username + access-token-as-password from ${GITEA_USER}/${GITEA_TOKEN}) plus a url catalog location for the catalog-seed repo. Kept OUT of the base app-config.yaml so the zero-secret stub boot never reaches Gitea; loaded via `make dev-gitea`.
- run-dev-secrets renders GITEA_USER + GITEA_TOKEN (Gitea auth is username + token).
- checkpoint-2 BDD (jest-cucumber): scenarios 1-4 exercise the tested dev-secrets functions (render / validate / target resolution); the catalog scenario is a pragmatic mock (a fixture with two entities plus the app wired to a Gitea url location); the @LiVe end-to-end is tag-excluded for the Task 8 live run.
ws test leidangr: 18/18 green. config:check passes on base + gitea overlay.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Six MADR v3 ADRs plus a README, recording the decisions made across the build: 0001 modern Backstage systems + Corepack toolchain; 0002 zero-secret stub mode as the default; 0003 provider-agnostic secrets via dev-secrets (OpenBao OIDC); 0004 Gitea catalog source overlay (GitHub deferred); 0005 guest auth first (Keycloak sign-in deferred, but Keycloak is used for OpenBao OIDC); 0006 BDD from day one over TDD'd tooling. The Backstage ADR plugin stays deferred (GitHub-only today); the ADRs live as files until a GitHub-readable source exists. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The one-time setup behind make secrets -> make dev-gitea: the unseal pointer (nidavellir secrets-management.md), OpenBao OIDC auth against the siliconsaga Keycloak realm with a read-only leidangr-dev policy on secret/leidangr/dev, seeding the Gitea catalog-seed repo, and seeding gitea_user/gitea_token into the KV. Human-gated (cluster access + an unseal + a browser OIDC login), so it is a runbook rather than automation. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Replaces the stock create-app README with a real entry point for a human contributor (no AI agent assumed): what the component is, prerequisites, the zero-secret stub quickstart, the stub vs Gitea run modes, the secrets policy, a command reference, testing, and the layout. Adds docs/development/testing.md explaining the two test stacks (the envelope jest-cucumber suite vs backstage-cli) and how to add a test. Both note standalone use vs the optional GDD-workspace ws commands. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… preflight The @LiVe path is validated end to end: a secret in OpenBao -> rendered to .env.local by run-dev-secrets -> the backend fetches the private Gitea catalog-seed repo with that token -> leidangr-portal + gear-swap ingest into the catalog. Fixes and additions surfaced by the live run: - make dev-gitea: source .env.local (Backstage has no built-in dotenv) and pass ABSOLUTE config paths (the backend resolves --config relative to its own cwd, not the repo root, so relative paths failed). Recipes pinned to bash (SHELL := bash) so the dotenv sourcing is portable. - scripts/preflight-gitea.mjs: fail fast when gitea.localhost does not resolve. Node, unlike curl/git, does not special-case *.localhost (RFC 6761), so the host needs a hosts entry; the preflight prints the exact fix so a fresh machine surfaces it instead of silently coming up with an empty catalog. Wired into dev-gitea and smoke-gitea. - make smoke-gitea (scripts/smoke-gitea.sh): the headless @LiVe check. Boots only the backend, asserts both entities via the catalog API (a static smoke token generated at runtime in gitignored .dev/), logs to .dev/backend.log, and tears down. PASS/FAIL. - Docs: README prereqs + commands and docs/development/openbao-setup.md document the hosts entry and the smoke. gitignore .dev/. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughSummary by CodeRabbit
WalkthroughA new Backstage skeleton adds stub and Gitea run modes, OpenBao-based local secrets generation, doctor and smoke scripts, BDD acceptance tests, and supporting ADRs and documentation. ChangesLeiðangr Backstage Phase 2 Skeleton
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.env.example:
- Around line 9-15: The .env.example key order is violating dotenv-linter
expectations; reorder the environment entries so the GITEA_* variables come
before GITHUB_TOKEN. Update the env block in .env.example accordingly while
keeping the existing comments and values intact, so the dotenv-linter warning in
CI is resolved.
In `@docs/adrs/0003-provider-agnostic-secrets-openbao-oidc.md`:
- Around line 9-10: Clarify the offline/bootstrap wording in the ADR so it does
not imply secret retrieval works with no OpenBao available. Update the wording
around the `dev-secrets` flow to state that it still requires one reachable
OpenBao endpoint (for example via `BAO_ADDR` or a port-forward), while keeping
the app itself provider-agnostic; use the existing “OpenBao” and “dev-secrets”
language in the document to keep the meaning precise.
In `@docs/adrs/0006-bdd-from-day-one.md`:
- Around line 23-24: Update the wording in the ADR example to say that `ws test
leidangr` runs the default non-`@live` envelope + acceptance suite, not the
entire suite, since the `@live` scenarios are tag-excluded from the default run.
Use the existing “Good:” bullet text near the acceptance suite description and
adjust the phrasing so it accurately reflects what `ws test leidangr` covers
while keeping the rest of the sentence intact.
In `@Makefile`:
- Around line 27-29: The dev-gitea target currently continues even when
.env.local is absent, so update the Makefile target to fail immediately with a
clear error before launching the app. Use the existing dev-gitea recipe as the
place to enforce the check, and make it stop fast when .env.local is missing
instead of relying on optional sourcing so unset Gitea credentials never reach
corepack yarn start.
In `@scripts/dev-secrets`:
- Around line 20-25: The dev-secrets port-forward flow assumes OpenBao is ready
after a fixed sleep, which can race with a slow or failed kubectl port-forward
startup. In the dev-secrets script, replace the blind sleep after launching the
background port-forward with a readiness check that waits for the local Bao
endpoint to respond and/or verifies the background PF_PID is still alive before
proceeding to login. Keep the existing PF_PID and BAO_ADDR handling, and gate
the subsequent login path on that check so failures surface early and clearly.
In `@scripts/lib/dev-secrets.ts`:
- Around line 26-31: The validateKeys function currently only checks for key
presence, so empty required secrets like gitea_user or gitea_token still pass
through. Update validateKeys to treat each required entry as valid only when it
exists in data and is a non-empty string after trimming, so the KeyValidation
missing list includes blank values too. Keep the change localized to
validateKeys and preserve its ok/missing return shape.
- Around line 44-50: The raw assignment generation in renderEnvLocal is unsafe
for shell-sourced env files because values are written unquoted into .env.local
and later sourced by scripts/smoke-gitea.sh. Update renderEnvLocal to emit
safely shell-quoted assignments for each mapped key/value (or otherwise ensure
the file is not sourced as shell), handling metacharacters, command
substitution, and newlines correctly so the generated env file can be loaded
without executing or mangling secret values.
In `@scripts/lib/run-dev-secrets.mjs`:
- Around line 33-34: The `.env.local` write in run-dev-secrets should not rely
on the caller’s umask; update the `writeFileSync` flow so the generated secrets
file is created with restrictive `0600` permissions. When overwriting an
existing file, explicitly normalize its mode after the write so `dev-secrets`
always leaves `.env.local` private, and keep the existing logging around the
write intact.
In `@scripts/smoke-gitea.sh`:
- Around line 44-52: The smoke-gitea readiness check currently waits a fixed 18
seconds after "Listening on" and then queries the catalog only once, which can
miss late ingestion. Update the polling logic in the script around the existing
loop and RESULT fetch so it repeatedly queries the catalog endpoint until the
expected entities are present or the overall timeout is reached, rather than
relying on a single sleep-and-check. Use the existing TOKEN, LOG, PID, and
catalog request logic to keep the behavior aligned with the current script.
In `@tests/acceptance/checkpoint-1.steps.ts`:
- Around line 73-87: The current scenario only reads the fixture file in the
test body, so it does not verify stub-mode serving or guest auth. Update the
acceptance flow in the named test to actually boot the backend in stub mode and
query the catalog API for entities, then assert on the returned response for the
example component and guest ownership. If this test is intended to cover only
fixture contents, rename the scenario accordingly so the name matches the
behavior exercised.
In `@tests/acceptance/checkpoint-2.steps.ts`:
- Around line 78-93: The current acceptance test only checks fixture content and
static config in the gitea catalog scenario, so it does not verify actual Gitea
ingestion or token-backed access. Update the test in checkpoint-2.steps.ts to
exercise the real smoke/backend flow used by the suite, or if that is out of
scope, rename the step to clearly reflect that it is only a fixture/config
contract check. Use the existing test block and step definitions around the
Gitea-sourced entities scenario as the place to switch from string counting/grep
assertions to a behavior-based validation.
- Around line 29-47: The current `dev-secrets fails clearly when a required key
is missing` scenario only checks `validateKeys()` and reasserts `result.ok ===
false`, so it never verifies that no env file is created. Update this test to
exercise the env-file writing path in `run-dev-secrets.mjs` or the related
writer helper, and assert that `.env.local` is not present when `validateKeys`
fails. Use the existing `validateKeys` and `run-dev-secrets.mjs` flow to confirm
the failure prevents file output.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: a0455c81-5371-4697-9964-a24a14b52409
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (32)
.env.example.gitattributes.gitignoreMakefileREADME.mdapp-config.gitea.yamlapp-config.local.yaml.exampledocs/adrs/0001-modern-backstage-and-corepack.mddocs/adrs/0002-zero-secret-stub-mode-default.mddocs/adrs/0003-provider-agnostic-secrets-openbao-oidc.mddocs/adrs/0004-gitea-catalog-source.mddocs/adrs/0005-guest-auth-first.mddocs/adrs/0006-bdd-from-day-one.mddocs/adrs/README.mddocs/development/openbao-setup.mddocs/development/testing.mdjest.envelope.config.cjspackage.jsonscripts/dev-secretsscripts/lib/dev-secrets.test.tsscripts/lib/dev-secrets.tsscripts/lib/doctor.test.tsscripts/lib/doctor.tsscripts/lib/run-dev-secrets.mjsscripts/lib/run-doctor.mjsscripts/preflight-gitea.mjsscripts/smoke-gitea.shtests/acceptance/checkpoint-1-stub-boot.featuretests/acceptance/checkpoint-1.steps.tstests/acceptance/checkpoint-2-openbao-gitea.featuretests/acceptance/checkpoint-2.steps.tstests/acceptance/fixtures/gitea-catalog-info.yaml
2 tasks
…s) + AGENTS.md Security/correctness: - dev-secrets.ts: validateKeys rejects blank values; renderEnvLocal shell-quotes values (the .env.local is sourced as shell) — both covered by new unit tests. - run-dev-secrets.mjs: write .env.local with 0600. - scripts/dev-secrets: poll the port-forward health before login instead of a blind sleep. - scripts/smoke-gitea.sh: poll the catalog until the entities appear instead of sleeping once and querying once. - Makefile dev-gitea: fail fast when .env.local is missing. Tests/docs: - Renamed the two pragmatic catalog scenarios (checkpoint-1/2) to honest source/contract-check names; the real token-backed ingestion is covered live by `make smoke-gitea` (the @LiVe path), with a real startTestBackend port tracked as phase-3 hardening. - checkpoint-2 "no environment file is written" now actually drives run-dev-secrets and asserts no .env.local is created. - .env.example key order (dotenv-linter); ADR 0003/0006 wording precision; doctor.ts docstrings. Adds components/leidangr/AGENTS.md — agent guidance plus a "when to write an ADR" convention (Backstage-repo-local; base GDD does not use ADRs, so workspace-wide is left to an optional realm distillation effort). ws test leidangr: 20/20. make smoke-gitea: PASS. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/smoke-gitea.sh (1)
37-65: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winRegister backend cleanup with
trap.The teardown only runs on the happy path. If this script is interrupted after the backend starts, the child process keeps running and can poison later
make smoke-gitea/make dev-gitearuns by leaving port7007occupied.Suggested fix
corepack yarn workspace backend start \ --config "$ROOT/app-config.yaml" \ --config "$ROOT/app-config.gitea.yaml" \ --config "$ROOT/.dev/app-config.smoke.yaml" >"$LOG" 2>&1 & PID=$! + +cleanup() { + kill "$PID" 2>/dev/null || true + wait "$PID" 2>/dev/null || true +} + +trap cleanup EXIT INT TERM up="" for _ in $(seq 1 120); do if grep -q "Listening on" "$LOG" 2>/dev/null; then up=1; break; fi if ! kill -0 "$PID" 2>/dev/null; then break; fi @@ -kill "$PID" 2>/dev/null || true -wait "$PID" 2>/dev/null || true +trap - EXIT INT TERM +cleanup🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/smoke-gitea.sh` around lines 37 - 65, The smoke-gitea script currently only kills the backend in the normal success path, so an interruption can leave the process running and block later runs. Add a trap-based cleanup near the backend launch in smoke-gitea.sh that always runs on exit, interrupt, and termination, and make it reuse the existing PID/kill/wait teardown logic so the backend started by corepack yarn workspace backend start is reliably cleaned up even when the script aborts early.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/dev-secrets`:
- Around line 24-35: The readiness check in dev-secrets should fail closed if
the OpenBao tunnel stays alive but never responds to /v1/sys/health. Update the
polling loop around the port-forward and BAO_ADDR health probe so that after the
15 attempts it exits with an error instead of continuing to bao login; keep the
existing kill -0 and curl checks, and add a clear timeout failure message in the
same flow.
In `@tests/acceptance/checkpoint-2.steps.ts`:
- Around line 50-55: The child process in the checkpoint acceptance test is
invoking Node via the hardcoded 'node' command, which can fail when that binary
is not available on PATH. Update the execFileSync call in the test helper that
runs run-dev-secrets.mjs to use process.execPath so it reuses the current Node
interpreter and stays portable across environments.
---
Outside diff comments:
In `@scripts/smoke-gitea.sh`:
- Around line 37-65: The smoke-gitea script currently only kills the backend in
the normal success path, so an interruption can leave the process running and
block later runs. Add a trap-based cleanup near the backend launch in
smoke-gitea.sh that always runs on exit, interrupt, and termination, and make it
reuse the existing PID/kill/wait teardown logic so the backend started by
corepack yarn workspace backend start is reliably cleaned up even when the
script aborts early.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 16402053-a4da-4cd9-8396-0b6e72a8d8d2
📒 Files selected for processing (15)
.env.exampleAGENTS.mdMakefiledocs/adrs/0003-provider-agnostic-secrets-openbao-oidc.mddocs/adrs/0006-bdd-from-day-one.mdscripts/dev-secretsscripts/lib/dev-secrets.test.tsscripts/lib/dev-secrets.tsscripts/lib/doctor.tsscripts/lib/run-dev-secrets.mjsscripts/smoke-gitea.shtests/acceptance/checkpoint-1-stub-boot.featuretests/acceptance/checkpoint-1.steps.tstests/acceptance/checkpoint-2-openbao-gitea.featuretests/acceptance/checkpoint-2.steps.ts
There was a problem hiding this comment.
Pull request overview
This PR scaffolds the Phase 2 Leiðangr Backstage “skeleton” and layers on a local DevEx envelope with two modes: a zero-secret stub boot (default/CI) and an optional OpenBao→Gitea-backed catalog mode, plus an “envelope” test stack (TDD tooling + BDD acceptance specs) and accompanying documentation/ADRs.
Changes:
- Adds a Makefile-driven developer envelope (doctor/dev/dev-gitea/secrets/smoke-gitea/test/lint/config-check/ci) and supporting scripts.
- Introduces OpenBao secret rendering into gitignored
.env.local, a Gitea catalog overlay, and a headless smoke test for live ingestion. - Adds a dedicated Jest config for envelope tooling + jest-cucumber acceptance specs, plus docs and MADR ADRs describing the decisions.
Reviewed changes
Copilot reviewed 32 out of 34 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| yarn.lock | Locks new dev dependencies (SWC + jest-cucumber stack). |
| package.json | Adds devDependencies for @swc/jest and jest-cucumber. |
| Makefile | Defines the DevEx “one command per workflow” envelope. |
| README.md | Replaces scaffold README with project quickstart, modes, and command guide. |
| AGENTS.md | Adds agent-facing repo conventions (Corepack/Yarn, secrets flow, test stacks). |
| .env.example | Documents env vars referenced by config and the .env.local pattern. |
| .gitignore | Ensures .env.local and .dev/ scratch outputs aren’t committed. |
| .gitattributes | Forces LF for bash scripts to work reliably across platforms. |
| app-config.gitea.yaml | Adds the token-authenticated Gitea integration + catalog overlay. |
| app-config.local.yaml.example | Adds a gitignored local override template for non-secret config. |
| jest.envelope.config.cjs | Adds a dedicated Jest config for scripts/ + tests/ (envelope suite). |
| scripts/lib/doctor.ts | Adds pure, testable “doctor” checks for toolchain + ports. |
| scripts/lib/doctor.test.ts | Adds unit tests for doctor logic. |
| scripts/lib/run-doctor.mjs | Adds the real-IO wrapper that prints doctor results. |
| scripts/lib/dev-secrets.ts | Adds pure logic for resolving OpenBao target, validating keys, and rendering .env.local. |
| scripts/lib/dev-secrets.test.ts | Adds unit tests for dev-secrets logic (including shell-quoting). |
| scripts/lib/run-dev-secrets.mjs | Adds stdin-driven renderer/writer for .env.local using tested logic. |
| scripts/dev-secrets | Adds orchestrator script (port-forward + OIDC login + kv fetch → renderer). |
| scripts/preflight-gitea.mjs | Adds DNS preflight for gitea.localhost resolution under Node. |
| scripts/smoke-gitea.sh | Adds headless live smoke that boots backend + polls catalog API for entities. |
| tests/acceptance/checkpoint-1-stub-boot.feature | Adds BDD spec for zero-secret stub boot expectations. |
| tests/acceptance/checkpoint-1.steps.ts | Implements checkpoint-1 step definitions (config-check, doctor, stub catalog contract). |
| tests/acceptance/checkpoint-2-openbao-gitea.feature | Adds BDD spec for OpenBao secret flow + Gitea wiring (+ a @live scenario). |
| tests/acceptance/checkpoint-2.steps.ts | Implements checkpoint-2 step definitions (dev-secrets behavior + wiring contracts). |
| tests/acceptance/fixtures/gitea-catalog-info.yaml | Adds a seed template fixture for the Gitea catalog-info.yaml content. |
| docs/development/testing.md | Documents the two test stacks (envelope vs Backstage-generated tests). |
| docs/development/openbao-setup.md | Adds a runbook for the live OpenBao→Gitea setup and workflow. |
| docs/adrs/README.md | Adds ADR index and context for deferring the Backstage ADR plugin. |
| docs/adrs/0001-modern-backstage-and-corepack.md | ADR: modern Backstage systems + Corepack-managed Yarn. |
| docs/adrs/0002-zero-secret-stub-mode-default.md | ADR: base config is the zero-secret stub config. |
| docs/adrs/0003-provider-agnostic-secrets-openbao-oidc.md | ADR: provider-agnostic local secrets via dev-secrets → .env.local. |
| docs/adrs/0004-gitea-catalog-source.md | ADR: Gitea catalog source via overlay; GitHub deferred. |
| docs/adrs/0005-guest-auth-first.md | ADR: guest auth now; Backstage OIDC sign-in deferred. |
| docs/adrs/0006-bdd-from-day-one.md | ADR: BDD acceptance over TDD tooling, with @live scenarios excluded by default. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ets, trap cleanup, fixture name) CodeRabbit (3): - dev-secrets: fail closed when the port-forward stays alive but never answers /v1/sys/health, instead of falling through the readiness poll and running bao login anyway. - smoke-gitea.sh: trap-based backend teardown on EXIT/INT/TERM so an interrupted run cannot leave port 7007 occupied and poison later runs. - checkpoint-2.steps.ts: spawn the child via process.execPath instead of a PATH-resolved 'node'. Copilot (valid): - dev-secrets: cd to the component root so the relative runner path resolves and .env.local lands in the repo regardless of caller cwd. - fixture gitea-catalog-info.yaml: rename the second entity from catalog-seed to gear-swap so the seed template matches the smoke assertion and the runbook (the Gitea repo stays named catalog-seed). - doctor.ts: evaluate portFree once per port rather than twice. Copilot (false positives, no change): the three "Node will not load .ts without --experimental-strip-types" comments. Node 24 is pinned in .nvmrc and strips types by default; make doctor, smoke-gitea, and the envelope suite all run as-is. ws test leidangr: 20/20. Shell scripts pass bash -n. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/acceptance/checkpoint-2.steps.ts (1)
100-108: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAssert the expected entity names in this contract check.
This test only counts two YAML documents, so a future rename in
tests/acceptance/fixtures/gitea-catalog-info.yamlwould still passmake testand then fail later inscripts/smoke-gitea.sh, which hard-codesleidangr-portalandgear-swap. Add assertions for those names here so the fixture/smoke contract breaks on the fast path. As per coding guidelines, "Treat scenarios that assert on source, config, or fixtures as contract checks."Proposed fix
and('the seed fixture declares two entities', () => { const entityCount = (fixture.match(/^apiVersion:/gm) || []).length; expect(entityCount).toBe(2); + expect(fixture).toMatch(/name:\s*leidangr-portal/); + expect(fixture).toMatch(/name:\s*gear-swap/); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/acceptance/checkpoint-2.steps.ts` around lines 100 - 108, The acceptance check in the fixture contract test only verifies the number of YAML entities, so it can miss a rename that later breaks the smoke script. Update the test around the existing giteaConfig and fixture assertions to also verify the two expected entity names from tests/acceptance/fixtures/gitea-catalog-info.yaml, matching the hard-coded names used by scripts/smoke-gitea.sh so the contract fails early if they change.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@tests/acceptance/checkpoint-2.steps.ts`:
- Around line 100-108: The acceptance check in the fixture contract test only
verifies the number of YAML entities, so it can miss a rename that later breaks
the smoke script. Update the test around the existing giteaConfig and fixture
assertions to also verify the two expected entity names from
tests/acceptance/fixtures/gitea-catalog-info.yaml, matching the hard-coded names
used by scripts/smoke-gitea.sh so the contract fails early if they change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: db1fdca2-b1fe-4181-b20c-8b338d067ee9
📒 Files selected for processing (5)
scripts/dev-secretsscripts/lib/doctor.tsscripts/smoke-gitea.shtests/acceptance/checkpoint-2.steps.tstests/acceptance/fixtures/gitea-catalog-info.yaml
…d 3) CodeRabbit (outside-diff): - checkpoint-2 contract check now asserts the two fixture entity names (leidangr-portal, gear-swap), not just the count, so a rename breaks on the fast path instead of later in the @LiVe smoke. Matches the coding guideline to treat fixture-asserting scenarios as contract checks. Copilot (2): - smoke-gitea.sh: add set -e so setup failures (sourcing .env.local, generating the smoke config, truncating the log) fail fast instead of surfacing later as a vague entities-missing error. - smoke-gitea.sh: when the backend never logs "Listening on", fail immediately and print the log tail for actionable diagnostics, rather than falling through to the narrow error grep. ws test leidangr: 20/20. smoke-gitea.sh passes bash -n. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/smoke-gitea.sh (1)
67-81: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winMatch the ingested entity names exactly.
Lines 67-81 still use substring greps, so
smoke-giteacan pass on renamed entities likeleidangr-portal-v2. The polling loop and the final PASS gate should both match the JSONnamefield exactly before declaring success.Suggested fix
+has_entity() { + local name="$1" + printf '%s' "$RESULT" | grep -Eq "\"name\"[[:space:]]*:[[:space:]]*\"${name}\"([[:space:]]*[,}])" +} + RESULT='[]' for _ in $(seq 1 120); do RESULT="$(curl -fsS -H "Authorization: Bearer ${TOKEN}" \ "http://localhost:7007/api/catalog/entities?filter=kind=component" 2>/dev/null || echo '[]')" - if printf '%s' "$RESULT" | grep -q "leidangr-portal" \ - && printf '%s' "$RESULT" | grep -q "gear-swap"; then + if has_entity "leidangr-portal" \ + && has_entity "gear-swap"; then break fi sleep 1 done @@ pass=1 -printf '%s' "$RESULT" | grep -q "leidangr-portal" || pass=0 -printf '%s' "$RESULT" | grep -q "gear-swap" || pass=0 +has_entity "leidangr-portal" || pass=0 +has_entity "gear-swap" || pass=0🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/smoke-gitea.sh` around lines 67 - 81, The smoke test in the polling and PASS checks is using substring matching, which can falsely succeed on similarly named entities. Update the logic in smoke-gitea.sh around the RESULT checks to match the JSON name field exactly for the ingested entities, and apply the same exact-match behavior in both the loop condition and the final pass validation so only the intended entity names can satisfy the test.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/acceptance/checkpoint-2.steps.ts`:
- Around line 108-111: The rename guard in the acceptance contract check is too
loose because the current regex in the fixture assertions can still match
renamed variants like suffixed names. Update the checks in the same test block
that asserts on leidangr-portal and gear-swap so they verify the exact YAML name
entries instead of prefix matching, either by anchoring the full line in the
regex or by parsing the fixture content and comparing the extracted names
exactly.
---
Outside diff comments:
In `@scripts/smoke-gitea.sh`:
- Around line 67-81: The smoke test in the polling and PASS checks is using
substring matching, which can falsely succeed on similarly named entities.
Update the logic in smoke-gitea.sh around the RESULT checks to match the JSON
name field exactly for the ingested entities, and apply the same exact-match
behavior in both the loop condition and the final pass validation so only the
intended entity names can satisfy the test.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: f297904f-5a96-452a-bce8-7ca4722936c0
📒 Files selected for processing (2)
scripts/smoke-gitea.shtests/acceptance/checkpoint-2.steps.ts
CodeRabbit (Major): the round-3 name guards used prefix matches, so name: leidangr-portal-v2 would still pass and the check would miss the rename it exists to catch. Anchor each assertion to the whole YAML line (^\s*name:\s*<name>\s*$ with the multiline flag) so a suffixed rename fails on the fast path. ws test leidangr: 20/20. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
tests/acceptance/checkpoint-2.steps.ts (2)
100-104: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winAssert the
${GITEA_USER}wiring too.This contract only guards
${GITEA_TOKEN}.run-dev-secrets.mjsrequires and renders bothGITEA_USERandGITEA_TOKENfor Gitea auth, so dropping the username reference fromapp-config.gitea.yamlwould still let this test pass.Suggested fix
then('the overlay defines a token-authenticated Gitea url location', () => { expect(giteaConfig).toMatch(/gitea:/); + expect(giteaConfig).toMatch(/username:\s*\$\{GITEA_USER\}/); expect(giteaConfig).toMatch(/password:\s*\$\{GITEA_TOKEN\}/); expect(giteaConfig).toMatch(/type:\s*url/); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/acceptance/checkpoint-2.steps.ts` around lines 100 - 104, The Gitea overlay assertion in the checkpoint step only verifies ${GITEA_TOKEN}, so it can miss a broken username wiring. Update the expectation in the then block for the giteaConfig contract to also assert ${GITEA_USER} is present in app-config.gitea.yaml, alongside the existing token, url, and type checks. Use the existing giteaConfig matcher in the checkpoint-2.steps test to ensure both GITEA_USER and GITEA_TOKEN are required for the Gitea auth overlay.
47-59: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winExercise the actual
gitea_token-missing runner path.The child-process check currently feeds
{}and only asserts “non-zero + no file”, so it passes for any early failure and does not prove the scenario you named here. Seedgitea_user, omit onlygitea_token, and assert stderr contains the missing-key message fromrun-dev-secrets.mjs.Suggested fix
and('no environment file is written', () => { // Drive the actual writer (run-dev-secrets.mjs) with the missing-key payload in a // throwaway cwd and confirm it exits non-zero WITHOUT creating .env.local. const tmp = mkdtempSync(join(tmpdir(), 'leidangr-dev-secrets-')); const mjs = resolve(__dirname, '../../scripts/lib/run-dev-secrets.mjs'); - let failed = false; + let stderr = ''; try { - execFileSync(process.execPath, [mjs], { input: JSON.stringify({ data: { data } }), cwd: tmp, stdio: 'pipe' }); - } catch { - failed = true; + execFileSync(process.execPath, [mjs], { + input: JSON.stringify({ data: { data: { gitea_user: 'alice' } } }), + cwd: tmp, + stdio: 'pipe', + }); + } catch (error) { + stderr = String((error as { stderr?: Buffer }).stderr ?? ''); } - expect(failed).toBe(true); + expect(stderr).toContain('missing required key(s): gitea_token'); expect(existsSync(join(tmp, '.env.local'))).toBe(false); rmSync(tmp, { recursive: true, force: true }); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/acceptance/checkpoint-2.steps.ts` around lines 47 - 59, The current child-process test only proves a generic failure and does not exercise the specific gitea_token-missing path. Update the acceptance check in checkpoint-2.steps.ts so the payload includes gitea_user and omits only gitea_token, then assert the stderr from run-dev-secrets.mjs contains the missing-key message. Keep the existing non-zero exit and .env.local absence assertions, but make the test target the actual missing-token runner behavior rather than any early error.
♻️ Duplicate comments (1)
tests/acceptance/checkpoint-2.steps.ts (1)
92-92: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRename this scenario as a
contractcheck.This is still a source/config/fixture-only assertion path, so the scenario name should say
contractrather than read like live-behavior verification. As per coding guidelines, acceptance scenarios that assert on source/config/fixtures should be named ascontractchecks.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/acceptance/checkpoint-2.steps.ts` at line 92, Rename the acceptance scenario in the step definition so it is clearly a contract check rather than a live-behavior test. Update the test name in the `test(...)` call inside `checkpoint-2.steps.ts` to use “contract” wording, keeping the existing assertion logic and step bindings (`given`, `when`, `then`, `and`) unchanged.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@tests/acceptance/checkpoint-2.steps.ts`:
- Around line 100-104: The Gitea overlay assertion in the checkpoint step only
verifies ${GITEA_TOKEN}, so it can miss a broken username wiring. Update the
expectation in the then block for the giteaConfig contract to also assert
${GITEA_USER} is present in app-config.gitea.yaml, alongside the existing token,
url, and type checks. Use the existing giteaConfig matcher in the
checkpoint-2.steps test to ensure both GITEA_USER and GITEA_TOKEN are required
for the Gitea auth overlay.
- Around line 47-59: The current child-process test only proves a generic
failure and does not exercise the specific gitea_token-missing path. Update the
acceptance check in checkpoint-2.steps.ts so the payload includes gitea_user and
omits only gitea_token, then assert the stderr from run-dev-secrets.mjs contains
the missing-key message. Keep the existing non-zero exit and .env.local absence
assertions, but make the test target the actual missing-token runner behavior
rather than any early error.
---
Duplicate comments:
In `@tests/acceptance/checkpoint-2.steps.ts`:
- Line 92: Rename the acceptance scenario in the step definition so it is
clearly a contract check rather than a live-behavior test. Update the test name
in the `test(...)` call inside `checkpoint-2.steps.ts` to use “contract”
wording, keeping the existing assertion logic and step bindings (`given`,
`when`, `then`, `and`) unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 00a46423-b2b7-4b16-8957-c98e2f8c7fbf
📒 Files selected for processing (1)
tests/acceptance/checkpoint-2.steps.ts
… runner path
CodeRabbit (Major): the overlay contract only guarded ${GITEA_TOKEN}, but the runner requires both keys, so dropping username from app-config.gitea.yaml would still pass. Added an assertion for username: ${GITEA_USER}.
CodeRabbit (Minor): the missing-key scenario fed an empty payload and only checked non-zero exit plus no file, so any early failure passed. It now seeds gitea_user, omits only gitea_token, and asserts the runner stderr contains the specific missing-key message.
CodeRabbit (naming): renamed the source/config/fixture scenario to a "Contract check:" name in both the feature and the steps, per the contract-check guideline (the code comment already described it as one).
ws test leidangr: 20/20.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Comment on lines
+52
to
+64
| const tmp = mkdtempSync(join(tmpdir(), 'leidangr-dev-secrets-')); | ||
| const mjs = resolve(__dirname, '../../scripts/lib/run-dev-secrets.mjs'); | ||
| let stderr = ''; | ||
| try { | ||
| execFileSync(process.execPath, [mjs], { input: JSON.stringify({ data: { data } }), cwd: tmp, stdio: 'pipe' }); | ||
| } catch (error) { | ||
| stderr = String((error as { stderr?: Buffer }).stderr ?? ''); | ||
| } | ||
| // Prove it failed on THIS path (missing gitea_token), not any early error. | ||
| expect(stderr).toContain('missing required key(s): gitea_token'); | ||
| expect(existsSync(join(tmp, '.env.local'))).toBe(false); | ||
| rmSync(tmp, { recursive: true, force: true }); | ||
| }); |
Cervator
approved these changes
Jun 29, 2026
This was referenced Jul 6, 2026
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.
Summary
The first slice of Leiðangr Phase 2 — a fresh, modern Backstage instance with a strong local DevEx envelope. It runs entirely locally with zero secrets by default, and can pull a real secret from OpenBao to authenticate a Gitea-backed software catalog.
@backstage/create-appon the new frontend + backend systems (Yarn 4 via Corepack, Node 24).Makefile(doctor/deps/dev/dev-gitea/smoke-gitea/test/lint/config-check/secrets/ci); Yarn runs through Corepack so no global install is needed.scripts/dev-secretsresolves the OpenBao target (direct URL or kubectl port-forward), doesbao login -method=oidc(browser to the existing Keycloak), and renders a gitignored.env.local. The app stays secret-store-agnostic; the resolve/validate/render logic is TDD'd.@swc/jestconfig, separate frombackstage-cli repo test.make smoke-giteais the headless@livecheck that boots the backend and asserts the Gitea entities ingest.gitea.localhostpreflight fails fast with the fix (Node, unlike curl, does not special-case*.localhost).Test plan
make devboots Backstage in stub mode at http://localhost:3000 (guest auth, example catalog), no secrets.make testis green — 18/18 (doctor + dev-secrets units, checkpoint-1 + checkpoint-2 BDD).docs/development/openbao-setup.mdandmake secrets,make smoke-giteaPASSes (both Gitea-sourced entities ingest), andmake dev-giteashows them in the catalog.make lintclean.Related
realm-siliconsaga, branchdocs/leidangr-phase2-backstage-skeleton.