feat(collector): add USD-M reference host gate - #263
Conversation
📝 WalkthroughWalkthroughAdds a USD-M reference shadow gate, deterministic end-to-end tests, and ACR workflow packaging and contract checks for the new gate asset. ChangesUSD-M reference shadow gate
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant ShadowGate
participant Systemd
participant Spool
participant ArtifactVerifier
participant GateEvidence
ShadowGate->>Systemd: inspect active unit and collector identity
ShadowGate->>Spool: scan artifact manifests and health.json
ShadowGate->>ArtifactVerifier: verify artifact data and manifest bindings
ShadowGate->>GateEvidence: write gate.json and production pass marker
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Checkov (3.3.8).github/workflows/acr-publish.ymlTraceback (most recent call last): Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 89a76f6a7c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| @@ -0,0 +1,245 @@ | |||
| #!/usr/bin/env bash | |||
There was a problem hiding this comment.
This regression suite is not invoked by any workflow or other test script. I checked the repository-wide references and the control-plane step in .github/workflows/ci.yml; it runs four older deployment suites but never test-binance-usdm-reference-host-gate.sh. Consequently, changes can break the production gate while CI still passes, despite this being the only suite exercising its host-level behavior.
Useful? React with 👍 / 👎.
| jq ".observed_at_ns=$((start_ns + 92000000000))" "$third_manifest" \ | ||
| >"$third_manifest.tmp" && mv "$third_manifest.tmp" "$third_manifest" |
There was a problem hiding this comment.
When this counterexample mutates the latest manifest timestamp, its SHA changes but health.json.manifest_sha256 is left bound to the original manifest. The runner therefore rejects the fixture at the health-binding check before testing artifact continuity, so removing or breaking the 90-second gap validation would still leave this test green. Update the fixture's canonical paths and health binding so the excessive gap is the sole rejection reason.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@deployment/aliyun/binance-usdm-reference-shadow-gate.sh`:
- Around line 53-56: Update the preflight validation near the
command-availability loop to require jq decimal-number support before processing
nanosecond epoch values. Invoke jq with have_decnum and fail through die unless
it reports true, preserving the existing missing-command checks and ensuring
downstream artifact filtering and timestamp calculations only run with a
precision-safe jq build.
🪄 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: b5d39a17-7ca6-4756-a52a-8d9191b8c211
📒 Files selected for processing (5)
.github/workflows/acr-publish.ymldeployment/aliyun/binance-usdm-reference-shadow-gate.shdeployment/aliyun/test-binance-usdm-reference-host-gate.shdeployment/aliyun/test-binance-usdm-reference-release-contract.shdeployment/aliyun/test-binance-usdm-reference-shadow-gate.sh
| for command in awk cmp date dirname find jq mkdir mktemp readlink rm sha256sum \ | ||
| sleep sort stat systemctl tr; do | ||
| command -v "$command" >/dev/null 2>&1 || die "missing required command: $command" | ||
| done |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
jq have_decnum builtin which version introduced default enabled
💡 Result:
The builtin function have_decnum was introduced in jq 1.7 [1]. It is enabled by default in the standard build [2][3], as it serves to identify whether the jq binary was compiled with decNumber support (which maintains decimal precision for literals) [3][1]. Users can explicitly disable this support at compile time using the --disable-decnum configuration option [2][3]. Because decnum is enabled by default in official distributions, have_decnum typically returns true [3].
Citations:
- 1: https://fossies.org/diffs/jq/1.7.1_vs_1.8.0/jq.1.prebuilt-diff.html
- 2: https://jqlang.org/manual/
- 3: https://manpages.debian.org/testing/jq/jq.1.en.html
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Repository files matching shadow gate:"
git ls-files | grep -F deployment/aliyun/binance-usdm-reference-shadow-gate.sh || true
echo
echo "Relevant source lines:"
sed -n '45,70p' deployment/aliyun/binance-usdm-reference-shadow-gate.sh
sed -n '288,305p' deployment/aliyun/binance-usdm-reference-shadow-gate.sh
sed -n '325,345p' deployment/aliyun/binance-usdm-reference-shadow-gate.sh
echo
echo "Policy files and jq usage references:"
git ls-files | grep 'binance-usdm.*policy\|binance-usdm.*gate' | sed -n '1,80p'
rg -n "observed_at_ns|last_success_at_ns|have_decnum|decnum|gate-policy|timing|gate.json" deployment/aliyun || trueRepository: proerror77/monday
Length of output: 25680
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "jq availability and version details:"
if command -v jq >/dev/null 2>&1; then
jq --version || true
jq -n 'have_decnum' || true
jq -n '"observed_at_ns_value": 1700000000000000000' || true
jq -n '.observed_at_ns == floor and .observed_at_ns > 0' <<'EOF'
{"observed_at_ns": 1700000000000000000}
EOF
else
echo "jq not available"
fi
echo
echo "Package version constraints:"
for f in Dockerfile* *.Dockerfile docker-compose*.yaml docker-compose*.yml requirements.txt pyproject.toml poetry.lock uv.lock Cargo.lock package-lock.json pnpm-lock.yaml yarn.lock Makefile; do
if [ -e "$f" ]; then
echo "--- $f ---"
rg -n "jq|jq-|jqlang|version|image|apt|apk|install" "$f" | sed -n '1,120p' || true
fi
doneRepository: proerror77/monday
Length of output: 454
Add a jq decnum guard before relying on nanosecond epochs.
observed_at_ns and last_success_at_ns are around 1.7e18, which exceeds safe integer precision on older/decnum-disabled jq builds. If jq reads/corrupts those literals, artifact filtering, evidence timestamp binding, and discontinuity math can fail or write incorrect evidence. Gate preflight on jq -n 'have_decnum' == true, or pin a jq ≥ 1.7+ build that preserves decimal integer literals.
🤖 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 `@deployment/aliyun/binance-usdm-reference-shadow-gate.sh` around lines 53 -
56, Update the preflight validation near the command-availability loop to
require jq decimal-number support before processing nanosecond epoch values.
Invoke jq with have_decnum and fail through die unless it reports true,
preserving the existing missing-command checks and ensuring downstream artifact
filtering and timestamp calculations only run with a precision-safe jq build.
Part of #222.
Change contract
Compile immutable, production-eligible USD-M reference shadow-gate evidence by read-only observation of one already-running, release-pinned collector and canonical Rust artifact verifier.
Out of scope
Dependency / merge order
Depends on merged PR #262, which packages the strict read-only Rust artifact verifier. Merge after #262; no stacked unmerged dependency remains.
Focused validation
Counterexamples reject tampered data, symlinked manifests, InvocationID replacement, historical API errors, artifact gaps over 90 seconds, and verifier SHA drift. Test-only gates cannot set passed/production_eligible or publish PASSED.sha256.
Rollout / rollback impact
Rollout packages the read-only runner in the dedicated USD-M reference control bundle. It does not mutate a host. A separately authorized single controller must later install/start the isolated shadow service and run the >=3600 second gate. Rollback removes the runner from the dedicated bundle; existing collector and research behavior are unchanged.
Summary by CodeRabbit
New Features
Tests