Conversation
WalkthroughChangesEnvironment deployment specification
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@SPEC.md`:
- Around line 2698-2701: Revise the environment comparison statement in the
specification to limit the shared aspect to the same application artifact.
Replace the broad “differ only” claim with wording that identifies configuration
and explicitly defined operational controls as differences, while retaining the
existing environment, credential, network, and Alpaca-account distinctions.
- Around line 2778-2791: Expand the staging telemetry section to define an
implementable contract: require the existing ENVIRONMENT value “staging” on
every log, metric, and trace; specify propagation through application
instrumentation and exporters; state that alert routing must reject staging
signals from production paging; and describe an automated validation that emits
a staging signal and verifies it cannot trigger production alerts.
- Around line 2705-2706: Update the staging and production deployment contract
so the production tag must reference the exact commit that passed staging smoke
tests, with the associated smoke-test evidence retained and traceable. Clarify
the required flow coverage by explicitly choosing and consistently defining
whether smoke testing covers all flows or only affected flows, and remove any
wording that permits production to deploy an independently created revision.
- Around line 2721-2747: Update the staging-network section to match the
validation flow: describe staging as a multichain deployment with Base and the
selected chain B both configured and exercised, including Base parity checks and
separate Ethereum asset registration. Keep the colocation requirements for the
staging vaults, liquidity bot, and Alpaca cash settlement, and clarify which
chain each validation applies to.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 7fdc5616-db07-4498-b108-7eca4d8d5f9b
📒 Files selected for processing (1)
SPEC.md
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: static
- GitHub Check: test
🧰 Additional context used
📓 Path-based instructions (2)
*
⚙️ CodeRabbit configuration file
Focus on providing constructive criticism. Whenever you see a suboptimal approach, suggest more idiomatic or robust alternative(s). Flag potential footguns. Suggest FP alternatives to mutable/imperative code. Point out architectural flaws like leaky abstractions, tight coupling, wrong level of abstraction, poor type modeling, over-abstraction, unclear domain boundaries. Code should generally be organized based on business concerns rather than technical aspects - suggest improvements if you find violations. Point out gaps in test coverage but suggest tests that are not too coupled to the implementation and actually test domain invariants and business logic
Files:
SPEC.md
**/*.md
⚙️ CodeRabbit configuration file
Focus on the contents of the docs and not on cosmetic things like markdown formatting. We use markdown files for various docs including but not limited to the north star system specification, SPEC.md, the plan for how to get there, ROADMAP.md, guidelines for AI contributors, AGENTS.md, project overview and instructions for human contributors, README.md. Think about the target audience of a document when deciding what comment to leave. For specifications and designs, suggest potential product, architectural, and UI/UX improvements. For plans, suggest changes that would make things more parallelizable and deliverable-focused. For instructions, suggest better rules and guidelines and point out missing instructions. In all cases, flag needless bloat, prefer clear concise writing, and consider the structure of the document and order of the sections
Files:
SPEC.md
🔇 Additional comments (1)
SPEC.md (1)
2708-2718: LGTM!Also applies to: 2749-2763, 2794-2802
cc75e4b to
f149c98
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@SPEC.md`:
- Around line 2781-2789: Update the environment-isolation specification around
the shared operator key and infrastructure API token: do not present host-key
separation as sufficient isolation while these credentials remain shared across
environments. Define separate per-environment credentials, or specify
enforceable least-privilege, break-glass approval, auditing, rotation, and
Terraform state-isolation controls that prevent cross-environment compromise.
- Around line 2791-2803: Update the endpoint-isolation requirements in the
specification to require fail-closed startup validation in addition to the
deploy-time gate. Before any privileged operation, validate the environment’s
chain endpoints, broker base URL, and broker account against an approved
deployment manifest or environment-specific allowlist, and reject mismatches;
retain the requirement that environment-file ENVIRONMENT overrides are rejected.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c668ebac-2634-45fd-9196-f6b4e6a0e6be
📒 Files selected for processing (1)
SPEC.md
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: static
- GitHub Check: test
🧰 Additional context used
📓 Path-based instructions (2)
*
⚙️ CodeRabbit configuration file
Focus on providing constructive criticism. Whenever you see a suboptimal approach, suggest more idiomatic or robust alternative(s). Flag potential footguns. Suggest FP alternatives to mutable/imperative code. Point out architectural flaws like leaky abstractions, tight coupling, wrong level of abstraction, poor type modeling, over-abstraction, unclear domain boundaries. Code should generally be organized based on business concerns rather than technical aspects - suggest improvements if you find violations. Point out gaps in test coverage but suggest tests that are not too coupled to the implementation and actually test domain invariants and business logic
Files:
SPEC.md
**/*.md
⚙️ CodeRabbit configuration file
Focus on the contents of the docs and not on cosmetic things like markdown formatting. We use markdown files for various docs including but not limited to the north star system specification, SPEC.md, the plan for how to get there, ROADMAP.md, guidelines for AI contributors, AGENTS.md, project overview and instructions for human contributors, README.md. Think about the target audience of a document when deciding what comment to leave. For specifications and designs, suggest potential product, architectural, and UI/UX improvements. For plans, suggest changes that would make things more parallelizable and deliverable-focused. For instructions, suggest better rules and guidelines and point out missing instructions. In all cases, flag needless bloat, prefer clear concise writing, and consider the structure of the document and order of the sections
Files:
SPEC.md
🔇 Additional comments (3)
SPEC.md (3)
2708-2709: Bind promotion to the exact smoke-tested revision and define flow coverage.This remains the previously identified issue: staging may follow moving trunk while production uses an independently created tag, and the specification inconsistently requires both “end to end” coverage and only the “flows it touches.” Require the production tag to reference the exact smoke-tested commit, retain its evidence, and explicitly choose all-flow or affected-flow coverage.
Also applies to: 2823-2835
Source: Path instructions
2837-2846: Define telemetry isolation as an enforceable contract.This remains under-specified: name the required field and value, define propagation through instrumentation/exporters, specify that production alert routing rejects staging signals, and add an automated validation proving staging telemetry cannot page production.
Also applies to: 2859-2860
Source: Path instructions
2622-2622: LGTM!Also applies to: 2695-2704, 2711-2723, 2728-2776, 2848-2871
| account. Secrets are encrypted per environment to that environment's service | ||
| role — its host key **plus** the shared operator key. Cross-environment | ||
| isolation is therefore one-directional and specific: staging ciphertext cannot | ||
| be read with production's host key, so compromising the staging host does not | ||
| expose production credentials. The shared operator key is not isolated by | ||
| environment: it decrypts both environments' env files and the Terraform state | ||
| and variables, including the single infrastructure API token that provisions | ||
| both stacks. That key is consequently a production-grade secret regardless of | ||
| which environment it is being used against. |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Do not treat shared operator and infrastructure keys as sufficient environment isolation.
The specification explicitly allows one operator key to decrypt both environments and one infrastructure token to provision both stacks. A compromise of either therefore crosses the environment boundary. Split these credentials, or define enforceable least-privilege, break-glass approval, audit, rotation, and state-isolation controls.
🤖 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 `@SPEC.md` around lines 2781 - 2789, Update the environment-isolation
specification around the shared operator key and infrastructure API token: do
not present host-key separation as sufficient isolation while these credentials
remain shared across environments. Define separate per-environment credentials,
or specify enforceable least-privilege, break-glass approval, auditing,
rotation, and Terraform state-isolation controls that prevent cross-environment
compromise.
Source: Path instructions
| Isolation of credentials does not by itself keep a staging process pointed at | ||
| staging. Chain endpoints and broker identity come from the environment file, and | ||
| nothing cross-checks them against `ENVIRONMENT`, so a staging host configured | ||
| with production endpoints would act on production funds. Vault addresses are not | ||
| env-configured at all — they are registered per asset and live in that | ||
| environment's database — so an environment inherits its vaults from whichever | ||
| database it opens, which makes the database boundary part of the isolation | ||
| boundary rather than an afterthought. Closing the endpoint gap requires the | ||
| deploy-time configuration gate to reject the mismatch: a staging deployment | ||
| configured with a production chain, broker base URL, or broker account must fail | ||
| the deploy, as must the reverse, and an environment file that sets `ENVIRONMENT` | ||
| itself must be rejected so the deploy-time and runtime environment cannot | ||
| diverge. |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Make endpoint validation fail closed at runtime as well as deployment time.
The specification acknowledges that a misconfigured staging process can act on production funds, but only requires a deploy-time gate. Manual edits, secret rotation, rollback, or configuration drift after deployment can bypass that check. Require startup-time validation against an approved deployment manifest or endpoint/account allowlist, and reject mismatches before any privileged operation.
🤖 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 `@SPEC.md` around lines 2791 - 2803, Update the endpoint-isolation requirements
in the specification to require fail-closed startup validation in addition to
the deploy-time gate. Before any privileged operation, validate the
environment’s chain endpoints, broker base URL, and broker account against an
approved deployment manifest or environment-specific allowlist, and reject
mismatches; retain the requirement that environment-file ENVIRONMENT overrides
are rejected.
Source: Path instructions
f149c98 to
8a9a66d
Compare
1 participant
Motivation
Staging already exists and already runs Alpaca sandbox mint and redeem
validation. Its limitation is the network: it runs on production chains, so the
cash-settlement path can only be exercised with production-chain value. That
limit is what creates pressure to validate in production instead, by registering
an asset there that exists only for testing — sharing the production event
store, signer, and Alpaca account with real Authorized Participant traffic.
A test-network staging environment was previously unreachable. While the chain
registry holds a single hardcoded Base entry, the network the broker settles
cash on is reachable from the trading chain only by bridging, and no bridge path
exists between the corresponding test networks. Multichain support removes that
constraint, so the desired end state is worth agreeing before the work is
planned.
Solution
Adds an
## Environmentssection toSPEC.mddescribing the intended end state.Spec only, no behaviour change. Delivery stages will be tracked in Linear rather
than in the spec.
Pairs with the
st0x.liquiditymultichain spec, which owns the settlement-topologyhalf of the same picture.
Checks
By submitting this for review, I'm confirming I've done the following: