Add OTR testnet deployment workflow - #189
Conversation
WalkthroughAdded a testnet deployment stage and lifecycle verification: workflows now support Changes
Sequence DiagramsequenceDiagram
participant Push as Git Push / CI
participant Registry as Container Registry
participant Terraform as Terraform Deployer
participant Testnet as Testnet Environment
participant Healthcheck as Healthcheck Poller
participant LifecycleAPI as Lifecycle API
Push->>Registry: build & push image (digest)
Registry->>Terraform: trigger deploy_otr_testnet (uses image digest)
Terraform->>Testnet: deploy via workspace "convos-otr-testnet"
Testnet-->>Healthcheck: service becomes reachable
Healthcheck->>Testnet: poll ${vars.TESTNET_API_URL}/healthcheck (<=~100s)
Testnet-->>Healthcheck: 200 OK
Healthcheck->>LifecycleAPI: POST /api/v2/assets/test/lifecycle (with token)
LifecycleAPI-->>Healthcheck: 200 + { success: true }
Healthcheck-->>Push: test_lifecycle_testnet completes (pass)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
|
Claude finished @neekolas's task —— View job PR Review Complete ✓Review checklist:
Review FeedbackOverall AssessmentThis PR successfully adds testnet deployment infrastructure following established patterns. The changes are well-structured and consistent with existing dev/prod deployments. I've identified a few areas for improvement below. Code Quality & Best Practices✅ Strengths
|
9d542ea to
797d80e
Compare
3eab43d to
09c00e0
Compare
797d80e to
9d954ee
Compare
09c00e0 to
5550f93
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/s3-lifecycle-verify.yml (1)
81-150: Correct implementation, but consider extracting to a reusable workflow.The
verify-testnetjob correctly mirrors the existingverify-devandverify-prodjobs. The endpoint, authorization, and validation logic are all consistent.However, all three jobs share ~70 lines of identical shell script logic. Consider extracting the verification logic into a reusable workflow or a composite action that accepts the API URL and environment name as inputs. This would reduce maintenance burden when updating the verification logic.
💡 Example reusable workflow structure
Create
.github/workflows/s3-lifecycle-verify-env.yml:name: S3 Lifecycle Verify (Reusable) on: workflow_call: inputs: environment: required: true type: string api_url: required: true type: string secrets: LIFECYCLE_TEST_TOKEN: required: true jobs: verify: name: Verify Lifecycle (${{ inputs.environment }}) runs-on: ubuntu-latest steps: - name: Verify S3 lifecycle canaries run: | # ... shared verification logic using ${{ inputs.api_url }}Then call it from the main workflow:
jobs: verify-dev: uses: ./.github/workflows/s3-lifecycle-verify-env.yml with: environment: Dev api_url: ${{ vars.DEV_API_URL }} secrets: LIFECYCLE_TEST_TOKEN: ${{ secrets.LIFECYCLE_TEST_TOKEN }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/s3-lifecycle-verify.yml around lines 81 - 150, The verify-testnet job duplicates ~70 lines of shared shell logic also present in verify-dev and verify-prod; extract that logic into a reusable workflow (e.g., .github/workflows/s3-lifecycle-verify-env.yml) or a composite action that exposes inputs environment and api_url and expects the LIFECYCLE_TEST_TOKEN secret, move the "Verify S3 lifecycle canaries" script into the reusable workflow (job name verify), and replace the verify-dev/verify-testnet/verify-prod jobs with uses: ./.github/workflows/s3-lifecycle-verify-env.yml + with: environment/api_url and the secrets mapping so all three jobs call the single shared implementation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/s3-lifecycle-verify.yml:
- Around line 81-150: The verify-testnet job duplicates ~70 lines of shared
shell logic also present in verify-dev and verify-prod; extract that logic into
a reusable workflow (e.g., .github/workflows/s3-lifecycle-verify-env.yml) or a
composite action that exposes inputs environment and api_url and expects the
LIFECYCLE_TEST_TOKEN secret, move the "Verify S3 lifecycle canaries" script into
the reusable workflow (job name verify), and replace the
verify-dev/verify-testnet/verify-prod jobs with uses:
./.github/workflows/s3-lifecycle-verify-env.yml + with: environment/api_url and
the secrets mapping so all three jobs call the single shared implementation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e2cca702-a46f-4140-963c-aca1bcc86655
📒 Files selected for processing (3)
.github/workflows/deploy-aws.yml.github/workflows/s3-lifecycle-verify.ymlRELEASE.md
ApprovabilityVerdict: Needs human review This PR adds testnet deployment infrastructure and includes a minor change to auth middleware logging. While the changes are straightforward additions, the author does not own any of the modified files (all owned by @xmtplabs/engineering), and one change touches the sensitive auth middleware directory. The designated code owners should review these changes. You can customize Macroscope's approvability policy. Learn more. |
9d954ee to
fb28045
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/middleware/auth.ts (1)
46-47: Consider applying the same error normalization to the other catch blocks.The normalization here is a nice improvement (avoids dumping non-
Errorthrowables as{}in structured logs). For consistency, thecatchblocks inauthMiddleware(Line 97) andauthMiddlewareAllowNSE(Line 166) still log the raw{ error }object and would benefit from the same treatment.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/middleware/auth.ts` around lines 46 - 47, The catch blocks in authMiddleware and authMiddlewareAllowNSE should normalize the caught value before logging like the AppCheck block does; replace logging of the raw { error } with a normalized message computed as const message = error instanceof Error ? error.message : String(error) and call req.log.error({ error: message }, "...") (use the same log text as the existing catch) so non-Error throwables are logged consistently; update the catch handlers inside the authMiddleware and authMiddlewareAllowNSE functions to use that normalization.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/middleware/auth.ts`:
- Around line 46-47: The catch blocks in authMiddleware and
authMiddlewareAllowNSE should normalize the caught value before logging like the
AppCheck block does; replace logging of the raw { error } with a normalized
message computed as const message = error instanceof Error ? error.message :
String(error) and call req.log.error({ error: message }, "...") (use the same
log text as the existing catch) so non-Error throwables are logged consistently;
update the catch handlers inside the authMiddleware and authMiddlewareAllowNSE
functions to use that normalization.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ebc1f0a8-75c4-4158-8a0a-affef5915d44
📒 Files selected for processing (4)
.github/workflows/deploy-aws.yml.github/workflows/s3-lifecycle-verify.ymlRELEASE.mdsrc/middleware/auth.ts
✅ Files skipped from review due to trivial changes (1)
- RELEASE.md
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/workflows/s3-lifecycle-verify.yml
- .github/workflows/deploy-aws.yml
|
@neekolas with the TF env removed, we can close this one? |
3 participants
Summary
Activates OTR testnet deployment support after the app understands the testnet environment.
otr-testnetto AWS deployment triggers and manual deployment options.deploy_otr_testnetTerraform deployment job using workspaceconvos-otr-testnet.vars.TESTNET_API_URL.otr-dev -> otr-testnet -> otr-prod.Verification
bun checkPsych.load_filefor both modified workflow files.bun test tests/renew-batch.test.ts tests/agent-join.test.ts tests/config-environment.test.tsNote
Add OTR testnet deployment workflow and lifecycle verification
otr-testnetbranch trigger,workflow_dispatchoption, andrepository_dispatchtypedeploy-otr-testnetto deploy-aws.yml, deploying to theconvos-otr-testnetTerraform workspace.test_lifecycle_testnetjob that waits for service health and asserts the lifecycle renewal endpoint returns HTTP 200 with.success == true.verify-testnetjob to s3-lifecycle-verify.yml that checks lifecycle canary status againstTESTNET_API_URLand publishes a workflow summary.otr-devintootr-testnet, verify, then promote tootr-prod.Macroscope summarized fb28045.
Summary by CodeRabbit
New Features
Chores
Documentation
Bug Fixes