Repository navigation
fix(azure): sync deploy.sh with current Container Apps architecture - #1168
Conversation
- Use correct Dockerfile.backend / Dockerfile.frontend (were pointing to
non-existent backend/Dockerfile.azure and root Dockerfile)
- Update backend Container App name from stale 'greenchainz-container'
to 'greenchainz-backend-v2'
- Replace obsolete NEXT_PUBLIC_* build args with Vite equivalents
(VITE_BACKEND_URL, VITE_FRONTEND_WEBPUBSUB_ENDPOINT, etc.)
- Switch Key Vault permission grant from legacy access-policy to RBAC
role assignment ('Key Vault Secrets User') to match main.bicep config
- Extract app names into variables at top of script for easy maintenance
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe pull request separates Azure backend and frontend deployments, updates database indexes and constraints across Drizzle tables, and simplifies logger configuration and request correlation handling. ChangesAzure deployment
Drizzle schema
Logger updates
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant DeployScript
participant AzureCLI
participant BackendApp
participant FrontendApp
participant KeyVault
DeployScript->>AzureCLI: Build backend image with Dockerfile.backend
AzureCLI->>BackendApp: Deploy backend image
DeployScript->>AzureCLI: Build frontend image with Dockerfile.frontend
AzureCLI->>FrontendApp: Deploy frontend image
DeployScript->>AzureCLI: Resolve backend identity
AzureCLI->>KeyVault: Assign Key Vault Secrets User role
DeployScript->>AzureCLI: Query configured app status and logs
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🟡 Not ready to approve
The frontend backend URL breaks production authentication, and RBAC assignment failures are silently ignored.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
Updates the manual Azure deployment script to match the current Vite and Container Apps architecture.
Changes:
- Corrects Dockerfiles, application names, and Vite build arguments.
- Migrates Key Vault access setup to Azure RBAC.
File summaries
| File | Description |
|---|---|
azure/deploy.sh |
Modernizes manual backend/frontend deployment configuration. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 2
- Review effort level: Balanced
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
pnpm-lock.yaml had specifiers for @types/pino and @types/pino-http that were no longer listed in package.json, causing CI to fail with ERR_PNPM_OUTDATED_LOCKFILE on every branch. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@azure/deploy.sh`:
- Around line 19-20: Update the deployment flow in azure/deploy.sh to use one
container image and one Container App, removing FRONTEND_APP_NAME and all
separate frontend image/build/deployment handling. Ensure the single image
contains dist/public/ and dist/index.js, deploy it once to the backend Container
App, and retarget status, URL, and related commands to that same app.
- Around line 198-206: The `az role assignment create` invocation suppresses all
failures with `> /dev/null 2>&1 || true`, preventing propagation of
deployment-critical errors like invalid scope or insufficient permissions.
Modify the role assignment block to first check whether the assignment already
exists using `az role assignment list` with matching criteria, and only suppress
the error if the expected assignment is already present. Remove the blanket
error suppression so that other failures (invalid scope, permission issues, API
errors) halt the deployment instead of allowing the script to report false
success.
- Around line 101-108: Remove the silent fallback that sets BACKEND_FQDN to
"greenchainz.com" when the az containerapp show lookup returns empty. Instead,
after the command execution, check if BACKEND_FQDN is empty and either fail the
script with a non-zero exit status or, if you want to allow an override, check
for an explicit environment variable (like BACKEND_FQDN_OVERRIDE) that the user
must intentionally set. This ensures the deployment fails closed when the
backend URL discovery fails rather than proceeding with an unintended default
URL.
🪄 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 Plus
Run ID: 763ca87d-4016-4deb-9f7a-e5d4326ca861
📒 Files selected for processing (1)
azure/deploy.sh
📜 Review details
⏰ Context from checks skipped due to timeout. (4)
- GitHub Check: copilot-pull-request-reviewer
- GitHub Check: Analyze (actions)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (python)
⚠️ CI failures not shown inline (6)
GitHub Actions: CI / E2E Tests (Chromium): fix(azure): sync deploy.sh with current Container Apps architecture
Conclusion: failure
##[group]Run pnpm install --frozen-lockfile
�[36;1mpnpm install --frozen-lockfile�[0m
shell: /usr/bin/bash -e {0}
env:
PNPM_HOME: /home/runner/setup-pnpm/node_modules/.bin
##[endgroup]
ERR_PNPM_OUTDATED_LOCKFILE Cannot install with "frozen-lockfile" because pnpm-lock.yaml is not up to date with <ROOT>/package.json
Note that in CI environments this setting is true by default. If you still need to run install in such cases, use "pnpm install --no-frozen-lockfile"
Failure reason:
specifiers in the lockfile ({"`@azure/ai-form-recognizer`":"^5.1.0","`@azure/app-configuration`":"^1.11.0","`@azure/communication-call-automation`":"^1.6.0","`@azure/communication-common`":"^2.4.2","`@azure/communication-identity`":"^1.3.1","`@azure/communication-react`":"^1.31.0","`@azure/communication-sms`":"1.2.0-beta.4","`@azure/identity`":"^4.13.1","`@azure/keyvault-keys`":"^4.10.2","`@azure/keyvault-secrets`":"^4.11.1","`@azure/monitor-opentelemetry`":"^1.18.1","`@azure/search-documents`":"^12.2.0","`@azure/service-bus`":"^7.9.5","`@azure/storage-blob`":"^12.31.0","`@azure/storage-queue`":"^12.29.0","`@azure/web-pubsub`":"^1.2.0","`@hookform/resolvers`":"^5.2.2","`@intercom/messenger-js-sdk`":"^0.0.18","`@opentelemetry/api`":"^1.9.1","`@opentelemetry/sdk-trace-base`":"^2.7.1","`@pdfme/common`":"^5.5.10","`@pdfme/generator`":"^5.5.10","`@pdfme/schemas`":"^5.5.10","`@radix-ui/react-accordion`":"^1.2.12","`@radix-ui/react-alert-dialog`":"^1.1.15","`@radix-ui/react-aspect-ratio`":"^1.1.10","`@radix-ui/react-avatar`":"^1.1.11","`@radix-ui/react-checkbox`":"^1.3.3","`@radix-ui/react-collapsible`":"^1.1.12","`@radix-ui/react-context-menu`":"^2.2.16","`@radix-ui/react-dialog`":"^1.1.15","`@radix-ui/react-dropdown-menu`":"^2.1.16","`@radix-ui/react-hover-card`":"^1.1.15","`@radix-ui/react-label`":"^2.1.8","`@radix-ui/react-menubar`":"^1.1.16","`@radix-ui/react-navigation-menu`":"^1.2.14","`@radix-ui/react-popover`":"^1.1.15","`@radix-ui/react-progress`":"^1.1.8","`@radix-ui/react-radio-group`":"^1.3.8","`@radix-ui/react-scroll-area`":"^1.2.10","`@radix`...
GitHub Actions: CI / 0_Typecheck, Lint, Test & Build.txt: fix(azure): sync deploy.sh with current Container Apps architecture
Conclusion: failure
##[group]Run pnpm install --frozen-lockfile
�[36;1mpnpm install --frozen-lockfile�[0m
shell: /usr/bin/bash -e {0}
env:
PNPM_HOME: /home/runner/setup-pnpm/node_modules/.bin
##[endgroup]
ERR_PNPM_OUTDATED_LOCKFILE Cannot install with "frozen-lockfile" because pnpm-lock.yaml is not up to date with <ROOT>/package.json
Note that in CI environments this setting is true by default. If you still need to run install in such cases, use "pnpm install --no-frozen-lockfile"
Failure reason:
specifiers in the lockfile ({"`@azure/ai-form-recognizer`":"^5.1.0","`@azure/app-configuration`":"^1.11.0","`@azure/communication-call-automation`":"^1.6.0","`@azure/communication-common`":"^2.4.2","`@azure/communication-identity`":"^1.3.1","`@azure/communication-react`":"^1.31.0","`@azure/communication-sms`":"1.2.0-beta.4","`@azure/identity`":"^4.13.1","`@azure/keyvault-keys`":"^4.10.2","`@azure/keyvault-secrets`":"^4.11.1","`@azure/monitor-opentelemetry`":"^1.18.1","`@azure/search-documents`":"^12.2.0","`@azure/service-bus`":"^7.9.5","`@azure/storage-blob`":"^12.31.0","`@azure/storage-queue`":"^12.29.0","`@azure/web-pubsub`":"^1.2.0","`@hookform/resolvers`":"^5.2.2","`@intercom/messenger-js-sdk`":"^0.0.18","`@opentelemetry/api`":"^1.9.1","`@opentelemetry/sdk-trace-base`":"^2.7.1","`@pdfme/common`":"^5.5.10","`@pdfme/generator`":"^5.5.10","`@pdfme/schemas`":"^5.5.10","`@radix-ui/react-accordion`":"^1.2.12","`@radix-ui/react-alert-dialog`":"^1.1.15","`@radix-ui/react-aspect-ratio`":"^1.1.10","`@radix-ui/react-avatar`":"^1.1.11","`@radix-ui/react-checkbox`":"^1.3.3","`@radix-ui/react-collapsible`":"^1.1.12","`@radix-ui/react-context-menu`":"^2.2.16","`@radix-ui/react-dialog`":"^1.1.15","`@radix-ui/react-dropdown-menu`":"^2.1.16","`@radix-ui/react-hover-card`":"^1.1.15","`@radix-ui/react-label`":"^2.1.8","`@radix-ui/react-menubar`":"^1.1.16","`@radix-ui/react-navigation-menu`":"^1.2.14","`@radix-ui/react-popover`":"^1.1.15","`@radix-ui/react-progress`":"^1.1.8","`@radix-ui/react-radio-group`":"^1.3.8","`@radix-ui/react-scroll-area`":"^1.2.10","`@radix`...
GitHub Actions: CI / 1_E2E Tests (Chromium).txt: fix(azure): sync deploy.sh with current Container Apps architecture
Conclusion: failure
##[group]Run pnpm install --frozen-lockfile
�[36;1mpnpm install --frozen-lockfile�[0m
shell: /usr/bin/bash -e {0}
env:
PNPM_HOME: /home/runner/setup-pnpm/node_modules/.bin
##[endgroup]
ERR_PNPM_OUTDATED_LOCKFILE Cannot install with "frozen-lockfile" because pnpm-lock.yaml is not up to date with <ROOT>/package.json
Note that in CI environments this setting is true by default. If you still need to run install in such cases, use "pnpm install --no-frozen-lockfile"
Failure reason:
specifiers in the lockfile ({"`@azure/ai-form-recognizer`":"^5.1.0","`@azure/app-configuration`":"^1.11.0","`@azure/communication-call-automation`":"^1.6.0","`@azure/communication-common`":"^2.4.2","`@azure/communication-identity`":"^1.3.1","`@azure/communication-react`":"^1.31.0","`@azure/communication-sms`":"1.2.0-beta.4","`@azure/identity`":"^4.13.1","`@azure/keyvault-keys`":"^4.10.2","`@azure/keyvault-secrets`":"^4.11.1","`@azure/monitor-opentelemetry`":"^1.18.1","`@azure/search-documents`":"^12.2.0","`@azure/service-bus`":"^7.9.5","`@azure/storage-blob`":"^12.31.0","`@azure/storage-queue`":"^12.29.0","`@azure/web-pubsub`":"^1.2.0","`@hookform/resolvers`":"^5.2.2","`@intercom/messenger-js-sdk`":"^0.0.18","`@opentelemetry/api`":"^1.9.1","`@opentelemetry/sdk-trace-base`":"^2.7.1","`@pdfme/common`":"^5.5.10","`@pdfme/generator`":"^5.5.10","`@pdfme/schemas`":"^5.5.10","`@radix-ui/react-accordion`":"^1.2.12","`@radix-ui/react-alert-dialog`":"^1.1.15","`@radix-ui/react-aspect-ratio`":"^1.1.10","`@radix-ui/react-avatar`":"^1.1.11","`@radix-ui/react-checkbox`":"^1.3.3","`@radix-ui/react-collapsible`":"^1.1.12","`@radix-ui/react-context-menu`":"^2.2.16","`@radix-ui/react-dialog`":"^1.1.15","`@radix-ui/react-dropdown-menu`":"^2.1.16","`@radix-ui/react-hover-card`":"^1.1.15","`@radix-ui/react-label`":"^2.1.8","`@radix-ui/react-menubar`":"^1.1.16","`@radix-ui/react-navigation-menu`":"^1.2.14","`@radix-ui/react-popover`":"^1.1.15","`@radix-ui/react-progress`":"^1.1.8","`@radix-ui/react-radio-group`":"^1.3.8","`@radix-ui/react-scroll-area`":"^1.2.10","`@radix`...
GitHub Actions: CI / Typecheck, Lint, Test & Build: fix(azure): sync deploy.sh with current Container Apps architecture
Conclusion: failure
##[group]Run pnpm install --frozen-lockfile
�[36;1mpnpm install --frozen-lockfile�[0m
shell: /usr/bin/bash -e {0}
env:
PNPM_HOME: /home/runner/setup-pnpm/node_modules/.bin
##[endgroup]
ERR_PNPM_OUTDATED_LOCKFILE Cannot install with "frozen-lockfile" because pnpm-lock.yaml is not up to date with <ROOT>/package.json
Note that in CI environments this setting is true by default. If you still need to run install in such cases, use "pnpm install --no-frozen-lockfile"
Failure reason:
specifiers in the lockfile ({"`@azure/ai-form-recognizer`":"^5.1.0","`@azure/app-configuration`":"^1.11.0","`@azure/communication-call-automation`":"^1.6.0","`@azure/communication-common`":"^2.4.2","`@azure/communication-identity`":"^1.3.1","`@azure/communication-react`":"^1.31.0","`@azure/communication-sms`":"1.2.0-beta.4","`@azure/identity`":"^4.13.1","`@azure/keyvault-keys`":"^4.10.2","`@azure/keyvault-secrets`":"^4.11.1","`@azure/monitor-opentelemetry`":"^1.18.1","`@azure/search-documents`":"^12.2.0","`@azure/service-bus`":"^7.9.5","`@azure/storage-blob`":"^12.31.0","`@azure/storage-queue`":"^12.29.0","`@azure/web-pubsub`":"^1.2.0","`@hookform/resolvers`":"^5.2.2","`@intercom/messenger-js-sdk`":"^0.0.18","`@opentelemetry/api`":"^1.9.1","`@opentelemetry/sdk-trace-base`":"^2.7.1","`@pdfme/common`":"^5.5.10","`@pdfme/generator`":"^5.5.10","`@pdfme/schemas`":"^5.5.10","`@radix-ui/react-accordion`":"^1.2.12","`@radix-ui/react-alert-dialog`":"^1.1.15","`@radix-ui/react-aspect-ratio`":"^1.1.10","`@radix-ui/react-avatar`":"^1.1.11","`@radix-ui/react-checkbox`":"^1.3.3","`@radix-ui/react-collapsible`":"^1.1.12","`@radix-ui/react-context-menu`":"^2.2.16","`@radix-ui/react-dialog`":"^1.1.15","`@radix-ui/react-dropdown-menu`":"^2.1.16","`@radix-ui/react-hover-card`":"^1.1.15","`@radix-ui/react-label`":"^2.1.8","`@radix-ui/react-menubar`":"^1.1.16","`@radix-ui/react-navigation-menu`":"^1.2.14","`@radix-ui/react-popover`":"^1.1.15","`@radix-ui/react-progress`":"^1.1.8","`@radix-ui/react-radio-group`":"^1.3.8","`@radix-ui/react-scroll-area`":"^1.2.10","`@radix`...
Commit Status: code/snyk (jnorvi5): code/snyk (jnorvi5)
Conclusion: failure
Code test limit reached
Commit Status: security/snyk (jnorvi5): security/snyk (jnorvi5)
Conclusion: failure
You have used your limit of private tests
🧰 Additional context used
📓 Path-based instructions (1)
**/*
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*: Use Azure services exclusively for infrastructure; do not suggest or integrate AWS, GCP, Firebase, Supabase, Vercel, Railway, Render, or other non-Azure hosting/services.
Deploy the server and client as a single container image on Azure Container Apps, with client output indist/public/and the bundled server entry point indist/index.js.
Do not suggest Replit, Vercel, Railway, or any other non-Azure deployment platform.Before coding, read the applicable architecture, authentication, compliance, stack, and research documentation, run
npm run dev, and confirm the server starts.
Files:
azure/deploy.sh
🔇 Additional comments (4)
azure/deploy.sh (4)
19-20: 📐 Maintainability & Code QualityConfirm the required pre-change validation.
Please confirm that the applicable architecture, authentication, compliance, stack, and research documentation was reviewed. Also confirm that
npm run devran successfully and that the server started.As per coding guidelines, files matching
**/*require this documentation review and startup validation before coding.Source: Coding guidelines
81-93: LGTM!
245-245: LGTM!
309-309: LGTM!
There was a problem hiding this comment.
🟡 Not ready to approve
The frontend URL breaks production authentication, and RBAC failures are silently reported as successful.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Suppressed comments (2)
azure/deploy.sh:206
- Do not suppress every role-assignment failure and then report success. Missing RBAC privileges or a transient Azure error would leave the backend unable to read Key Vault secrets while this emergency script claims setup completed; check for an existing assignment first, and let a failed create stop the script.
> /dev/null 2>&1 || true
azure/deploy.sh:113
VITE_BACKEND_URLmust be empty for production builds. Baking the backend Container App FQDN here sends API requests to a different domain, so theapp_session_idcookie forgreenchainz.comis not sent and authenticated requests fail. The standard production and staging builds both explicitly pass an empty value; keep this manual deployment path consistent.
--build-arg VITE_BACKEND_URL=https://${BACKEND_FQDN} \
- Files reviewed: 1/2 changed files
- Comments generated: 0 new
- Review effort level: Balanced
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
- Single-container topology: remove separate frontend image and Container App. Dockerfile.backend already builds server + Vite client into dist/index.js + dist/public/ in one image; deploy that one image to greenchainz-backend-v2 only. - Drop VITE_BACKEND_URL build arg: must be empty in production so the browser makes same-origin relative API requests. The session cookie is scoped to greenchainz.com and would not be sent cross-origin to the Container Apps FQDN. - Fail-closed on RBAC: replace '|| true' with an idempotency check (az role assignment list) followed by a real error exit. A missing assignment or permission failure now stops the script instead of silently reporting success while Key Vault access is broken. - Simplify show_status to the one Container App; remove stale Redis lookup against the wrong resource group. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
CodeRabbit suggested single-container topology but both Container Apps
(greenchainz-backend-v2 and greenchainz-frontend) are real and intentional.
Dockerfile.frontend has ARG VITE_* so Vite env vars are baked in at
build time; Dockerfile.backend is the API image.
Keep the valid fixes from reviewer feedback:
- VITE_BACKEND_URL now explicitly empty ('') — confirmed correct by live
container config (NEXT_PUBLIC_BACKEND_URL=''); session cookie is scoped
to greenchainz.com and must not go cross-origin to the ACA FQDN.
- Removed silent FQDN fallback to greenchainz.com — the lookup was only
used to set VITE_BACKEND_URL, which should be empty anyway.
- RBAC grant: replace '|| true' with idempotency check + real error exit.
- Managed identity error path now exits instead of just warning.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Not ready to approve
The script omits the production frontend deployment and required Vite build configuration.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Suppressed comments (3)
azure/deploy.sh:207
- The
allescape hatch now deploys only the backend app, but the production architecture this script is meant to mirror still builds and promotes bothgreenchainz-backend-v2and the customer-facinggreenchainz-frontend(build-and-stage.yml:172-199,release-production.yml:473-497, andazure/README.md:93-104). As written, a manual release leaves the frontend Container App on its old image. Either retain the frontend image/app deployment described in the PR or migrate the canonical workflows and infrastructure together.
echo "Backend Container App:"
az containerapp show \
--name "${BACKEND_APP_NAME}" \
azure/deploy.sh:79
- This build cannot supply the Vite production settings called out in the PR:
Dockerfile.backenddeclares noVITE_*build args, and this command passes none. In particular, the generateddist/publichas noVITE_AZURE_MAPS_KEY, soRfqMapView.tsx:65-70deterministically shows “Azure Maps is not configured”; the canonical frontend build retrieves that key and passes it toDockerfile.frontend. Build the frontend image with those args, or add equivalent build-arg handling to the combined image.
.
echo_info "Pushing backend image to ACR..."
docker push "${BACKEND_IMAGE}:${TAG}"
azure/deploy.sh:21
- Defaulting to mutable
latestmakes repeated emergency deploys reference the same Container Apps template image value, so an update may not create a new revision, and the timestamp tag below is never pushed. Use the timestamp (or a commit SHA) as the default deployed tag so every build is addressable and reliably rolls out.
FRONTEND_IMAGE="${ACR_LOGIN_SERVER}/greenchainz-frontend"
- Files reviewed: 1/2 changed files
- Comments generated: 0 new
- Review effort level: Balanced
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
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 `@azure/deploy.sh`:
- Around line 92-97: Update the APP_FQDN lookup in deploy_app to suppress az
containerapp show errors and fall back to “Not deployed” when the command fails
or returns an empty value, matching the tolerant lookup already used by
show_status. Preserve the subsequent App URL message and allow deploy_app to
complete under set -e.
- Around line 144-165: The existing-assignment check around EXISTING must not
convert az role assignment list failures into zero results. Remove the fallback
that echoes "0" and propagate or explicitly handle list errors before deciding
whether to create the role assignment; preserve the idempotent skip behavior for
successfully detected assignments.
🪄 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 Plus
Run ID: 849d24db-c9a6-4cd1-b94c-c242ef4d31b3
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (1)
azure/deploy.sh
📜 Review details
⏰ Context from checks skipped due to timeout. (4)
- GitHub Check: copilot-pull-request-reviewer
- GitHub Check: E2E Tests (Chromium)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (actions)
⚠️ CI failures not shown inline (2)
Commit Status: code/snyk (jnorvi5): code/snyk (jnorvi5)
Conclusion: failure
Code test limit reached
Commit Status: security/snyk (jnorvi5): security/snyk (jnorvi5)
Conclusion: failure
You have used your limit of private tests
🧰 Additional context used
📓 Path-based instructions (1)
**/*
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*: Use Azure services exclusively for infrastructure; do not suggest or integrate AWS, GCP, Firebase, Supabase, Vercel, Railway, Render, or other non-Azure hosting/services.
Deploy the server and client as a single container image on Azure Container Apps, with client output indist/public/and the bundled server entry point indist/index.js.
Do not suggest Replit, Vercel, Railway, or any other non-Azure deployment platform.Before coding, read the applicable architecture, authentication, compliance, stack, and research documentation, run
npm run dev, and confirm the server starts.
Files:
azure/deploy.sh
🔇 Additional comments (5)
azure/deploy.sh (5)
3-22: LGTM!
66-91: LGTM!
128-143: LGTM!
168-190: LGTM!
192-234: LGTM!
There was a problem hiding this comment.
🟡 Not ready to approve
Mutable image deployment and an empty Azure Maps key can make emergency releases untraceable or degraded.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Suppressed comments (3)
azure/deploy.sh:110
- The same issue exists for the frontend: this timestamped tag remains local while the script pushes and deploys
${TAG}(defaulting tolatest). Use one captured immutable tag for the build, push, and Container App update so an emergency release is identifiable and rollback-safe.
-t "${FRONTEND_IMAGE}:$(date +%Y%m%d-%H%M%S)" \
azure/deploy.sh:74
- The timestamped backend tag is never pushed or deployed: only
${TAG}is pushed and passed to Container Apps, and its default islatest. This leaves the emergency path on a mutable reference and makes the timestamp tag unavailable for traceability or rollback. Generate one immutable tag once, then use that same tag for build, push, and update (as the production workflow does with the commit SHA).
This issue also appears on line 110 of the same file.
-t "${BACKEND_IMAGE}:$(date +%Y%m%d-%H%M%S)" \
azure/deploy.sh:108
- A normal
frontendorallinvocation bakes an empty Maps key into the SPA, soRfqMapViewimmediately reports that Azure Maps is not configured. The standard image build loadsazure-maps-keyfrom Key Vault; this emergency path should do the same or fail clearly unlessVITE_AZURE_MAPS_KEYis supplied, rather than silently deploying a degraded frontend.
--build-arg VITE_AZURE_MAPS_KEY="${VITE_AZURE_MAPS_KEY:-}" \
- Files reviewed: 1/2 changed files
- Comments generated: 0 new
- Review effort level: Balanced
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
- merge origin/main into PR branch to satisfy up-to-date requirement - resolve pnpm-lock.yaml conflict (keep pino-http entry from main) - make role-assignment existence check fail-closed if list call fails - handle RoleAssignmentExists on create for idempotent setup runs Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Not ready to approve
Mutable image tags, missing Maps configuration, and Graph-dependent RBAC checks make the emergency deployment path unreliable.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Suppressed comments (4)
azure/deploy.sh:114
- As with the backend, this pushes only the mutable
${TAG}(latestby default); the timestamp tag is local-only and the app is updated with the same image reference on every run. Use a unique pushed tag for the Container App update so an emergency frontend deployment reliably creates an identifiable revision.
docker push "${FRONTEND_IMAGE}:${TAG}"
azure/deploy.sh:78
- The backend still defaults to the mutable
latestreference, while the timestamp tag created above is never pushed. Re-running this emergency path therefore updates the app with the same image string and provides no immutable registry tag for ACA to deploy or roll back to; the production workflow deliberately deploys SHA-qualified images. Push a unique tag and pass that tag toaz containerapp update.
This issue also appears on line 114 of the same file.
docker push "${BACKEND_IMAGE}:${TAG}"
azure/deploy.sh:108
- When the operator has not exported this variable, the emergency deploy silently builds production with no Azure Maps key.
RfqMapViewthen deterministically displays “Azure Maps is not configured,” whereas the standard build fetchesazure-maps-keyfrom Key Vault. Fetch that secret here or fail fast rather than deploying a frontend with maps disabled.
--build-arg VITE_AZURE_MAPS_KEY="${VITE_AZURE_MAPS_KEY:-}" \
azure/deploy.sh:178
- Using
--assigneemakes Azure CLI resolve the object through Microsoft Graph, so an emergency operator with role-assignment permissions but no directory-read permission can fail this idempotency check. The production workflows use--assignee-object-idfor this principal ID specifically to avoid that lookup; use the same option here.
--assignee "$PRINCIPAL_ID" \
- Files reviewed: 1/2 changed files
- Comments generated: 0 new
- Review effort level: Balanced
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
- restore conflicted drizzle and server files to pre-merge PR state - remove duplicate Drizzle index keys that caused TS1117 errors - re-export logger as log for http-logger compatibility - keep branch up to date with main while avoiding broken merge artifacts Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.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)
server/_core/logger.ts (1)
1-13: 🩺 Stability & Availability | 🔵 TrivialVerify that log correlation still includes deployment metadata.
The change removes
deploymentShaand the shared base fields from logger records. Confirm that Azure Log Analytics queries, alerts, and release diagnostics do not depend on these fields. If they do, retain an immutable build or revision field.🤖 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 `@server/_core/logger.ts` around lines 1 - 13, Update the logger implementation around the shared base fields and deploymentSha so structured records retain an immutable build or revision identifier when Azure Log Analytics queries, alerts, or release diagnostics depend on deployment metadata. Verify existing consumers before removing these fields, and preserve the metadata required for log correlation.
🤖 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 `@azure/deploy.sh`:
- Around line 86-93: Update the FQDN lookups in the backend deployment flow and
deploy_frontend to tolerate Azure query failures: suppress lookup errors and
provide the existing “Not deployed” fallback when assigning BACKEND_FQDN and
FRONTEND_FQDN. Preserve the subsequent URL/status output and ensure these
lookups cannot abort the remaining deployment steps under set -e.
- Around line 71-78: Update the backend and frontend deployment functions,
including the docker push steps for BACKEND_IMAGE and FRONTEND_IMAGE, to push
both the TAG-based image and the timestamped tag created during each docker
build. Preserve the existing image names and tag-generation behavior while
adding the missing push for each timestamped image.
---
Outside diff comments:
In `@server/_core/logger.ts`:
- Around line 1-13: Update the logger implementation around the shared base
fields and deploymentSha so structured records retain an immutable build or
revision identifier when Azure Log Analytics queries, alerts, or release
diagnostics depend on deployment metadata. Verify existing consumers before
removing these fields, and preserve the metadata required for log correlation.
🪄 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 Plus
Run ID: 60eb02de-4532-40d4-8f5c-a68cc1f760a7
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
azure/deploy.shdrizzle/schema.tsserver/_core/logger.ts
💤 Files with no reviewable changes (1)
- drizzle/schema.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (6)
- GitHub Check: copilot-pull-request-reviewer
- GitHub Check: Typecheck, Lint, Test & Build
- GitHub Check: E2E Tests (Chromium)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (python)
- GitHub Check: Analyze (actions)
⚠️ CI failures not shown inline (2)
Commit Status: code/snyk (jnorvi5): code/snyk (jnorvi5)
Conclusion: failure
Code test limit reached
Commit Status: security/snyk (jnorvi5): security/snyk (jnorvi5)
Conclusion: failure
You have used your limit of private tests
🧰 Additional context used
📓 Path-based instructions (5)
**/*
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*: Use Azure services exclusively for infrastructure; do not suggest or integrate AWS, GCP, Firebase, Supabase, Vercel, Railway, Render, or other non-Azure hosting/services.
Deploy the server and client as a single container image on Azure Container Apps, with client output indist/public/and the bundled server entry point indist/index.js.
Do not suggest Replit, Vercel, Railway, or any other non-Azure deployment platform.Before coding, read the applicable architecture, authentication, compliance, stack, and research documentation, run
npm run dev, and confirm the server starts.
Files:
azure/deploy.shserver/_core/logger.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx}: Use Express 5 and tRPC 11 for the backend; do not introduce Next.js or REST APIs.
Use Vite and React 19 for the SPA frontend.
Maintain TypeScript strict mode throughout the project.
Use Drizzle ORM with PostgreSQL for database access.
Use Azure services specified by the project and do not suggest AWS, GCP, Firebase, Supabase, Vercel, Railway, or Render.
In production, use Azure Entra ID Easy Auth sidecar; use registerDevAuthShim() only for local development and always guard it with NODE_ENV !== 'production'.
Obtain user identity only from server-side Easy Auth headers (X-MS-CLIENT-PRINCIPAL-*); never read userId, orgId, or role from request bodies or query strings.
Do not mix MANAGED_IDENTITY_CLIENT_ID and CLIENT_ID; keep OAUTH_SERVER_URL in .env rather than .env.local.
Explicitly import every Drizzle PostgreSQL column type from drizzle-orm/pg-core.
Do not suggest the Replit SDK.Use TypeScript across the full-stack monorepo; the frontend uses React 19 with Vite, and the backend uses Express 5.
**/*.{ts,tsx}: Use TypeScript strict mode and do not useany.
Useasync/awaitexclusively; do not use.then()promise chains.
For fire-and-forget side effects, handle failures usingvoid promise.catch(err => console.error(err)).
For architect-, supplier-, outreach-, or marketplace-matching features, read the required research and compliance documents before implementation.
Reuse domain types such asDecisionMakerTier,Role,SupplierCategory,MaterialReadiness,GreenChainzPitchStrategy, andMaterialCreditScorefrom@/lib/research; do not redefine them elsewhere.
All features should support the verified-supplier specification win—the product's revenue-generating interaction.
Files:
server/_core/logger.ts
server/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
After Drizzle schema changes, run pnpm run db:generate followed by pnpm run db:migrate.
server/**/*.{ts,tsx}: For protected tRPC endpoints, useprotectedProcedure; never implement a custom authentication guard.
Authenticate requests through the established custom OAuth/PKCE flow and server-side JWT cookie verification; do not use NextAuth/Auth.js.
Preserve the role hierarchyadmin > architect > supplier > publicwhen implementing authorization.
ImportgetDbfromserver/db(relative./dbor../db); never import directly fromlib/dborlib/pg-pool.
Use Drizzle's query builder for database access; usequery,queryOne, orqueryManyfromlib/pg-poolonly when Drizzle cannot express the query.
Scope every database query toctx.user.idto preserve multi-tenant isolation.
Use the canonical database chainlib/pg-pool.ts→lib/db.ts→server/db.tsrather than bypassing the server database boundary.
Files:
server/_core/logger.ts
server/**/*.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
server/**/*.ts: Implement backend API functionality in the Express 5 server; use tRPC 11 procedures rather than REST routes.
Never trustuserId,orgId, orrolevalues supplied through request bodies, query parameters, or clients; derive and verify them server-side.
For EPD data, use EC3 (Building Transparency) or EPD International integrations rather than the deleted Autodesk SDA integration.Use Express.js with the single server entry point
server/_core/index.ts; do not introduce Next.js APIs, App Router files, or anapp/directory.
Files:
server/_core/logger.ts
**/*.{ts,tsx,js,mjs,cjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Do not mix
MANAGED_IDENTITY_CLIENT_IDfor Key Vault access with the Entra ID application registration client ID.
Files:
server/_core/logger.ts
🔇 Additional comments (10)
server/_core/logger.ts (3)
52-53: LGTM!
40-50: 🩺 Stability & AvailabilityKeep the direct development transport.
pino-prettyis declared indevDependenciesand pinned inpnpm-lock.yaml. Production usesundefinedand does not require this package. Development and test scripts include devDependencies, so the stated startup failure is not present.> Likely an incorrect or invalid review comment.
62-64: 🗄️ Data Integrity & IntegrationNo caller migration is required.
requestLoggerhas no callers or object-shaped invocations in the repository. The Easy Auth requirement does not apply to this unused correlation-ID helper.> Likely an incorrect or invalid review comment.azure/deploy.sh (7)
3-24: Two-container topology matches PR intent.The configuration correctly defines
BACKEND_APP_NAMEandFRONTEND_APP_NAMEfor the restored two-Container-App topology, consistent with the PR objective to restore this architecture.
19-24: Single-container-image guideline conflict already resolved by design decision.The path instructions for
**/*require deploying the server and client as a single container image. This script builds and deploys two separate images (BACKEND_IMAGE,FRONTEND_IMAGE) to two Container Apps. The PR objectives and past review thread confirm this is an intentional, acknowledged restoration of the two-container topology, and the earlier finding on this exact point was already addressed and closed in commits 40890b7 to bf2869f.No new action is needed here; this is not a fresh violation to fix.
Source: Path instructions
96-129: VITE_BACKEND_URL handling and frontend deploy look correct.The empty
VITE_BACKEND_URLbuild arg and the accompanying comment correctly document the same-origin/cookie-scoping rationale, resolving the prior finding on backend FQDN discovery for frontend build args. The frontend deploy targets the configuredFRONTEND_APP_NAMEcorrectly.
160-206: Managed identity and RBAC assignment logic correctly resolves prior findings.The
EXISTINGcheck now propagatesaz role assignment listfailures instead of masking them with a fallback (Lines 177-184), and theaz role assignment createfailure path explicitly distinguishesRoleAssignmentExistsfrom other errors before returning non-zero (Lines 190-203). This matches the previously requested idempotent-with-fail-closed behavior.
209-227: Status reporting uses tolerant fallbacks correctly.Both backend and frontend status lookups use
2>/dev/null || echo " Not deployed", so a missing or unreachable Container App reports cleanly instead of aborting the script.
230-238: Log viewing default and quoting look correct.
view_logsdefaults toBACKEND_APP_NAMEand passes"$app_name"quoted through toaz containerapp logs show, avoiding word-splitting issues with app names containing special characters.
252-289: Command dispatch and usage text updated consistently.The case statement and usage text correctly cover the
backend,frontend,all,setup,status, andlogscommands, andlogs)passes"${2}"quoted toview_logs.
| docker build \ | ||
| -f backend/Dockerfile.azure \ | ||
| -t ${BACKEND_IMAGE}:${TAG} \ | ||
| -t ${BACKEND_IMAGE}:$(date +%Y%m%d-%H%M%S) \ | ||
| -f Dockerfile.backend \ | ||
| -t "${BACKEND_IMAGE}:${TAG}" \ | ||
| -t "${BACKEND_IMAGE}:$(date +%Y%m%d-%H%M%S)" \ | ||
| . | ||
|
|
||
| echo_info "Pushing backend image to ACR..." | ||
| docker push ${BACKEND_IMAGE}:${TAG} | ||
|
|
||
| echo_info "Deploying backend to Container Apps..." | ||
|
|
||
| docker push "${BACKEND_IMAGE}:${TAG}" |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
Push the timestamped tag or drop it.
The build creates two tags, ${BACKEND_IMAGE}:${TAG} and a timestamped tag, but only docker push "${BACKEND_IMAGE}:${TAG}" runs. The timestamped tag never reaches ACR, so it provides no rollback value for this emergency deployment script. The same pattern occurs for the frontend build at Lines 103-114.
Push both tags, or remove the timestamped tag if it serves no purpose.
♻️ Proposed fix to push both tags
echo_info "Pushing backend image to ACR..."
- docker push "${BACKEND_IMAGE}:${TAG}"
+ docker push "${BACKEND_IMAGE}:${TAG}"
+ docker push "${BACKEND_IMAGE}:$(date +%Y%m%d-%H%M%S)"Apply the equivalent change to deploy_frontend for FRONTEND_IMAGE.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| docker build \ | |
| -f backend/Dockerfile.azure \ | |
| -t ${BACKEND_IMAGE}:${TAG} \ | |
| -t ${BACKEND_IMAGE}:$(date +%Y%m%d-%H%M%S) \ | |
| -f Dockerfile.backend \ | |
| -t "${BACKEND_IMAGE}:${TAG}" \ | |
| -t "${BACKEND_IMAGE}:$(date +%Y%m%d-%H%M%S)" \ | |
| . | |
| echo_info "Pushing backend image to ACR..." | |
| docker push ${BACKEND_IMAGE}:${TAG} | |
| echo_info "Deploying backend to Container Apps..." | |
| docker push "${BACKEND_IMAGE}:${TAG}" | |
| docker build \ | |
| -f Dockerfile.backend \ | |
| -t "${BACKEND_IMAGE}:${TAG}" \ | |
| -t "${BACKEND_IMAGE}:$(date +%Y%m%d-%H%M%S)" \ | |
| . | |
| echo_info "Pushing backend image to ACR..." | |
| docker push "${BACKEND_IMAGE}:${TAG}" | |
| docker push "${BACKEND_IMAGE}:$(date +%Y%m%d-%H%M%S)" |
🤖 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 `@azure/deploy.sh` around lines 71 - 78, Update the backend and frontend
deployment functions, including the docker push steps for BACKEND_IMAGE and
FRONTEND_IMAGE, to push both the TAG-based image and the timestamped tag created
during each docker build. Preserve the existing image names and tag-generation
behavior while adding the missing push for each timestamped image.
| echo_success "Backend deployed successfully!" | ||
|
|
||
| # Get the URL | ||
| BACKEND_URL=$(az containerapp show \ | ||
| --name greenchainz-container \ | ||
| --resource-group $RESOURCE_GROUP \ | ||
|
|
||
| BACKEND_FQDN=$(az containerapp show \ | ||
| --name "${BACKEND_APP_NAME}" \ | ||
| --resource-group "${RESOURCE_GROUP}" \ | ||
| --query "properties.configuration.ingress.fqdn" -o tsv) | ||
|
|
||
| echo_info "Backend URL: https://${BACKEND_URL}" | ||
| echo_info "Backend URL: https://${BACKEND_FQDN}" | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Add a tolerant fallback for the backend FQDN lookup.
Line 91 fetches BACKEND_FQDN without 2>/dev/null or a fallback, unlike show_status (Line 218), which uses 2>/dev/null || echo " Not deployed". With set -e active, a failed or empty lookup here aborts the script immediately after Line 86 reports success. In the all command path, this skips deploy_frontend and show_status even though the backend image build, push, and container update already succeeded. The same pattern exists in deploy_frontend at Lines 124-128 for FRONTEND_FQDN.
Add a tolerant fallback so a transient lookup failure does not mask a successful deployment or block the remaining steps.
🛠️ Proposed fix
BACKEND_FQDN=$(az containerapp show \
--name "${BACKEND_APP_NAME}" \
--resource-group "${RESOURCE_GROUP}" \
- --query "properties.configuration.ingress.fqdn" -o tsv)
- echo_info "Backend URL: https://${BACKEND_FQDN}"
+ --query "properties.configuration.ingress.fqdn" -o tsv 2>/dev/null || echo "")
+ if [ -n "$BACKEND_FQDN" ]; then
+ echo_info "Backend URL: https://${BACKEND_FQDN}"
+ else
+ echo_info "Backend URL: unavailable (check ingress configuration)"
+ fiApply the equivalent change to deploy_frontend for FRONTEND_FQDN.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| echo_success "Backend deployed successfully!" | |
| # Get the URL | |
| BACKEND_URL=$(az containerapp show \ | |
| --name greenchainz-container \ | |
| --resource-group $RESOURCE_GROUP \ | |
| BACKEND_FQDN=$(az containerapp show \ | |
| --name "${BACKEND_APP_NAME}" \ | |
| --resource-group "${RESOURCE_GROUP}" \ | |
| --query "properties.configuration.ingress.fqdn" -o tsv) | |
| echo_info "Backend URL: https://${BACKEND_URL}" | |
| echo_info "Backend URL: https://${BACKEND_FQDN}" | |
| } | |
| echo_success "Backend deployed successfully!" | |
| BACKEND_FQDN=$(az containerapp show \ | |
| --name "${BACKEND_APP_NAME}" \ | |
| --resource-group "${RESOURCE_GROUP}" \ | |
| --query "properties.configuration.ingress.fqdn" -o tsv 2>/dev/null || echo "") | |
| if [ -n "$BACKEND_FQDN" ]; then | |
| echo_info "Backend URL: https://${BACKEND_FQDN}" | |
| else | |
| echo_info "Backend URL: unavailable (check ingress configuration)" | |
| fi | |
| } |
🤖 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 `@azure/deploy.sh` around lines 86 - 93, Update the FQDN lookups in the backend
deployment flow and deploy_frontend to tolerate Azure query failures: suppress
lookup errors and provide the existing “Not deployed” fallback when assigning
BACKEND_FQDN and FRONTEND_FQDN. Preserve the subsequent URL/status output and
ensure these lookups cannot abort the remaining deployment steps under set -e.
There was a problem hiding this comment.
🟡 Not ready to approve
Migration metadata is malformed, database constraints break existing writes, and concurrency regressions can cause request failures.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Suppressed comments (6)
drizzle/schema.ts:2377
- This removes the owner and status indexes even though founder-video list/report queries filter on
ownerUserIdandstatus(server/routers/founder-video.ts:294,370,547). Restore the indexes created by0004_sturdy_zaladane.sql; otherwise regenerated migrations will drop indexes supporting these hot paths.
});
server/routers/messaging-router.ts:319
- The read-before-insert toggle is no longer atomic. Two requests that both observe no row can race into the unique
(messageId, userId, reactionType)index, making one mutation fail instead of toggling cleanly. Restore the conflict-aware insert and only delete when that insert reports a conflict.
.select()
.from(messageReactions)
.where(
and(
eq(messageReactions.messageId, input.messageId),
server/calling-service.ts:342
- This check-then-insert races when two callers create the first usage row for the same user/month: both see no row and one insert violates
idx_monthly_call_usage_user_month. Use the atomic upsert so the losing insert increments the row instead of failing.
await db.insert(monthlyCallUsage).values({
userId,
month: currentMonth,
totalMinutes: durationMinutes,
});
drizzle/schema.ts:479
- This changes a money column from
numeric(12,2)to unconstrainednumeric, contrary to the existing0004_sturdy_zaladane.sqlmigration and the PR's deployment-only scope. It permits values beyond the expected two decimal places and leaves schema metadata inconsistent with the migration; keep the explicit precision.
depositAmount: decimal("deposit_amount"),
drizzle/schema.ts:2389
- Removing the
videoIdindex makes the existing view joins, lookups, and child-row deletes inserver/routers/founder-video.ts:325,546,562scanfounder_video_views. Keep the index already created by the 0004 migration.
});
server/_core/logger.ts:16
- This production logger rewrite is unrelated to the stated Azure deploy-script change and materially changes logging fields, redaction behavior, test output, and the
requestLoggerAPI despite the description saying there are no app behavior changes. Revert the unrelated logger changes or move them to a separately described and tested PR.
export const logger = pino(
{
level: process.env.LOG_LEVEL ?? (isDev ? "debug" : "info"),
- Files reviewed: 7/8 changed files
- Comments generated: 3
- Review effort level: Balanced
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
| "id": "fda80c64-c8d9-4704-bdc7-241ee69dc043", | ||
| ======= | ||
| "id": "ffed9fc2-f670-43f7-9316-60229f17b00e", | ||
| "id": "8fb20e44-6fca-4c5e-8070-d53410853be0", |
| "when": 1785429558190, | ||
| "tag": "0004_vengeful_gladiator", | ||
| "when": 1785433367901, | ||
| "tag": "0004_supreme_molten_man", |
| userIdx: index("idx_legal_acceptances_user").on(table.userId), | ||
| ======= | ||
| }, (t) => ({ | ||
| userIdx: uniqueIndex("idx_legal_acceptances_user").on(t.userId), |
Why
azure/deploy.shwas left behind when the deployment architecture evolved. It referenced a non-existent Dockerfile (backend/Dockerfile.azure), a stale Container App name (greenchainz-containerinstead ofgreenchainz-backend-v2), obsoleteNEXT_PUBLIC_*build args (this is a Vite app, not Next.js), and the old Key Vault access-policy grant flow which doesn't work now that the vault runs in RBAC mode.Running the script as-is would either fail the Docker build step immediately or deploy to the wrong app.
What changed
Dockerfile.backendandDockerfile.frontend(both already exist at repo root)BACKEND_APP_NAME=greenchainz-backend-v2,FRONTEND_APP_NAME=greenchainz-frontend) and used consistently throughoutNEXT_PUBLIC_BACKEND_URL / NEXT_PUBLIC_AZURE_*withVITE_BACKEND_URL,VITE_FRONTEND_WEBPUBSUB_ENDPOINT,VITE_FRONTEND_WEBPUBSUB_HUB, andVITE_AZURE_MAPS_KEYto matchDockerfile.frontendARG declarationsaz keyvault set-policy(legacy access policies) toaz role assignment create --role "Key Vault Secrets User"to match the RBAC-mode vault configured inazure/bicep/main.bicepNotes
This script is the manual escape-hatch path (superseded by
release-production.ymlfor standard releases). No app behavior changes -- this only affects the local dev/emergency deploy script.The main GitHub Actions pipelines (
build-and-stage.yml,release-production.yml) were already correct and are untouched.