promote: foundation batch and platform gates - #20
Conversation
…-batch feat: harden foundation local infrastructure and delivery gates
|
Warning Review limit reached
Next review available in: 5 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR hardens CI workflows, AWS infrastructure, local service management, telemetry parsing, artifact CLIs, and execution policy validation. It also updates operational evidence, development instructions, and foundation handoff records. ChangesFoundation quality and infrastructure
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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: 10
🧹 Nitpick comments (12)
tools/repo-cli/test/sbom.test.mjs (1)
20-33: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAssert that the requested SBOM file exists.
The test does not inspect
outputafter the generator exits. Assert that the file exists, and preferably parse it, so the test verifies the explicit output-path contract.🤖 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 `@tools/repo-cli/test/sbom.test.mjs` around lines 20 - 33, Update the explicit-output test around “SBOM generation writes to an explicit output path” to assert that the requested output file exists after the process succeeds, and parse its contents as JSON to validate it is a readable SBOM artifact before the temporary directory is removed.tools/repo-cli/test/provenance.test.mjs (1)
11-29: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAssert the recorded SHA-256 digests.
The test verifies only sorted subject paths. A regression that emits an incorrect or missing digest still passes. Compute the expected digests for
aandz, then assert the corresponding provenance subject values.🤖 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 `@tools/repo-cli/test/provenance.test.mjs` around lines 11 - 29, Extend the test around provenance.subject in “provenance generation records sorted artifact digests” to compute SHA-256 digests for the fixture contents “a” and “z”, then assert each sorted subject’s recorded digest value matches the corresponding expected digest. Keep the existing path-order assertion and verify the digest field used by the provenance output.tools/repo-cli/test/ci-policy.test.mjs (1)
63-169: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract a shared fixture builder to reduce duplication.
The tests at Line 63-113 and Line 115-169 each rebuild nearly identical
quality.yml,security.yml, andrelease.ymlfixtures, differing only in the one property under test. A shared helper that returns valid baseline fixtures, with an override for the property under test, would remove this duplication and make future required-workflow changes easier to keep in sync across tests.♻️ Example helper approach
function validWorkflowSet(checkout, overrides = {}) { const base = { 'quality.yml': [ 'name: q', 'permissions:', ' contents: read', 'jobs:', ' check:', ' runs-on: ubuntu-24.04', ' timeout-minutes: 10', ' steps:', ` - uses: ${checkout}`, ' with:', ' persist-credentials: false', ].join('\n'), // security.yml, release.yml similarly... }; return { ...base, ...overrides }; }🤖 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 `@tools/repo-cli/test/ci-policy.test.mjs` around lines 63 - 169, Extract the duplicated workflow fixture construction from the timeout and artifact policy tests into a shared validWorkflowSet helper, using the checkout reference and optional overrides for test-specific changes. Keep the shared quality.yml, security.yml, and release.yml definitions valid by default, then override only the timeout or artifact property needed by each test while preserving their existing assertions.tools/repo-cli/src/local-services.mjs (4)
67-76: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the unreachable comment guard and trim value whitespace.
The key group is
([A-Z][A-Z0-9_]*), somatch[1]can never start with#. Thematch[1].startsWith('#')test is dead code. Also,(.*)\s*$is greedy, so.*consumes trailing whitespace and the trailing\s*matches nothing. A value written asPOSTGRES_PORT=5432keeps its trailing spaces.♻️ Proposed change
for (const line of readFileSync(file, 'utf8').split(/\r?\n/u)) { - const match = /^\s*([A-Z][A-Z0-9_]*)\s*=\s*(.*)\s*$/u.exec(line); - if (!match || match[1].startsWith('#')) continue; - values.set(match[1], match[2].replace(/^(['"])(.*)\1$/u, '$2')); + const match = /^\s*([A-Z][A-Z0-9_]*)\s*=\s*(.*?)\s*$/u.exec(line); + if (!match) continue; + values.set(match[1], match[2].replace(/^(['"])(.*)\1$/u, '$2')); }🤖 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 `@tools/repo-cli/src/local-services.mjs` around lines 67 - 76, Update parseEnvFile to remove the unreachable match[1].startsWith('#') guard and ensure captured environment values have trailing whitespace trimmed before quote removal. Preserve the existing key matching and quoted-value handling.
389-419: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
persistence-checkleaves the sentinel key if a later step fails.The
SETusesEX 300, so the key expires. That bound is good. However, if theGETcomparison fails,failthrows before theDELruns, and the key stays until expiry. The behavior is safe because of the TTL. Consider atry/finallyaround the delete so the check is self-cleaning in every path.🤖 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 `@tools/repo-cli/src/local-services.mjs` around lines 389 - 419, Update the persistence-check flow around runDocker and fail so the Redis sentinel cleanup runs in a finally block even when the GET comparison or readiness step fails. Keep the existing TTL and success logging behavior, while ensuring DEL executes on every path after the sentinel is created.
176-209: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value
portAvailablechecks only the loopback interface.
server.listen({ host: '127.0.0.1', port })detects a conflict only when another process binds127.0.0.1or0.0.0.0. Compose publishes the same ports on all interfaces. A process bound to a specific non-loopback address passes this preflight and then breaksdocker compose up. This is acceptable for a local preflight, but record the limit so a later failure is easier to diagnose.🤖 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 `@tools/repo-cli/src/local-services.mjs` around lines 176 - 209, Document the interface limitation in or immediately above portAvailable: its 127.0.0.1 probe does not detect conflicts on specific non-loopback addresses, even though Compose publishes ports on all interfaces. Keep the existing preflight behavior unchanged and state that such undetected conflicts may only surface when docker compose up runs.
115-144: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd a timeout to every
spawnSynccall.Neither
runDockernorrequireDockersetstimeout. If the Docker daemon becomes unresponsive,docker info,docker compose config, ordocker compose up -dblocks the process forever. The CLI then hangs in local use and in any CI job that calls it. Set a boundedtimeoutand map the resultingETIMEDOUTerror to a clear message.♻️ Proposed change
+const dockerTimeoutMs = 120_000; + function runDocker(args, { allowFailure = false } = {}) { - const result = spawnSync('docker', args, { cwd: repositoryRoot, encoding: 'utf8' }); + const result = spawnSync('docker', args, { + cwd: repositoryRoot, + encoding: 'utf8', + timeout: dockerTimeoutMs, + });const result = spawnSync('docker', ['info', '--format', '{{.ServerVersion}}'], { cwd: repositoryRoot, encoding: 'utf8', + timeout: 30_000, });Note that
up -dand image pulls can be slow, so choose the value forrunDockerwith that in mind.🤖 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 `@tools/repo-cli/src/local-services.mjs` around lines 115 - 144, Update every spawnSync invocation in runDocker and requireDocker to use bounded timeouts, choosing a sufficiently generous limit for docker compose up and image pulls. Detect timeout results, including ETIMEDOUT, and route them to clear fail messages distinguishing the Docker operation from daemon unavailability; preserve existing ENOENT and nonzero-status handling for other failures.docs/operations/foundation-local-infrastructure-2026-08-02.md (1)
13-16: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd
persistence-checkto the command list.The list names
config,preflight,check,start,stop,reset,restart-check,status,logs, andsmoke. It omitspersistence-check, which Line 23 of this file then describes in detail. Add it so the boundary list matches the CLI intools/repo-cli/src/local-services.mjs.🤖 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 `@docs/operations/foundation-local-infrastructure-2026-08-02.md` around lines 13 - 16, Update the local-services command list in the introductory documentation to include persistence-check alongside the existing commands, matching the CLI commands described later and implemented by local-services.mjs.tools/repo-cli/test/local-infrastructure.test.mjs (3)
40-40: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the network-membership count as well.
Lines 36 and 45 count
init: trueandlogging: *default-loggingat seven occurrences each. Line 40 only proves thatnetworks: [local]appears once. A service that loses its network membership still passes. Use the same counting form for symmetry.💚 Proposed change
- assert.match(compose, /networks: \[local\]/u); + assert.equal((compose.match(/^\s{4}networks: \[local\]$/gmu) ?? []).length, 7);🤖 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 `@tools/repo-cli/test/local-infrastructure.test.mjs` at line 40, Update the network assertion in the local infrastructure test to count all occurrences of the networks: [local] configuration, matching the counting style used for init: true and logging: *default-logging, and assert the expected service-membership count rather than merely checking presence.
214-219: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winA conditional assertion can silence a real failure.
If
composeConfig.statusis non-zero, the block runs no assertion at all. The test then passes on a machine without the Docker CLI and on a machine wheredocker compose configrejectscompose.yml. Distinguish the two cases: treat a missing Docker CLI as a skip, and treat a Compose validation error as a failure.💚 Proposed change
const composeConfig = spawnSync(process.execPath, [helpScript, 'config'], { cwd: repositoryRoot, encoding: 'utf8', }); - if (composeConfig.status === 0) - assert.match(composeConfig.stdout, /Compose configuration is valid/u); + const composeOutput = `${composeConfig.stdout}\n${composeConfig.stderr}`; + if (composeConfig.status === 0) { + assert.match(composeConfig.stdout, /Compose configuration is valid/u); + } else { + assert.match(composeOutput, /Docker CLI is not installed/u, composeOutput); + }🤖 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 `@tools/repo-cli/test/local-infrastructure.test.mjs` around lines 214 - 219, Update the composeConfig check in the local infrastructure test to explicitly detect a missing Docker CLI and skip only in that environment; for any other non-zero status from the helpScript config invocation, fail the test with the command’s stderr or equivalent error details. Keep the existing success assertion for valid Compose configuration.
148-156: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftSource-text assertions do not verify behavior.
These assertions read
local-services.mjsas a string and match identifiers such asstatfsSync,portAvailable, andconfigured = new Map. A match proves only that the text exists. The test still passes if the function is defined and never called, or if a caller is removed. The same applies to thedoesNotMatchguards: a rename todockerRemovewould satisfy/docker\s+(?:rm|volume\s+rm|system\s+prune)/iuwhile the destructive behavior remains.The negative guards on
--volumesandFLUSHALLstill have value as a cheap policy gate. Keep them, but add unit coverage that importsmainand asserts observable behavior for the argument-validation and command-dispatch paths. That coverage would also catch thelogsoutput defect noted intools/repo-cli/src/local-services.mjs.Also applies to: 220-225
🤖 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 `@tools/repo-cli/test/local-infrastructure.test.mjs` around lines 148 - 156, Replace the positive source-text assertions in the local infrastructure test with unit coverage that imports main and exercises observable argument-validation and command-dispatch behavior, including the logs output path. Retain the negative --volumes and FLUSHALL policy guards, while removing or limiting identifier-presence checks that do not verify execution; ensure the tests detect removed callers and renamed destructive Docker operations through behavior.infrastructure/local/compose.yml (1)
17-19: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winConsider binding published ports to the loopback interface.
The published ports use the short form, so Docker binds them to
0.0.0.0. The stack ships default credentials such asdatabreeze-local-change-me. On a shared or untrusted network, PostgreSQL, Redis, and MinIO become reachable from other hosts. Prefix the host side with127.0.0.1to keep the stack local.🔒 Example for the PostgreSQL mapping
ports: - - '${POSTGRES_PORT:-5432}:5432' + - '127.0.0.1:${POSTGRES_PORT:-5432}:5432'Apply the same change to the Redis, MinIO, Mailpit, and OpenTelemetry mappings. If remote access from another device is a deliberate local-development need, keep the current form and record that decision in
infrastructure/local/README.md.Also applies to: 38-39, 58-60, 97-99, 115-118
🤖 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 `@infrastructure/local/compose.yml` around lines 17 - 19, Update the port mappings in the local Compose service definitions for PostgreSQL, Redis, MinIO, Mailpit, and OpenTelemetry to bind the host side explicitly to 127.0.0.1, preserving each existing host-port variable and container-port mapping. If remote access is intentionally required, leave the mappings unchanged and document that decision in the local README instead.
🤖 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 `@docs/operations/foundation-local-infrastructure-2026-08-02.md`:
- Around line 30-36: Move the persistence-check command from the Passed list to
the Environment-gated list in the FND-003 evidence record, preserving its
Docker-gated annotation. Keep the other passed commands unchanged and ensure the
final lists accurately reflect that persistence-check did not run without a
Docker daemon.
In `@infrastructure/aws/modules/compute/variables.tf`:
- Around line 90-99: Update the validation for worker_memory, in coordination
with worker_cpu, to accept only AWS-supported Fargate CPU/memory combinations
before aws_ecs_task_definition.worker is created. Preserve the existing overall
memory bounds while enforcing each var.worker_cpu value’s allowed memory range
or values, including rejecting combinations such as 1024 CPU with 512 MiB.
In `@infrastructure/local/.env.example`:
- Line 5: Update parseArguments/main in the local-services CLI so minFreeGib
reads DATABREEZE_MIN_FREE_GIB from the parsed environment Map, while preserving
process.env as the higher-priority value and the existing default when neither
is set. Do not move this logic into environment(), since that is resolved after
argument parsing.
In `@packages/telemetry/src/v1.ts`:
- Around line 169-175: Update packages/telemetry/src/v1.ts at lines 169-175 in
ownDataEntries to catch Object.keys and getOwnPropertyDescriptor failures and
return an empty sanitized attribute record; at lines 200-205, convert
reflective-read exceptions into the existing stable strict-validation rejection;
and at lines 266-277, convert reflective-read exceptions into “Unreadable
telemetry ... header” while accepting direct header values only when typeof
value is string or when they are arrays. Add regression tests covering throwing
Proxy traps and {"x-correlation-id": 1}.
In `@services/engine/src/databreeze_engine/telemetry.py`:
- Around line 183-191: Update the attribute-validation block around
_validate_key and _safe_scalar to distinguish locally raised validation
ValueErrors from provider failures, normalizing all exceptions originating from
attributes.items(), iteration, or values to “telemetry attributes are not
readable”; apply the same change to the header-validation block at
services/engine/src/databreeze_engine/telemetry.py lines 259-276, normalizing
provider failures to “telemetry headers are not readable”. Add tests covering
items() raising ValueError("provider cause must not escape") at both sites and
verify the provider message is not exposed.
In `@tools/repo-cli/src/check-aws-infrastructure.mjs`:
- Around line 61-66: Update the infrastructure checks around the existing
Terraform regexes to detect wildcard principals in structured principals blocks,
including the rendered aws_s3_bucket_policy.web policy when applicable, and
replace the 400-character ingress scan with logic that evaluates each complete
ingress block regardless of length. Preserve the existing fail messages and
reject unrestricted 0.0.0.0/0 ingress and wildcard principals in all supported
forms.
In `@tools/repo-cli/src/check-ci-policy.mjs`:
- Around line 33-64: Update the workflow validation around assertBoundedJobs and
assertArtifactOutputs to parse each workflow as YAML before applying policy
checks. Validate the parsed top-level permissions map contains contents: read,
require timeout-minutes on every runner job, and inspect each upload-artifact
step’s with mapping for if-no-files-found: error. Replace the regex-based text
matching that truncates step and job context while preserving the existing
policy errors and checks.
- Around line 94-96: Update the release-gate configuration associated with
check-ci-policy so the protected release environment exists and has required
reviewers and deployment branch restrictions configured; ensure the existing
release.yml validation remains aligned with that protected environment.
In `@tools/repo-cli/src/local-services.mjs`:
- Around line 115-127: Update runDocker and the logs command path so log output
is streamed directly to the terminal rather than captured in
result.stdout/result.stderr, avoiding the default buffer limit for large
multi-service logs. Preserve captured output for commands that need to inspect
results, and add a behavioral test in the local infrastructure test suite when a
Docker-capable environment is available.
- Around line 211-223: Update inspectHealth so the docker inspect call uses
allowFailure: true, matching the existing ps call. Handle empty or failed
inspect output by returning a transient non-ready health result instead of
allowing runDocker to throw, while preserving the existing parsed state/health
result for valid output.
---
Nitpick comments:
In `@docs/operations/foundation-local-infrastructure-2026-08-02.md`:
- Around line 13-16: Update the local-services command list in the introductory
documentation to include persistence-check alongside the existing commands,
matching the CLI commands described later and implemented by local-services.mjs.
In `@infrastructure/local/compose.yml`:
- Around line 17-19: Update the port mappings in the local Compose service
definitions for PostgreSQL, Redis, MinIO, Mailpit, and OpenTelemetry to bind the
host side explicitly to 127.0.0.1, preserving each existing host-port variable
and container-port mapping. If remote access is intentionally required, leave
the mappings unchanged and document that decision in the local README instead.
In `@tools/repo-cli/src/local-services.mjs`:
- Around line 67-76: Update parseEnvFile to remove the unreachable
match[1].startsWith('#') guard and ensure captured environment values have
trailing whitespace trimmed before quote removal. Preserve the existing key
matching and quoted-value handling.
- Around line 389-419: Update the persistence-check flow around runDocker and
fail so the Redis sentinel cleanup runs in a finally block even when the GET
comparison or readiness step fails. Keep the existing TTL and success logging
behavior, while ensuring DEL executes on every path after the sentinel is
created.
- Around line 176-209: Document the interface limitation in or immediately above
portAvailable: its 127.0.0.1 probe does not detect conflicts on specific
non-loopback addresses, even though Compose publishes ports on all interfaces.
Keep the existing preflight behavior unchanged and state that such undetected
conflicts may only surface when docker compose up runs.
- Around line 115-144: Update every spawnSync invocation in runDocker and
requireDocker to use bounded timeouts, choosing a sufficiently generous limit
for docker compose up and image pulls. Detect timeout results, including
ETIMEDOUT, and route them to clear fail messages distinguishing the Docker
operation from daemon unavailability; preserve existing ENOENT and
nonzero-status handling for other failures.
In `@tools/repo-cli/test/ci-policy.test.mjs`:
- Around line 63-169: Extract the duplicated workflow fixture construction from
the timeout and artifact policy tests into a shared validWorkflowSet helper,
using the checkout reference and optional overrides for test-specific changes.
Keep the shared quality.yml, security.yml, and release.yml definitions valid by
default, then override only the timeout or artifact property needed by each test
while preserving their existing assertions.
In `@tools/repo-cli/test/local-infrastructure.test.mjs`:
- Line 40: Update the network assertion in the local infrastructure test to
count all occurrences of the networks: [local] configuration, matching the
counting style used for init: true and logging: *default-logging, and assert the
expected service-membership count rather than merely checking presence.
- Around line 214-219: Update the composeConfig check in the local
infrastructure test to explicitly detect a missing Docker CLI and skip only in
that environment; for any other non-zero status from the helpScript config
invocation, fail the test with the command’s stderr or equivalent error details.
Keep the existing success assertion for valid Compose configuration.
- Around line 148-156: Replace the positive source-text assertions in the local
infrastructure test with unit coverage that imports main and exercises
observable argument-validation and command-dispatch behavior, including the logs
output path. Retain the negative --volumes and FLUSHALL policy guards, while
removing or limiting identifier-presence checks that do not verify execution;
ensure the tests detect removed callers and renamed destructive Docker
operations through behavior.
In `@tools/repo-cli/test/provenance.test.mjs`:
- Around line 11-29: Extend the test around provenance.subject in “provenance
generation records sorted artifact digests” to compute SHA-256 digests for the
fixture contents “a” and “z”, then assert each sorted subject’s recorded digest
value matches the corresponding expected digest. Keep the existing path-order
assertion and verify the digest field used by the provenance output.
In `@tools/repo-cli/test/sbom.test.mjs`:
- Around line 20-33: Update the explicit-output test around “SBOM generation
writes to an explicit output path” to assert that the requested output file
exists after the process succeeds, and parse its contents as JSON to validate it
is a readable SBOM artifact before the temporary directory is removed.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d8330588-58ef-4a06-82cb-601328761e73
📒 Files selected for processing (46)
.github/workflows/quality.yml.github/workflows/release.yml.github/workflows/security.ymldocs/development/README.mddocs/operations/engineering-foundation-verification.mddocs/operations/foundation-batch-handoff-2026-08-02.mddocs/operations/foundation-ci-supply-chain-2026-08-02.mddocs/operations/foundation-local-infrastructure-2026-08-02.mddocs/operations/foundation-reconciliation-2026-08-02.mddocs/operations/telemetry-safety.mddocs/plans/000-platform-program.mddocs/plans/003-luna-handoff-runbook.mddocs/plans/010-engineering-foundation.mddocs/plans/execution-orchestration.jsoninfrastructure/aws/README.mdinfrastructure/aws/environments/alpha/README.mdinfrastructure/aws/environments/alpha/main.tfinfrastructure/aws/environments/alpha/variables.tfinfrastructure/aws/modules/compute/main.tfinfrastructure/aws/modules/compute/variables.tfinfrastructure/aws/modules/data/main.tfinfrastructure/aws/modules/web/main.tfinfrastructure/local/.env.exampleinfrastructure/local/README.mdinfrastructure/local/compose.ymlpackage.jsonpackages/telemetry/README.mdpackages/telemetry/src/v1.tspackages/telemetry/test/telemetry-v1.test.mjsservices/engine/src/databreeze_engine/telemetry.pyservices/engine/tests/test_telemetry.pytools/repo-cli/src/check-aws-infrastructure.mjstools/repo-cli/src/check-ci-policy.mjstools/repo-cli/src/check-execution-orchestration.mjstools/repo-cli/src/detect-change-scope.mjstools/repo-cli/src/generate-provenance.mjstools/repo-cli/src/generate-sbom.mjstools/repo-cli/src/local-services-smoke.mjstools/repo-cli/src/local-services.mjstools/repo-cli/test/aws-infrastructure.test.mjstools/repo-cli/test/change-scope.test.mjstools/repo-cli/test/ci-policy.test.mjstools/repo-cli/test/execution-orchestration.test.mjstools/repo-cli/test/local-infrastructure.test.mjstools/repo-cli/test/provenance.test.mjstools/repo-cli/test/sbom.test.mjs
| if (!/^\s+environment:\s*release\s*$/im.test(release)) { | ||
| throw new Error('release.yml must use the protected release environment'); | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
repo="$(gh repo view --json nameWithOwner --jq '.nameWithOwner')"
gh api "repos/${repo}/environments/release" \
--jq '{name, protection_rules, deployment_branch_policy}'Repository: DatabreezeService/databreeze-platform
Length of output: 333
🏁 Script executed:
set -euo pipefail
printf '%s\n' '--- candidate files ---'
git ls-files | grep -E '(^|/)(check-ci-policy\.mjs|release\.ya?ml)$' || true
printf '%s\n' '--- checker context ---'
file="$(git ls-files | grep -E '(^|/)tools/repo-cli/src/check-ci-policy\.mjs$' | head -n1)"
if [ -n "$file" ]; then
sed -n '1,150p' "$file"
fi
printf '%s\n' '--- release workflow references ---'
rg -n -C 4 'environment:|release|required_review|deployment' .github tools 2>/dev/null || trueRepository: DatabreezeService/databreeze-platform
Length of output: 24039
🏁 Script executed:
set -euo pipefail
repo="$(gh repo view --json nameWithOwner --jq '.nameWithOwner')"
printf '%s\n' '--- repository access context ---'
gh repo view "$repo" --json nameWithOwner,visibility,viewerPermission \
--jq '{nameWithOwner,visibility,viewerPermission}'
printf '%s\n' '--- configured environments ---'
gh api "repos/${repo}/environments" \
--jq '.environments[] | {name,protection_rules,deployment_branch_policy}' || true
printf '%s\n' '--- release environment response ---'
gh api --include "repos/${repo}/environments/release" 2>&1 || trueRepository: DatabreezeService/databreeze-platform
Length of output: 1698
Configure the protected release environment.
The repository has no visible release environment. The checker validates only the workflow name and cannot enforce required reviewers or deployment branch restrictions. Configure these protections before relying on this release gate.
🤖 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 `@tools/repo-cli/src/check-ci-policy.mjs` around lines 94 - 96, Update the
release-gate configuration associated with check-ci-policy so the protected
release environment exists and has required reviewers and deployment branch
restrictions configured; ensure the existing release.yml validation remains
aligned with that protected environment.
fix: address promotion review findings
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@packages/telemetry/test/telemetry-v1.test.mjs`:
- Around line 97-111: Strengthen the assertions around
sanitizeTelemetryAttributesV1, assertSafeTelemetryAttributesV1, and
correlationFromHeadersV1 to verify the exact normalized error key/message rather
than only the error type or prefix. Assert that the normalized outputs do not
expose “attribute trap cause” or “header trap cause”, while preserving the
existing safe empty-object result and expected error classes.
In `@tools/repo-cli/src/check-aws-infrastructure.mjs`:
- Around line 35-65: Update balancedBlocks to ignore Terraform line comments,
block comments, and heredoc bodies both when matching block starts and when
tracking brace depth, so braces inside those constructs cannot terminate a block
before its full contents. Preserve quoted-string handling and add fixtures
covering # comments, /* ... */ comments, and heredoc bodies containing braces.
In `@tools/repo-cli/src/check-ci-policy.mjs`:
- Around line 41-50: Update containsText so matching is restricted to node.run
command values, rather than every string in a workflow node, ensuring name, env,
and action with mappings cannot satisfy the check. Add bypass tests covering
required command text appearing only in non-run fields, while preserving matches
for actual run steps.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3a908a9a-a577-4893-b86e-0a9f4559887d
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (17)
docs/operations/foundation-ci-supply-chain-2026-08-02.mddocs/operations/foundation-local-infrastructure-2026-08-02.mdinfrastructure/aws/modules/compute/main.tfinfrastructure/local/README.mdinfrastructure/local/compose.ymlpackage.jsonpackages/telemetry/src/v1.tspackages/telemetry/test/telemetry-v1.test.mjsservices/engine/src/databreeze_engine/telemetry.pyservices/engine/tests/test_telemetry.pytools/repo-cli/src/check-aws-infrastructure.mjstools/repo-cli/src/check-ci-policy.mjstools/repo-cli/src/local-services.mjstools/repo-cli/test/aws-infrastructure.test.mjstools/repo-cli/test/local-infrastructure.test.mjstools/repo-cli/test/provenance.test.mjstools/repo-cli/test/sbom.test.mjs
🚧 Files skipped from review as they are similar to previous changes (14)
- tools/repo-cli/test/provenance.test.mjs
- infrastructure/local/README.md
- package.json
- infrastructure/local/compose.yml
- docs/operations/foundation-local-infrastructure-2026-08-02.md
- packages/telemetry/src/v1.ts
- infrastructure/aws/modules/compute/main.tf
- tools/repo-cli/test/sbom.test.mjs
- docs/operations/foundation-ci-supply-chain-2026-08-02.md
- services/engine/src/databreeze_engine/telemetry.py
- tools/repo-cli/test/aws-infrastructure.test.mjs
- tools/repo-cli/test/local-infrastructure.test.mjs
- services/engine/tests/test_telemetry.py
- tools/repo-cli/src/local-services.mjs
fix: close incremental promotion review findings
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tools/repo-cli/test/terraform-safety.test.mjs (1)
6-28: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a test case for
//line comments.
maskTerraformNonCodemasks//line comments the same way as#comments. No test exercises this branch. Add a fixture with a//comment containing a misleading brace or identifier, similar to the existing#comment case.🤖 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 `@tools/repo-cli/test/terraform-safety.test.mjs` around lines 6 - 28, Extend the test source in “Terraform block scanning ignores comments, heredocs, and braces in strings” with a // line comment containing a misleading ingress identifier or brace. Keep the existing assertions and ensure balancedBlocks still returns only the real ingress block, confirming maskTerraformNonCode handles // comments.
🤖 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.
Nitpick comments:
In `@tools/repo-cli/test/terraform-safety.test.mjs`:
- Around line 6-28: Extend the test source in “Terraform block scanning ignores
comments, heredocs, and braces in strings” with a // line comment containing a
misleading ingress identifier or brace. Keep the existing assertions and ensure
balancedBlocks still returns only the real ingress block, confirming
maskTerraformNonCode handles // comments.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f9ad944a-0ab9-467b-8b49-5690d44c3fa6
📒 Files selected for processing (6)
packages/telemetry/test/telemetry-v1.test.mjstools/repo-cli/src/check-aws-infrastructure.mjstools/repo-cli/src/check-ci-policy.mjstools/repo-cli/src/terraform-safety.mjstools/repo-cli/test/ci-policy.test.mjstools/repo-cli/test/terraform-safety.test.mjs
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/telemetry/test/telemetry-v1.test.mjs
- tools/repo-cli/src/check-ci-policy.mjs
- tools/repo-cli/test/ci-policy.test.mjs
test: cover Terraform slash comments
1 participant
Promotion summary
Promote the reviewed
devintegration branch tomainafter feature PR #19 merged successfully.This promotion contains the complete currently merged foundation baseline, including:
dev.Promotion gates
dev: hosted checks passed (repository, Android, Python, security/dependency scans, and scope detection).This PR is the only promotion review point. CodeRabbit should perform one full review here; no CodeRabbit review was requested on the feature PR.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation