feat(db): add resource_usage_logs table with repository functions - #246
AbdulmalikAlayande merged 5 commits into
Conversation
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (15)
📜 Recent review details🧰 Additional context used🪛 ast-grep (0.44.0)tests/commands/db.test.ts[warning] 66-73: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use. (detect-non-literal-fs-filename-typescript) 🪛 Betterleaks (1.6.0)tests/core/vault.test.ts[high] 26-26: Detected a Generic API Key, potentially exposing access to various services and sensitive operations. (generic-api-key) [high] 55-55: Detected a Generic API Key, potentially exposing access to various services and sensitive operations. (generic-api-key) [high] 168-168: Detected a Generic API Key, potentially exposing access to various services and sensitive operations. (generic-api-key) [high] 198-198: Detected a Generic API Key, potentially exposing access to various services and sensitive operations. (generic-api-key) 🪛 GitHub Check: GitGuardian Security Checkstests/core/vault.test.ts[error] 116-116: GitGuardian detected hardcoded secret in commit 0a77b16. GitGuardian id: 34377146. Secret type: Generic High Entropy Secret. Please revoke/rotate and remove the secret from the code. [error] 54-54: GitGuardian detected hardcoded secret in commit 0a77b16. GitGuardian id: 34377147. Secret type: Generic High Entropy Secret. Please revoke/rotate and remove the secret from the code. [error] 114-114: GitGuardian detected hardcoded secret in commit 0a77b16. GitGuardian id: 34377148. Secret type: Generic High Entropy Secret. Please revoke/rotate and remove the secret from the code. 🛑 Comments failed to post (15)
🔇 Additional comments (14)
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds a new SQLite-backed ChangesResource usage logs
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 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: 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 `@src/db/repositories.ts`:
- Around line 899-903: The recorded_at fallback in the repository insert path is
using a non-ISO timestamp, which creates mixed text formats and breaks
consistent sorting/filtering. Update the relevant query in the repository code
that uses COALESCE(`@recorded_at`, CURRENT_TIMESTAMP) so the fallback matches the
ISO-8601 UTC format used by callers and stored rows. Keep the change localized
to the insert/query logic in repositories.ts so ORDER BY recorded_at DESC and
since-based comparisons remain consistent.
In `@tests/db/resource_usage_logs.test.ts`:
- Around line 198-223: Add a repository test in resource_usage_logs.test.ts that
mixes a row inserted with the default recorded_at and another inserted with an
explicit ISO-8601 recorded_at to cover the CURRENT_TIMESTAMP vs ISO ordering
path. Use insertResourceUsageLog and getResourceUsageLogs to verify the ordering
still matches expected descending recorded_at behavior when both timestamp
sources are present, and update the existing ordering/since coverage if needed
to include this mixed-timestamp case.
🪄 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: 292d7b71-7c32-4b13-924c-349d1737e4a1
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
docs/pr-164.mdsrc/db/migrations/001_resource_usage_logs.sqlsrc/db/repositories.tssrc/db/schema.sqltests/db/resource_usage_logs.test.ts
📜 Review details
⚠️ CI failures not shown inline (2)
GitHub Actions: CI Pipeline / build-and-test (22.x): feat(db): add resource_usage_logs table with repository functions
Conclusion: failure
##[group]Run npm run lint
�[36;1mnpm run lint�[0m
shell: /usr/bin/bash -e {0}
##[endgroup]
> sorokeep@0.1.2 lint
> eslint src/ tests/
/home/runner/work/sorokeep/sorokeep/src/alerts/slack.ts
##[warning] 66:15 warning 'resourceType' is assigned a value but never used `@typescript-eslint/no-unused-vars`
/home/runner/work/sorokeep/sorokeep/src/commands/alerts.ts
##[warning] 11:5 warning 'getResourceAlertConfigsForContract' is defined but never used `@typescript-eslint/no-unused-vars`
##[warning] 12:5 warning 'deleteResourceAlertConfig' is defined but never used `@typescript-eslint/no-unused-vars`
/home/runner/work/sorokeep/sorokeep/src/core/discovery.ts
92:9 warning Unused eslint-disable directive (no problems were reported from 'no-constant-condition')
/home/runner/work/sorokeep/sorokeep/src/core/introspection.ts
##[warning] 10:5 warning 'db' is defined but never used `@typescript-eslint/no-unused-vars`
##[warning] 11:5 warning 'network' is defined but never used `@typescript-eslint/no-unused-vars`
##[warning] 12:5 warning 'rpcUrl' is defined but never used `@typescript-eslint/no-unused-vars`
/home/runner/work/sorokeep/sorokeep/src/core/rent_projection.ts
##[error] 128:18 error An interface declaring no members is equivalent to its supertype `@typescript-eslint/no-empty-object-type`
GitHub Actions: CI Pipeline / 0_build-and-test (22.x).txt: feat(db): add resource_usage_logs table with repository functions
Conclusion: failure
##[group]Run npm run lint
�[36;1mnpm run lint�[0m
shell: /usr/bin/bash -e {0}
##[endgroup]
> sorokeep@0.1.2 lint
> eslint src/ tests/
/home/runner/work/sorokeep/sorokeep/src/alerts/slack.ts
##[warning] 66:15 warning 'resourceType' is assigned a value but never used `@typescript-eslint/no-unused-vars`
/home/runner/work/sorokeep/sorokeep/src/commands/alerts.ts
##[warning] 11:5 warning 'getResourceAlertConfigsForContract' is defined but never used `@typescript-eslint/no-unused-vars`
##[warning] 12:5 warning 'deleteResourceAlertConfig' is defined but never used `@typescript-eslint/no-unused-vars`
/home/runner/work/sorokeep/sorokeep/src/core/discovery.ts
92:9 warning Unused eslint-disable directive (no problems were reported from 'no-constant-condition')
/home/runner/work/sorokeep/sorokeep/src/core/introspection.ts
##[warning] 10:5 warning 'db' is defined but never used `@typescript-eslint/no-unused-vars`
##[warning] 11:5 warning 'network' is defined but never used `@typescript-eslint/no-unused-vars`
##[warning] 12:5 warning 'rpcUrl' is defined but never used `@typescript-eslint/no-unused-vars`
/home/runner/work/sorokeep/sorokeep/src/core/rent_projection.ts
##[error] 128:18 error An interface declaring no members is equivalent to its supertype `@typescript-eslint/no-empty-object-type`
🔇 Additional comments (4)
src/db/schema.sql (1)
125-145: LGTM!src/db/migrations/001_resource_usage_logs.sql (1)
1-28: LGTM!docs/pr-164.md (1)
1-79: LGTM!src/db/repositories.ts (1)
283-283: LGTM!Also applies to: 807-807
|
The build errors referencing snapshotRow and currentDayRow in repositories.ts are pre-existing in the upstream codebase and unrelated to this PR. This PR only adds the resource_usage_logs table, schema migration, and repository functions. Local build and lint pass clean with zero errors. |
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| 34377146 | Triggered | Generic High Entropy Secret | 0a77b16 | tests/core/vault.test.ts | View secret |
| 34377147 | Triggered | Generic High Entropy Secret | 0a77b16 | tests/core/vault.test.ts | View secret |
| 34377148 | Triggered | Generic High Entropy Secret | 0a77b16 | tests/core/vault.test.ts | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secrets safely. Learn here the best practices.
- Revoke and rotate these secrets.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
|
@coderabbitai resolve |
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/db/repositories.ts (2)
1090-1091: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUpdate the
recorded_atAPI comment to match the actual fallback.The implementation now writes an ISO-8601 UTC timestamp via
strftime(...), so theCURRENT_TIMESTAMPwording is stale and misleading.🛠️ Suggested change
- /** Optional ISO-8601 timestamp; defaults to CURRENT_TIMESTAMP when omitted. */ + /** Optional ISO-8601 UTC timestamp; defaults to `strftime('%Y-%m-%dT%H:%M:%fZ', 'now')` when omitted. */🤖 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 `@src/db/repositories.ts` around lines 1090 - 1091, The `recorded_at` field comment is stale and should match the actual fallback used by the repository implementation. Update the JSDoc in `repositories.ts` near `recorded_at` to describe the ISO-8601 UTC timestamp generated by the existing write path (the one using `strftime(...)`) instead of saying `CURRENT_TIMESTAMP`, so the API documentation aligns with `recorded_at` and the surrounding repository insert logic.
1156-1166: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReject non-integer
limitvalues before building the query.Only negative numbers are rejected here.
NaN,Infinity, or1.5still flow intoLIMIT ${limit}, which can produce runtime SQL errors or undefined paging behavior.🛠️ Suggested change
- if (limit !== undefined && limit < 0) { - throw new Error("limit must be non-negative"); + if (limit !== undefined && (!Number.isInteger(limit) || limit < 0)) { + throw new Error("limit must be a non-negative integer"); } @@ - const limitClause = limit !== undefined ? `LIMIT ${limit}` : ""; + const limitClause = limit !== undefined ? "LIMIT `@limit`" : ""; @@ - `).all({ contractId, since: since ?? null }) as ResourceUsageLog[]; + `).all({ contractId, since: since ?? null, limit }) as ResourceUsageLog[];Also applies to: 1168-1174
🤖 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 `@src/db/repositories.ts` around lines 1156 - 1166, In the query-building logic that assembles the SQL with limitClause, tighten validation for limit so only finite integers are accepted before interpolation. Update the existing limit guard alongside the current negative check to reject NaN, Infinity, and fractional values, and keep the error thrown from this repository method consistent with the existing validation style. Apply the same validation wherever this limit is used in the related query paths referenced by the same repository routine.
🤖 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 `@src/alerts/pagerduty.ts`:
- Line 1: Remove the top-level `@typescript-eslint/no-unused-vars` suppression in
pagerduty.ts and clean up the underlying unused variables/imports instead of
masking them. Review the file’s exported functions and helpers in pagerduty.ts,
delete any genuinely unused parameters, locals, or imports, and keep only the
symbols that are actually referenced so the lint rule can remain enabled.
- Around line 20-21: The dedup key in the `state_changed` branch of
`pagerduty.ts` can include the literal `"undefined"` when `event.entry.keyXdr`
is missing. Update the `event.type === "state_changed"` handling in the dedup
key builder to use a safe fallback (or tighten the event typing so `keyXdr` is
guaranteed present) before interpolating `event.entry.keyXdr`, keeping the key
generation logic consistent with the existing `event.entry.type` fallback
behavior.
In `@src/core/vault.ts`:
- Around line 94-100: The Vault request path in the fetch-based lookup is
missing a timeout, so the call can hang forever on an unresponsive server.
Update the request in the vault logic that wraps fetch and VaultSecretError
handling to use an AbortSignal with a timeout, and make sure the existing catch
block logs and rethrows a clear timeout-related failure when the abort is
triggered.
In `@tests/alerts/state_change_alerts.test.ts`:
- Around line 35-45: The State Change Alerts test suite creates and closes a
test database in beforeEach/afterEach, but no assertions in this file use db, so
the SQLite lifecycle is unnecessary. Remove the db setup and teardown from
describe("State Change Alerts"), including getDatabaseForTesting() and
db.close(), and keep the vi.clearAllMocks() test isolation if needed.
- Around line 1-16: Remove the unused test imports and the ESLint suppression
from the state change alerts test file. In `state_change_alerts.test.ts`, delete
`Database`, `getDatabaseForTesting`, and the unused repository imports
(`insertContract`, `upsertEntry`, `getEntriesForContract`, `insertAlertConfig`,
`getAlertConfigsForContract`) since only the alert type and dispatcher helpers
are needed. Also remove the `eslint-disable` comment once those unused bindings
are gone.
In `@tests/commands/budget.test.ts`:
- Around line 12-13: The mock spy variables are using overly broad typing, so
update the test setup in the budget test to use the specific spy types instead
of any. Replace the declarations for mockExit and mockLog with
vi.SpiedFunction<typeof process.exit> and vi.SpiedFunction<typeof console.log>,
and keep the existing mock assignments in the same test scope so the typed spies
are used consistently throughout the test.
In `@tests/commands/db.test.ts`:
- Around line 41-92: The backup/restore shape is missing the new
resource_usage_logs table, so update the backup flow to persist and restore it
end-to-end. In src/db/backup.ts, adjust the exportDatabase and importDatabase
logic to include resource_usage_logs alongside the existing collections, then
update the db command tests in registerDbCommand mocks to expect that field in
both the export JSON and the imported payload.
In `@tests/core/budget.test.ts`:
- Around line 43-46: The `getMonthlySpendProgress` test in `budget.test.ts` is
just a placeholder and does not assert anything. Either implement it with real
assertions by mocking or fixtureing `getContractCostSummary`/any DB access used
by `getMonthlySpendProgress`, or remove the empty `it` block entirely so the
test suite stays clean.
In `@tests/core/vault.test.ts`:
- Line 55: The vault test still uses a value that can resemble a real secret,
triggering scanner alerts. Update the private_key test fixture in vault.test.ts
to use an obviously fake, non-production-looking token consistent with the other
test constant, and keep it within the vault-related test setup so GitGuardian no
longer flags it.
- Line 26: The vault test fixture uses a token-like secret_key value that
triggers GitGuardian scanning. Update the test data in vault.test.ts to use an
obviously fake, non-token-looking placeholder (for example via the secret_key
fixture in the relevant test case) so the scanner won’t flag it; if needed, also
ensure the test path is covered by a GitGuardian ignore rule for
tests/**/*.test.ts.
- Around line 86-90: The test in vault.test.ts should be simplified by removing
the redundant try/catch around resolver.getSecret, since the existing
rejects.toThrow assertion already covers the failure path. Keep the rejection
assertion and delete the manual error capture and message match so the test
remains focused and non-duplicative in the relevant vault secret resolution test
case.
- Line 168: The mocked secret value in the vault test is too realistic and
triggers GitGuardian scanning, so replace it in the test fixture with an
obviously fake token value. Update the stubbed response in the vault test around
the json() mock to use a clearly non-sensitive placeholder while keeping the
same shape and length expectations if needed.
- Line 198: The test stub in the vault test is still using a token-like string
that can trigger GitGuardian, so replace the current secret value in the async
json response with an obviously fake non-secret test placeholder. Update the
mocked response in the vault test so the unique secret payload is clearly
synthetic and not shaped like a real token, keeping the same test flow intact.
- Around line 184-193: The `vi.mock` call inside the test is unsafe because
Vitest hoists mocks, so it should not live in an `it()` block. Move the
`vi.mock("../../src/utils/config.js", ...)` setup to the top level of
`tests/core/vault.test.ts` near the other test mocks, and remove the in-test
mock from the affected case; if the test needs per-case dynamic behavior, switch
to `vi.doMock` instead. Use the `loadConfig` mock and the vault config fixture
as the identifiers to relocate.
In `@tests/db/resource_usage_logs.test.ts`:
- Around line 241-246: The ordering test uses a hard-coded future timestamp that
will stop being “future” once the default recorded_at catches up, so make the
timestamp relative to the test clock instead. Update the inserts in the resource
usage log test to derive the “future” recorded_at from the same mocked/fixed
time used by the test, and keep the ordering assertions in terms of that
relative value. Locate the affected rows in the resource usage log test around
the insertResourceUsageLog calls.
---
Outside diff comments:
In `@src/db/repositories.ts`:
- Around line 1090-1091: The `recorded_at` field comment is stale and should
match the actual fallback used by the repository implementation. Update the
JSDoc in `repositories.ts` near `recorded_at` to describe the ISO-8601 UTC
timestamp generated by the existing write path (the one using `strftime(...)`)
instead of saying `CURRENT_TIMESTAMP`, so the API documentation aligns with
`recorded_at` and the surrounding repository insert logic.
- Around line 1156-1166: In the query-building logic that assembles the SQL with
limitClause, tighten validation for limit so only finite integers are accepted
before interpolation. Update the existing limit guard alongside the current
negative check to reject NaN, Infinity, and fractional values, and keep the
error thrown from this repository method consistent with the existing validation
style. Apply the same validation wherever this limit is used in the related
query paths referenced by the same repository routine.
🪄 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: 08a37c0a-3017-4c8f-b6f8-c899a852354e
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (15)
src/alerts/pagerduty.tssrc/core/rent_projection.tssrc/core/vault.tssrc/db/repositories.tssrc/db/schema.sqltests/alerts/state_change_alerts.test.tstests/commands/budget.test.tstests/commands/db.test.tstests/core/aws_secrets.test.tstests/core/budget.test.tstests/core/discovery.test.tstests/core/state_diff.test.tstests/core/vault.test.tstests/db/resource_usage_logs.test.tstests/rpc/client.test.ts
📜 Review details
🧰 Additional context used
🪛 ast-grep (0.44.0)
tests/commands/db.test.ts
[warning] 66-73: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(filePath, JSON.stringify({
contracts: [],
contract_entries: [],
extension_policies: [],
alert_configs: [],
channel_accounts: [],
resource_alert_configs: [],
}))
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename-typescript)
🪛 Betterleaks (1.6.0)
tests/core/vault.test.ts
[high] 26-26: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 55-55: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 168-168: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 198-198: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🪛 GitHub Check: GitGuardian Security Checks
tests/core/vault.test.ts
[error] 116-116: GitGuardian detected hardcoded secret in commit 0a77b16. GitGuardian id: 34377146. Secret type: Generic High Entropy Secret. Please revoke/rotate and remove the secret from the code.
[error] 54-54: GitGuardian detected hardcoded secret in commit 0a77b16. GitGuardian id: 34377147. Secret type: Generic High Entropy Secret. Please revoke/rotate and remove the secret from the code.
[error] 114-114: GitGuardian detected hardcoded secret in commit 0a77b16. GitGuardian id: 34377148. Secret type: Generic High Entropy Secret. Please revoke/rotate and remove the secret from the code.
🔇 Additional comments (14)
tests/rpc/client.test.ts (1)
394-458: LGTM!tests/core/discovery.test.ts (2)
77-129: LGTM!
131-148: LGTM!tests/alerts/state_change_alerts.test.ts (2)
50-137: LGTM!
139-278: LGTM!tests/core/aws_secrets.test.ts (1)
1-64: LGTM!src/core/rent_projection.ts (1)
128-128: LGTM!tests/core/budget.test.ts (1)
1-47: LGTM!src/alerts/pagerduty.ts (3)
39-42: LGTM!
61-75: LGTM!
91-102: LGTM!src/core/vault.ts (1)
1-210: LGTM!tests/core/state_diff.test.ts (1)
1-217: LGTM!tests/commands/budget.test.ts (1)
1-56: LGTM!
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/db/repositories.ts (2)
1090-1091: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUpdate the
recorded_atAPI comment to match the actual fallback.The implementation now writes an ISO-8601 UTC timestamp via
strftime(...), so theCURRENT_TIMESTAMPwording is stale and misleading.🛠️ Suggested change
- /** Optional ISO-8601 timestamp; defaults to CURRENT_TIMESTAMP when omitted. */ + /** Optional ISO-8601 UTC timestamp; defaults to `strftime('%Y-%m-%dT%H:%M:%fZ', 'now')` when omitted. */🤖 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 `@src/db/repositories.ts` around lines 1090 - 1091, The `recorded_at` field comment is stale and should match the actual fallback used by the repository implementation. Update the JSDoc in `repositories.ts` near `recorded_at` to describe the ISO-8601 UTC timestamp generated by the existing write path (the one using `strftime(...)`) instead of saying `CURRENT_TIMESTAMP`, so the API documentation aligns with `recorded_at` and the surrounding repository insert logic.
1156-1166: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReject non-integer
limitvalues before building the query.Only negative numbers are rejected here.
NaN,Infinity, or1.5still flow intoLIMIT ${limit}, which can produce runtime SQL errors or undefined paging behavior.🛠️ Suggested change
- if (limit !== undefined && limit < 0) { - throw new Error("limit must be non-negative"); + if (limit !== undefined && (!Number.isInteger(limit) || limit < 0)) { + throw new Error("limit must be a non-negative integer"); } @@ - const limitClause = limit !== undefined ? `LIMIT ${limit}` : ""; + const limitClause = limit !== undefined ? "LIMIT `@limit`" : ""; @@ - `).all({ contractId, since: since ?? null }) as ResourceUsageLog[]; + `).all({ contractId, since: since ?? null, limit }) as ResourceUsageLog[];Also applies to: 1168-1174
🤖 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 `@src/db/repositories.ts` around lines 1156 - 1166, In the query-building logic that assembles the SQL with limitClause, tighten validation for limit so only finite integers are accepted before interpolation. Update the existing limit guard alongside the current negative check to reject NaN, Infinity, and fractional values, and keep the error thrown from this repository method consistent with the existing validation style. Apply the same validation wherever this limit is used in the related query paths referenced by the same repository routine.
🤖 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 `@src/alerts/pagerduty.ts`:
- Line 1: Remove the top-level `@typescript-eslint/no-unused-vars` suppression in
pagerduty.ts and clean up the underlying unused variables/imports instead of
masking them. Review the file’s exported functions and helpers in pagerduty.ts,
delete any genuinely unused parameters, locals, or imports, and keep only the
symbols that are actually referenced so the lint rule can remain enabled.
- Around line 20-21: The dedup key in the `state_changed` branch of
`pagerduty.ts` can include the literal `"undefined"` when `event.entry.keyXdr`
is missing. Update the `event.type === "state_changed"` handling in the dedup
key builder to use a safe fallback (or tighten the event typing so `keyXdr` is
guaranteed present) before interpolating `event.entry.keyXdr`, keeping the key
generation logic consistent with the existing `event.entry.type` fallback
behavior.
In `@src/core/vault.ts`:
- Around line 94-100: The Vault request path in the fetch-based lookup is
missing a timeout, so the call can hang forever on an unresponsive server.
Update the request in the vault logic that wraps fetch and VaultSecretError
handling to use an AbortSignal with a timeout, and make sure the existing catch
block logs and rethrows a clear timeout-related failure when the abort is
triggered.
In `@tests/alerts/state_change_alerts.test.ts`:
- Around line 35-45: The State Change Alerts test suite creates and closes a
test database in beforeEach/afterEach, but no assertions in this file use db, so
the SQLite lifecycle is unnecessary. Remove the db setup and teardown from
describe("State Change Alerts"), including getDatabaseForTesting() and
db.close(), and keep the vi.clearAllMocks() test isolation if needed.
- Around line 1-16: Remove the unused test imports and the ESLint suppression
from the state change alerts test file. In `state_change_alerts.test.ts`, delete
`Database`, `getDatabaseForTesting`, and the unused repository imports
(`insertContract`, `upsertEntry`, `getEntriesForContract`, `insertAlertConfig`,
`getAlertConfigsForContract`) since only the alert type and dispatcher helpers
are needed. Also remove the `eslint-disable` comment once those unused bindings
are gone.
In `@tests/commands/budget.test.ts`:
- Around line 12-13: The mock spy variables are using overly broad typing, so
update the test setup in the budget test to use the specific spy types instead
of any. Replace the declarations for mockExit and mockLog with
vi.SpiedFunction<typeof process.exit> and vi.SpiedFunction<typeof console.log>,
and keep the existing mock assignments in the same test scope so the typed spies
are used consistently throughout the test.
In `@tests/commands/db.test.ts`:
- Around line 41-92: The backup/restore shape is missing the new
resource_usage_logs table, so update the backup flow to persist and restore it
end-to-end. In src/db/backup.ts, adjust the exportDatabase and importDatabase
logic to include resource_usage_logs alongside the existing collections, then
update the db command tests in registerDbCommand mocks to expect that field in
both the export JSON and the imported payload.
In `@tests/core/budget.test.ts`:
- Around line 43-46: The `getMonthlySpendProgress` test in `budget.test.ts` is
just a placeholder and does not assert anything. Either implement it with real
assertions by mocking or fixtureing `getContractCostSummary`/any DB access used
by `getMonthlySpendProgress`, or remove the empty `it` block entirely so the
test suite stays clean.
In `@tests/core/vault.test.ts`:
- Line 55: The vault test still uses a value that can resemble a real secret,
triggering scanner alerts. Update the private_key test fixture in vault.test.ts
to use an obviously fake, non-production-looking token consistent with the other
test constant, and keep it within the vault-related test setup so GitGuardian no
longer flags it.
- Line 26: The vault test fixture uses a token-like secret_key value that
triggers GitGuardian scanning. Update the test data in vault.test.ts to use an
obviously fake, non-token-looking placeholder (for example via the secret_key
fixture in the relevant test case) so the scanner won’t flag it; if needed, also
ensure the test path is covered by a GitGuardian ignore rule for
tests/**/*.test.ts.
- Around line 86-90: The test in vault.test.ts should be simplified by removing
the redundant try/catch around resolver.getSecret, since the existing
rejects.toThrow assertion already covers the failure path. Keep the rejection
assertion and delete the manual error capture and message match so the test
remains focused and non-duplicative in the relevant vault secret resolution test
case.
- Line 168: The mocked secret value in the vault test is too realistic and
triggers GitGuardian scanning, so replace it in the test fixture with an
obviously fake token value. Update the stubbed response in the vault test around
the json() mock to use a clearly non-sensitive placeholder while keeping the
same shape and length expectations if needed.
- Line 198: The test stub in the vault test is still using a token-like string
that can trigger GitGuardian, so replace the current secret value in the async
json response with an obviously fake non-secret test placeholder. Update the
mocked response in the vault test so the unique secret payload is clearly
synthetic and not shaped like a real token, keeping the same test flow intact.
- Around line 184-193: The `vi.mock` call inside the test is unsafe because
Vitest hoists mocks, so it should not live in an `it()` block. Move the
`vi.mock("../../src/utils/config.js", ...)` setup to the top level of
`tests/core/vault.test.ts` near the other test mocks, and remove the in-test
mock from the affected case; if the test needs per-case dynamic behavior, switch
to `vi.doMock` instead. Use the `loadConfig` mock and the vault config fixture
as the identifiers to relocate.
In `@tests/db/resource_usage_logs.test.ts`:
- Around line 241-246: The ordering test uses a hard-coded future timestamp that
will stop being “future” once the default recorded_at catches up, so make the
timestamp relative to the test clock instead. Update the inserts in the resource
usage log test to derive the “future” recorded_at from the same mocked/fixed
time used by the test, and keep the ordering assertions in terms of that
relative value. Locate the affected rows in the resource usage log test around
the insertResourceUsageLog calls.
---
Outside diff comments:
In `@src/db/repositories.ts`:
- Around line 1090-1091: The `recorded_at` field comment is stale and should
match the actual fallback used by the repository implementation. Update the
JSDoc in `repositories.ts` near `recorded_at` to describe the ISO-8601 UTC
timestamp generated by the existing write path (the one using `strftime(...)`)
instead of saying `CURRENT_TIMESTAMP`, so the API documentation aligns with
`recorded_at` and the surrounding repository insert logic.
- Around line 1156-1166: In the query-building logic that assembles the SQL with
limitClause, tighten validation for limit so only finite integers are accepted
before interpolation. Update the existing limit guard alongside the current
negative check to reject NaN, Infinity, and fractional values, and keep the
error thrown from this repository method consistent with the existing validation
style. Apply the same validation wherever this limit is used in the related
query paths referenced by the same repository routine.
🪄 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: 08a37c0a-3017-4c8f-b6f8-c899a852354e
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (15)
src/alerts/pagerduty.tssrc/core/rent_projection.tssrc/core/vault.tssrc/db/repositories.tssrc/db/schema.sqltests/alerts/state_change_alerts.test.tstests/commands/budget.test.tstests/commands/db.test.tstests/core/aws_secrets.test.tstests/core/budget.test.tstests/core/discovery.test.tstests/core/state_diff.test.tstests/core/vault.test.tstests/db/resource_usage_logs.test.tstests/rpc/client.test.ts
📜 Review details
🔇 Additional comments (14)
tests/rpc/client.test.ts (1)
394-458: LGTM!tests/core/discovery.test.ts (2)
77-129: LGTM!
131-148: LGTM!tests/alerts/state_change_alerts.test.ts (2)
50-137: LGTM!
139-278: LGTM!tests/core/aws_secrets.test.ts (1)
1-64: LGTM!src/core/rent_projection.ts (1)
128-128: LGTM!tests/core/budget.test.ts (1)
1-47: LGTM!src/alerts/pagerduty.ts (3)
39-42: LGTM!
61-75: LGTM!
91-102: LGTM!src/core/vault.ts (1)
1-210: LGTM!tests/core/state_diff.test.ts (1)
1-217: LGTM!tests/commands/budget.test.ts (1)
1-56: LGTM!
🛑 Comments failed to post (15)
src/alerts/pagerduty.ts (2)
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Remove the ESLint suppression and clean up unused variables.
The
/* eslint-disable@typescript-eslint/no-unused-vars*/directive at the top of the file indicates there are unused variables that should be removed rather than suppressed. This degrades code hygiene and masks real issues.🤖 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 `@src/alerts/pagerduty.ts` at line 1, Remove the top-level `@typescript-eslint/no-unused-vars` suppression in pagerduty.ts and clean up the underlying unused variables/imports instead of masking them. Review the file’s exported functions and helpers in pagerduty.ts, delete any genuinely unused parameters, locals, or imports, and keep only the symbols that are actually referenced so the lint rule can remain enabled.
20-21: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Guard against undefined
event.entry.keyXdrin dedup key.Line 23 shows
event.entry.keyXdrcan be falsy (fallback toevent.entry.type). Forstate_changedevents, ifkeyXdris undefined, the dedup key will contain the literal string"undefined". Use a fallback or ensure the type guaranteeskeyXdris always present forstate_changed.🤖 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 `@src/alerts/pagerduty.ts` around lines 20 - 21, The dedup key in the `state_changed` branch of `pagerduty.ts` can include the literal `"undefined"` when `event.entry.keyXdr` is missing. Update the `event.type === "state_changed"` handling in the dedup key builder to use a safe fallback (or tighten the event typing so `keyXdr` is guaranteed present) before interpolating `event.entry.keyXdr`, keeping the key generation logic consistent with the existing `event.entry.type` fallback behavior.src/core/vault.ts (1)
94-100: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Add timeout to
fetchcall to prevent indefinite hangs.The
fetch(url, { headers })call lacks a timeout option. In production, unresponsive Vault instances could cause request threads to hang indefinitely. Add anAbortSignalwith timeout.🔧 Proposed fix
let res: Response; try { - res = await fetch(url, { headers }); + const controller = new AbortController(); + const timeoutId = setTimeout(() => controller.abort(), 10000); + res = await fetch(url, { headers, signal: controller.signal }); + clearTimeout(timeoutId); } catch (err) {📝 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.try { const controller = new AbortController(); const timeoutId = setTimeout(() => controller.abort(), 10000); res = await fetch(url, { headers, signal: controller.signal }); clearTimeout(timeoutId); } catch (err) { const message = err instanceof Error ? err.message : String(err); logger.error(`Vault request failed: ${message}`); throw new VaultSecretError(`Vault request failed: ${message}`); }🤖 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 `@src/core/vault.ts` around lines 94 - 100, The Vault request path in the fetch-based lookup is missing a timeout, so the call can hang forever on an unresponsive server. Update the request in the vault logic that wraps fetch and VaultSecretError handling to use an AbortSignal with a timeout, and make sure the existing catch block logs and rethrows a clear timeout-related failure when the abort is triggered.tests/alerts/state_change_alerts.test.ts (2)
1-16: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Remove unused imports and ESLint suppression.
Database,getDatabaseForTesting, and all../../src/db/repositoriesimports are unused in this test file. Remove them and theeslint-disablecomment.🤖 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 `@tests/alerts/state_change_alerts.test.ts` around lines 1 - 16, Remove the unused test imports and the ESLint suppression from the state change alerts test file. In `state_change_alerts.test.ts`, delete `Database`, `getDatabaseForTesting`, and the unused repository imports (`insertContract`, `upsertEntry`, `getEntriesForContract`, `insertAlertConfig`, `getAlertConfigsForContract`) since only the alert type and dispatcher helpers are needed. Also remove the `eslint-disable` comment once those unused bindings are gone.
35-45: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value
Database lifecycle without database assertions.
beforeEach/afterEachmanage a test database that is never exercised by any assertion in this file. If state-change alert tests don't need SQLite, remove the DB lifecycle to speed up the suite.🤖 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 `@tests/alerts/state_change_alerts.test.ts` around lines 35 - 45, The State Change Alerts test suite creates and closes a test database in beforeEach/afterEach, but no assertions in this file use db, so the SQLite lifecycle is unnecessary. Remove the db setup and teardown from describe("State Change Alerts"), including getDatabaseForTesting() and db.close(), and keep the vi.clearAllMocks() test isolation if needed.tests/commands/budget.test.ts (1)
12-13: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Use specific types instead of
anyfor mock spies.
mockExitandmockLogare typed asany. Usevi.SpiedFunction<typeof process.exit>andvi.SpiedFunction<typeof console.log>for better type safety and IDE support.🤖 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 `@tests/commands/budget.test.ts` around lines 12 - 13, The mock spy variables are using overly broad typing, so update the test setup in the budget test to use the specific spy types instead of any. Replace the declarations for mockExit and mockLog with vi.SpiedFunction<typeof process.exit> and vi.SpiedFunction<typeof console.log>, and keep the existing mock assignments in the same test scope so the typed spies are used consistently throughout the test.tests/commands/db.test.ts (1)
41-92: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Backup/restore contract omits
resource_usage_logs.The mocked
exportDatabase/importDatabaseshapes in this test match the currentsrc/db/backup.tsbehavior, which does not includeresource_usage_logs. Since this PR introduces theresource_usage_logstable for historical analytics, the backup module should persist and restore it too; otherwise resource consumption history will be silently dropped during backup/restore operations.Please update
src/db/backup.tsto includeresource_usage_logsin both export and import, then update these test mocks to match.🧰 Tools
🪛 ast-grep (0.44.0)
[warning] 66-73: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(filePath, JSON.stringify({
contracts: [],
contract_entries: [],
extension_policies: [],
alert_configs: [],
channel_accounts: [],
resource_alert_configs: [],
}))
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').(detect-non-literal-fs-filename-typescript)
🤖 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 `@tests/commands/db.test.ts` around lines 41 - 92, The backup/restore shape is missing the new resource_usage_logs table, so update the backup flow to persist and restore it end-to-end. In src/db/backup.ts, adjust the exportDatabase and importDatabase logic to include resource_usage_logs alongside the existing collections, then update the db command tests in registerDbCommand mocks to expect that field in both the export JSON and the imported payload.tests/core/budget.test.ts (1)
43-46: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Implement or remove the placeholder test.
The
getMonthlySpendProgresstest is empty and only contains a comment. Either implement the test with proper mocks/fixtures or remove it to keep the test suite clean.🤖 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 `@tests/core/budget.test.ts` around lines 43 - 46, The `getMonthlySpendProgress` test in `budget.test.ts` is just a placeholder and does not assert anything. Either implement it with real assertions by mocking or fixtureing `getContractCostSummary`/any DB access used by `getMonthlySpendProgress`, or remove the empty `it` block entirely so the test suite stays clean.tests/core/vault.test.ts (6)
26-26: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
GitGuardian false positive: use obviously fake test token to prevent scanner alerts.
The test token
"hvs.testtoken"triggered GitGuardian. While this is a test fixture, the pipeline failure is real. Replace with an obviously fake value like"test-token-not-real"or configure GitGuardian to ignoretests/**/*.test.ts.🧰 Tools
🪛 Betterleaks (1.6.0)
[high] 26-26: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🤖 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 `@tests/core/vault.test.ts` at line 26, The vault test fixture uses a token-like secret_key value that triggers GitGuardian scanning. Update the test data in vault.test.ts to use an obviously fake, non-token-looking placeholder (for example via the secret_key fixture in the relevant test case) so the scanner won’t flag it; if needed, also ensure the test path is covered by a GitGuardian ignore rule for tests/**/*.test.ts.
55-55: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
GitGuardian false positive: use obviously fake test token to prevent scanner alerts.
Same issue as line 26. Replace with obviously fake test value to prevent scanner alerts.
🧰 Tools
🪛 Betterleaks (1.6.0)
[high] 55-55: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🤖 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 `@tests/core/vault.test.ts` at line 55, The vault test still uses a value that can resemble a real secret, triggering scanner alerts. Update the private_key test fixture in vault.test.ts to use an obviously fake, non-production-looking token consistent with the other test constant, and keep it within the vault-related test setup so GitGuardian no longer flags it.
86-90: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Remove redundant try/catch block.
The
rejects.toThrowassertion on line 83-84 already verifies the error type. The subsequent try/catch that manually checks the message is redundant and can be removed.🤖 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 `@tests/core/vault.test.ts` around lines 86 - 90, The test in vault.test.ts should be simplified by removing the redundant try/catch around resolver.getSecret, since the existing rejects.toThrow assertion already covers the failure path. Keep the rejection assertion and delete the manual error capture and message match so the test remains focused and non-duplicative in the relevant vault secret resolution test case.
168-168: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
GitGuardian false positive: use obviously fake test token to prevent scanner alerts.
Same issue. Replace with obviously fake test value.
🧰 Tools
🪛 Betterleaks (1.6.0)
[high] 168-168: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🤖 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 `@tests/core/vault.test.ts` at line 168, The mocked secret value in the vault test is too realistic and triggers GitGuardian scanning, so replace it in the test fixture with an obviously fake token value. Update the stubbed response in the vault test around the json() mock to use a clearly non-sensitive placeholder while keeping the same shape and length expectations if needed.
184-193: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Move
vi.mockto top level; hoisted mocks inside tests cause undefined behavior.
vi.mockis hoisted to the top of the file and evaluated before test execution. Placing it inside anit()block creates confusing module resolution where the mock may apply globally or not at all depending on test ordering. Move it to the top level and usevi.doMockif dynamic mocking is required.🔧 Proposed fix
// We will import after mocking const mockFetch = vi.fn(); vi.stubGlobal("fetch", mockFetch); + +vi.mock("../../src/utils/config.js", () => ({ + loadConfig: () => ({ + network: "testnet", + pollingIntervalSeconds: 300, + vault: { + url: "https://vault.example.com", + token: "hvs.testtoken" + } + }) +})); describe("VaultResolver", () => {Then remove the
vi.mockcall from inside the test.📝 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.🤖 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 `@tests/core/vault.test.ts` around lines 184 - 193, The `vi.mock` call inside the test is unsafe because Vitest hoists mocks, so it should not live in an `it()` block. Move the `vi.mock("../../src/utils/config.js", ...)` setup to the top level of `tests/core/vault.test.ts` near the other test mocks, and remove the in-test mock from the affected case; if the test needs per-case dynamic behavior, switch to `vi.doMock` instead. Use the `loadConfig` mock and the vault config fixture as the identifiers to relocate.
198-198: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
GitGuardian false positive: use obviously fake test token to prevent scanner alerts.
Same issue. Replace with obviously fake test value.
🧰 Tools
🪛 Betterleaks (1.6.0)
[high] 198-198: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🤖 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 `@tests/core/vault.test.ts` at line 198, The test stub in the vault test is still using a token-like string that can trigger GitGuardian, so replace the current secret value in the async json response with an obviously fake non-secret test placeholder. Update the mocked response in the vault test so the unique secret payload is clearly synthetic and not shaped like a real token, keeping the same test flow intact.tests/db/resource_usage_logs.test.ts (1)
241-246: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Make the “future” timestamp relative to the test clock.
This ordering assertion becomes false on or after January 1, 2030, because the default
recorded_atcan then sort after the hard-coded"2030-01-01..."row.🛠️ Suggested change
+ const futureTs = new Date(Date.now() + 24 * 60 * 60 * 1000).toISOString(); + // Explicit timestamp far in the future insertResourceUsageLog(db, { contract_id: CONTRACT_ID, cpu_insns: 300, mem_bytes: 300, - recorded_at: "2030-01-01T00:00:00.000Z", + recorded_at: futureTs, });Also applies to: 251-254
🤖 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 `@tests/db/resource_usage_logs.test.ts` around lines 241 - 246, The ordering test uses a hard-coded future timestamp that will stop being “future” once the default recorded_at catches up, so make the timestamp relative to the test clock instead. Update the inserts in the resource usage log test to derive the “future” recorded_at from the same mocked/fixed time used by the test, and keep the ordering assertions in terms of that relative value. Locate the affected rows in the resource usage log test around the insertResourceUsageLog calls.
✅ Action performedComments resolved and changes approved. |
* feat(db): add resource_usage_logs table with repository functions * fix(db): add type assertions for snapshotRow and currentDayRow in repositories.ts * fix(core): replace empty RentWindowProjection interface with type alias * fix: resolve schema corruption and linting bypasses --------- Co-authored-by: AbdulmalikAlayande <114596864+AbdulmalikAlayande@users.noreply.github.com>
* feat(db): add resource_usage_logs table with repository functions * fix(db): add type assertions for snapshotRow and currentDayRow in repositories.ts * fix(core): replace empty RentWindowProjection interface with type alias * fix: resolve schema corruption and linting bypasses --------- Co-authored-by: AbdulmalikAlayande <114596864+AbdulmalikAlayande@users.noreply.github.com>
2 participants
Closes #164