Repository navigation
fix(observability): bind release identity and verify live Sentry deployments - #1610
Conversation
Code Review Completed! 🔥The code review was successfully completed based on your current configurations. Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
Business Rules ValidationTask: 5718332261 — [Future] Fleet-wide daily model budgets and routing through Usage Monitor Status: Issues Found FindingsMUST_FIX: PR scope does not match the taskRequirement: “Keep this as future design/backlog work. Budget enforcement and a separate custom gateway are paused pending a later decision; this issue does not authorize implementation, deployment, credential changes, or paid model calls.” “Do not start implementation from this backlog issue without a new owner go-ahead.” Missing in code: No evidence in this PR diff of design or implementation for Usage Monitor's fleet-wide model budgets, provider routing, admission control, or policy accounting. Evidence (Code): The changed files concern Sentry deployment reporting and release identity: Suggested action: Relink or re-scope this PR to the Sentry deployment task it actually addresses. Keep this task limited to reviewed design work unless separate owner approval authorizes implementation. MUST_FIX: No organization-wide policy ledger or routing policy is definedRequirements:
Missing in code: No evidence in this PR diff of an organization-scoped daily ledger, the Evidence (Code): The PR diff contains no budget-ledger schema, policy-window calculation, provider-cost calculation, or DeepSeek/MiniMax decision logic. Suggested action: In a design document, define the organization scope, ledger entities, policy-spend calculation, MiniMax accounting separation, threshold state machine, and DST-safe America/Chicago window boundaries. MUST_FIX: Atomic admission, reservations, and reconciliation are absentRequirements:
Missing in code: No evidence in this PR diff of an atomic admission operation, reservation lifecycle, durable model-request identity, reconciliation state machine, pricing validation, or fail-closed DeepSeek behavior. Evidence (Code): The idempotency and failure handling added in Suggested action: Specify the smallest transactional admission API, reservation data model, conservative cost-bound calculation, durable request state transitions, reconciliation rules, and fail-closed conditions. Keep activation blocked until separately approved. MUST_FIX: Shared routing authority and distinguishable cost reporting are not definedRequirements:
Missing in code: No evidence in this PR diff of a provider-decision response, routing reason, central-authority enforcement, workflow integration inventory, separate accounting fields, or corresponding UI. Evidence (Code): The diff does not change application APIs, persistence models, provider routing integrations, usage telemetry, or UI components. Suggested action: Define an explicit routing decision schema with provider, reason, policy state, reservation state, and trustedness; define separate API/UI fields for policy spend, cash cost, estimates, and outstanding reservations; document how every workflow uses the single authority. MUST_FIX: Task-specific verification and rollout governance are absentRequirements:
Missing in code: No evidence in this PR diff of budget threshold tests, 19-workflow concurrency tests, reservation reconciliation tests, or timezone/DST tests. No reviewed budget-activation rollout and rollback plan is included. Evidence (Code): Suggested action: Add a task-specific test matrix covering every required boundary and failure mode, plus a reviewed rollout/rollback plan. Do not activate the policy until both artifacts receive the required review and separate implementation approval. MUST_FIX: All required design questions remain unansweredRequirements:
Missing in code: No evidence in this PR diff of an ADR, design proposal, API contract, cross-midnight reservation semantics, authenticated workflow identity, or model-usage reconciliation design. Evidence (Code): None of the changed files addresses these questions; the documentation and code shown are limited to Sentry deployment behavior. Suggested action: Resolve all three questions in a reviewed design artifact before authorizing implementation, including explicit cross-midnight attribution, client authentication, replay protection, and credential-separation rules. Intent Comparison
Acceptance Criterion Assessment
|
![kody-ai[bot]](https://avatars.githubusercontent.com/in/413034?s=60&v=4){kind=small}
|
@codex review Please review current head |
Code Review Completed! 🔥The code review was successfully completed based on your current configurations. Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Rule [2] applies to the new exported helpers in Kody rule violation: Every exported schema or utility ships with tests and must typecheck |
|
Business Rules ValidationTask: 5718332261 — [Future] Fleet-wide daily model budgets and routing through Usage Monitor Intent Comparison
FindingsMUST_FIX: PR scope does not match the task scopeRequirement: "Keep this as future design/backlog work. Budget enforcement and a separate custom gateway are paused pending a later decision; this issue does not authorize implementation, deployment, credential changes, or paid model calls." MUST_FIX: AC #1 — MISSING: organization-wide daily policy ledgerRequirement: "One organization-wide daily policy ledger shared by all participating workflows, rather than a separate allowance per repository or runner." MUST_FIX: AC #2 — MISSING: DeepSeek primary below $0.50Requirement: "DeepSeek is primary until the daily policy spend reaches $0.50 USD." MUST_FIX: AC #3 — MISSING: MiniMax routing after $0.50Requirement: "After that, MiniMax is primary and DeepSeek is fallback." MUST_FIX: AC #4 — MISSING: DeepSeek hard cap at $1.50Requirement: "At $1.50 USD of daily policy spend, stop new DeepSeek calls; MiniMax can continue." MUST_FIX: AC #5 — MISSING: MiniMax policy accounting and separate cash costsRequirement: "Count MiniMax as $0 in this routing-policy ledger. This is a policy accounting choice, not a claim that the provider is free. Preserve actual billed/estimated costs separately." MUST_FIX: AC #6 — MISSING: America/Chicago daily reset and DST handlingRequirement: "Reset the daily policy window at midnight in America/Chicago, including daylight-saving transitions." MUST_FIX: AC #7 — MISSING: atomic admission contract and shared reservationsRequirement: "Define a Usage Monitor-owned admission contract and atomic shared ledger; account for settled spend plus outstanding reservations across concurrent callers." MUST_FIX: AC #8 — MISSING: bounded DeepSeek reservationsRequirement: "Reserve a conservative upper bound before each DeepSeek request, using validated pricing and bounded input/output. Never admit a reservation that could exceed the $1.50 daily cap." MUST_FIX: AC #9 — MISSING: reservation reconciliation and ambiguous outcomesRequirement: "Reconcile reservations with reported usage/cost, with durable request IDs, idempotent retries, and clear handling for cancellation, timeout, missing usage, late responses, and crashed callers. An ambiguous outcome must not silently release potential spend." MUST_FIX: AC #10 — MISSING: pricing provenance and trusted-cost fail-closed behaviorRequirement: "Specify pricing provenance, freshness, token/cache/reasoning accounting, and safe behavior for unknown models or missing/stale usage. Fail closed for new paid DeepSeek admission when the cost bound or ledger cannot be trusted." MUST_FIX: AC #11 — MISSING: explicit and centralized routing decisionsRequirement: "Return an explicit provider/routing decision and reason; ensure all participating workflows use the same authority and cannot accidentally multiply the budget." MUST_FIX: AC #12 — MISSING: distinct policy, cash, estimate, and reservation reportingRequirement: "Keep routing-policy spend, actual cash costs, estimates, and outstanding reservations distinguishable in API responses and the UI." MUST_FIX: AC #13 — MISSING: required budget-policy test coverageRequirement: "Test threshold boundaries, concurrent callers (at least 19 workflows), duplicate requests, retries, reconciliation failures, midnight rollover, and daylight-saving changes." MUST_FIX: AC #14 — MISSING: reviewed budget-policy rollout and rollback planRequirement: "Document a reviewed rollout and rollback plan before activation." SUGGESTION: AC #15 — MISSING: smallest admission API design remains unansweredRequirement: "What is the smallest admission API that fits Usage Monitor's existing persistence and usage-telemetry contract?" SUGGESTION: AC #16 — MISSING: reservation-transition and cross-midnight attribution remain unansweredRequirement: "How should reservations affect the $0.50 routing transition, and how should in-flight work be attributed across midnight?" SUGGESTION: AC #17 — MISSING: authenticated client and reconciliation design remains unansweredRequirement: "What authenticated client identity and reconciliation mechanism prevent double-counting or bypass while keeping provider credentials out of the ledger?" Additional Task-Context CheckThe description's statement that the existing reader "is not an atomic pre-call reservation/admission contract" is addressed by AC #7 but is not implemented in this diff. The three later design questions are covered by AC #15–#17. The explicit constraint "Do not start implementation from this backlog issue without a new owner go-ahead" reinforces the leading scope-mismatch finding. Analysis performed by Kodus AI Business Rules Validator
|
| org: process.env.SENTRY_ORG, | ||
| project: process.env.SENTRY_SELF_PROJECT || "usage-monitor", | ||
| authToken: process.env.SENTRY_AUTH_TOKEN, | ||
| release: { name: sentryBuildRelease() }, |
There was a problem hiding this comment.
Revises an earlier Kody suggestion on .github/workflows/sentry-deploy.yml:60 (2026-10-06 12:11 UTC).
release: { name: sentryBuildRelease() } is passed unconditionally, so every build where sentryBuildRelease() returns undefined hands withSentryConfig an explicitly present release object with no name instead of an omitted option. sentryBuildRelease returns undefined for an unset build arg (Dockerfile:23-25 defaults SOURCE_COMMIT/GIT_COMMIT_SHA to ""), for the repo's own unknown/short-SHA values, and for any local/CI build — and per the @sentry/nextjs contract release detection is tied to the name being omitted, so the present-but-undefined option can suppress the plugin's own detection (leaving the build with no usable release name instead of the inferred one), which is the exact opposite of the comment at scripts/sentry-build-release.cjs:6. add the key only when a name was resolved. Note the guard regex at scripts/sentry-report-deploy.node-test.mjs:94 asserts the literal release: { name: sentryBuildRelease() } text, so that assertion must be relaxed alongside this change.
Prompt for LLM
File next.config.js:
Line 72:
**Revises an earlier Kody suggestion** on `.github/workflows/sentry-deploy.yml:60` (2026-10-06 12:11 UTC).
`release: { name: sentryBuildRelease() }` is passed unconditionally, so every build where `sentryBuildRelease()` returns `undefined` hands `withSentryConfig` an explicitly present release object with no name instead of an omitted option. `sentryBuildRelease` returns undefined for an unset build arg (`Dockerfile:23-25` defaults `SOURCE_COMMIT`/`GIT_COMMIT_SHA` to `""`), for the repo's own `unknown`/short-SHA values, and for any local/CI build — and per the @sentry/nextjs contract release detection is tied to the name being *omitted*, so the present-but-undefined option can suppress the plugin's own detection (leaving the build with no usable release name instead of the inferred one), which is the exact opposite of the comment at `scripts/sentry-build-release.cjs:6`. add the key only when a name was resolved. Note the guard regex at `scripts/sentry-report-deploy.node-test.mjs:94` asserts the literal `release: { name: sentryBuildRelease() }` text, so that assertion must be relaxed alongside this change.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| import { resolve } from 'node:path'; | ||
| import { fileURLToPath } from 'node:url'; | ||
|
|
||
| export const CONFIG = Object.freeze({"project": "usage-monitor", "health": "https://usage.jays.services/api/health", "repositoryId": "1284098482", "repository": "Simple-With-Us/Usage-Monitor"}); |
There was a problem hiding this comment.
This commits environment-specific health and repository configuration in shared source. Read these values through a validated runtime configuration factory instead of embedding deployment-specific literals.
Also found in:
scripts/sentry-report-deploy.mjs:8-8
Kody rule violation: Never hardcode credentials or secrets in shared code
const requiredEnv = (name) => {
const value = process.env[name]?.trim();
if (!value) throw new Error(`${name} is required`);
return value;
};
export const CONFIG = Object.freeze({
project: "usage-monitor",
health: requiredEnv("PRODUCTION_HEALTH_URL"),
repositoryId: requiredEnv("GITHUB_REPOSITORY_ID"),
repository: requiredEnv("GITHUB_REPOSITORY"),
});Prompt for LLM
File scripts/sentry-report-deploy.mjs:
Line 7:
This commits environment-specific health and repository configuration in shared source. Read these values through a validated runtime configuration factory instead of embedding deployment-specific literals.
**Also found in:**
- `scripts/sentry-report-deploy.mjs:8-8`
Suggested Code:
const requiredEnv = (name) => {
const value = process.env[name]?.trim();
if (!value) throw new Error(`${name} is required`);
return value;
};
export const CONFIG = Object.freeze({
project: "usage-monitor",
health: requiredEnv("PRODUCTION_HEALTH_URL"),
repositoryId: requiredEnv("GITHUB_REPOSITORY_ID"),
repository: requiredEnv("GITHUB_REPOSITORY"),
});
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| import { resolve } from 'node:path'; | ||
| import { fileURLToPath } from 'node:url'; | ||
|
|
||
| export const CONFIG = Object.freeze({"project": "usage-monitor", "health": "https://usage.jays.services/api/health", "repositoryId": "1284098482", "repository": "Simple-With-Us/Usage-Monitor"}); |
There was a problem hiding this comment.
The committed script exposes the production health hostname and route as deployment inventory. Keep the endpoint outside public source and read a validated runtime value instead.
Kody rule violation: Keep credentials out of public source and verify UI changes with automated screenshots
const requiredEnv = (name) => {
const value = process.env[name]?.trim();
if (!value) throw new Error(`${name} is required`);
return value;
};
export const CONFIG = Object.freeze({
project: "usage-monitor",
health: requiredEnv("PRODUCTION_HEALTH_URL"),
repositoryId: requiredEnv("GITHUB_REPOSITORY_ID"),
repository: requiredEnv("GITHUB_REPOSITORY"),
});Prompt for LLM
File scripts/sentry-report-deploy.mjs:
Line 7:
The committed script exposes the production health hostname and route as deployment inventory. Keep the endpoint outside public source and read a validated runtime value instead.
Suggested Code:
const requiredEnv = (name) => {
const value = process.env[name]?.trim();
if (!value) throw new Error(`${name} is required`);
return value;
};
export const CONFIG = Object.freeze({
project: "usage-monitor",
health: requiredEnv("PRODUCTION_HEALTH_URL"),
repositoryId: requiredEnv("GITHUB_REPOSITORY_ID"),
repository: requiredEnv("GITHUB_REPOSITORY"),
});
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| import { resolve } from 'node:path'; | ||
| import { fileURLToPath } from 'node:url'; | ||
|
|
||
| export const CONFIG = Object.freeze({"project": "usage-monitor", "health": "https://usage.jays.services/api/health", "repositoryId": "1284098482", "repository": "Simple-With-Us/Usage-Monitor"}); |
There was a problem hiding this comment.
The source string publishes a hosting and routing endpoint. Supply the deployment URL through validated runtime configuration rather than committing the private infrastructure link.
Kody rule violation: Never edit in the owner's integration tree — work only in your agent lane worktree
const requiredEnv = (name) => {
const value = process.env[name]?.trim();
if (!value) throw new Error(`${name} is required`);
return value;
};
export const CONFIG = Object.freeze({
project: "usage-monitor",
health: requiredEnv("PRODUCTION_HEALTH_URL"),
repositoryId: requiredEnv("GITHUB_REPOSITORY_ID"),
repository: requiredEnv("GITHUB_REPOSITORY"),
});Prompt for LLM
File scripts/sentry-report-deploy.mjs:
Line 7:
The source string publishes a hosting and routing endpoint. Supply the deployment URL through validated runtime configuration rather than committing the private infrastructure link.
Suggested Code:
const requiredEnv = (name) => {
const value = process.env[name]?.trim();
if (!value) throw new Error(`${name} is required`);
return value;
};
export const CONFIG = Object.freeze({
project: "usage-monitor",
health: requiredEnv("PRODUCTION_HEALTH_URL"),
repositoryId: requiredEnv("GITHUB_REPOSITORY_ID"),
repository: requiredEnv("GITHUB_REPOSITORY"),
});
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| import { resolve } from 'node:path'; | ||
| import { fileURLToPath } from 'node:url'; | ||
|
|
||
| export const CONFIG = Object.freeze({"project": "usage-monitor", "health": "https://usage.jays.services/api/health", "repositoryId": "1284098482", "repository": "Simple-With-Us/Usage-Monitor"}); |
There was a problem hiding this comment.
Private infrastructure endpoints must not be committed even when they are not credentials. Replace the literal endpoint and deployment identifiers with validated runtime configuration.
Kody rule violation: Never expose secrets or private infrastructure values; reuse existing fleet env vars
const requiredEnv = (name) => {
const value = process.env[name]?.trim();
if (!value) throw new Error(`${name} is required`);
return value;
};
export const CONFIG = Object.freeze({
project: "usage-monitor",
health: requiredEnv("PRODUCTION_HEALTH_URL"),
repositoryId: requiredEnv("GITHUB_REPOSITORY_ID"),
repository: requiredEnv("GITHUB_REPOSITORY"),
});Prompt for LLM
File scripts/sentry-report-deploy.mjs:
Line 7:
Private infrastructure endpoints must not be committed even when they are not credentials. Replace the literal endpoint and deployment identifiers with validated runtime configuration.
Suggested Code:
const requiredEnv = (name) => {
const value = process.env[name]?.trim();
if (!value) throw new Error(`${name} is required`);
return value;
};
export const CONFIG = Object.freeze({
project: "usage-monitor",
health: requiredEnv("PRODUCTION_HEALTH_URL"),
repositoryId: requiredEnv("GITHUB_REPOSITORY_ID"),
repository: requiredEnv("GITHUB_REPOSITORY"),
});
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
2 participants
Summary
Verification
Task state
Owner-approved code/test publication with task notes here, excluding existing effort-log and other documentation uploads. SDK redaction/CSP work remains in a separate change. Draft pending hosted gates and review. Production is not yet verified; normal main webhook deployment only.
Verification update (2026-10-06)
Hosted checks on
0477a067443dd74da34d4e936ca2f0536ab83cdepassed: required verify (2,953 tests passed, one skipped, 11 todo), gitleaks, focused reporter tests, CodeQL, smoke, and Docker build. Automated review is still running. Production rollout is not yet verified.Corrected rollout state (2026-10-06)
Existing repository automation merged this PR at 12:25:59 UTC as
6a21cf4a4457a1c5d0ccb1fd2a4598fa798dbafc; the earlier pending/unmerged status above is superseded. The separately required named Codex review was quota-blocked, so this merge did not satisfy that gate.Production verification confirmed the exact healthy revision. The initial Sentry lookup raced release visibility and failed closed. A reporter-only retry (no application redeploy) succeeded, and an independent Sentry read confirmed one production deployment receipt, ID 165610508, release/ref/lastCommit matching the live SHA, and name
production seat:codex pr:#1610. Successful run: https://github.com/Simple-With-Us/Usage-Monitor/actions/runs/37463623620 . Bounded missing-release read retries are proposed separately in draft #1612.