Skip to content

feat(cli): add per-contract polling interval overrides - #261

Closed
Stephan-Thomas wants to merge 5 commits into
TegoLabs:mainfrom
Stephan-Thomas:feat-poll-interval-overrides
Closed

Stephan-Thomas wants to merge 5 commits into
TegoLabs:mainfrom
Stephan-Thomas:feat-poll-interval-overrides

Conversation

@Stephan-Thomas

Copy link
Copy Markdown
Contributor

Closes #149

@coderabbitai

coderabbitai Bot commented Jun 27, 2026 •

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@Stephan-Thomas, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 54 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 34de6322-57c8-4753-8f50-ca7f27a44e7b

📥 Commits

Reviewing files that changed from the base of the PR and between 3f03ec9 and 9345c62.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (9)
  • src/alerts/keys.test.ts
  • src/core/discovery.ts
  • src/rpc/client.ts
  • tests/commands/costs.test.ts
  • tests/core/monitor.test.ts
  • tests/core/rate_limiter.test.ts
  • tests/db/rate_limiter.test.ts
  • tests/e2e/sandbox-network.test.ts
  • tests/rpc/resource_estimate.test.ts
📝 Walkthrough

Walkthrough

Introduces a centralized LIVE_MIGRATIONS SQL array in src/db/database.ts, replacing the previous inline migrations array used in both getDatabase() and getDatabaseForTesting(). Error handling now rethrows all errors except duplicate-column-name errors, replacing prior broad error suppression.

Changes

Live migration centralization

Layer / File(s) Summary
Centralized migration statements
src/db/database.ts
Adds a shared LIVE_MIGRATIONS array containing schema-evolution SQL, including new columns and channel_accounts table creation plus a contracts column addition.
Production and test migration execution
src/db/database.ts
Updates getDatabase() and getDatabaseForTesting() to iterate LIVE_MIGRATIONS, rethrowing all errors except duplicate-column-name errors, replacing the prior broad no-op error handling.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

  • AbdulmalikAlayande/sorokeep#236: Also modifies src/db/database.ts migration logic, replacing the live-migration block with a Migrator-driven engine, directly overlapping this change area.

Poem

A column hops in, snug and neat,
Duplicate errors? No repeat!
One list to rule the migrate flow,
Both prod and test now hop in row. 🐇
Thump-thump, the schema's safe below!

🚥 Pre-merge checks | ✅ 2 | ❌ 3

❌ Failed checks (3 warnings)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The changed file updates database migrations, not the requested polling-interval override feature from #149. Implement the CLI override, persist it on contracts, and update the daemon loop behavior with tests as required by #149.
Out of Scope Changes check ⚠️ Warning The database migration changes in src/db/database.ts are unrelated to the polling-interval override objective. Remove or justify the migration-only database changes, and keep the PR focused on the per-contract polling override work.
Docstring Coverage ⚠️ Warning Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title matches the intended feature of per-contract polling interval overrides.
Description check ✅ Passed The description is related to the linked issue, even though it is very terse.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@drips-wave

drips-wave Bot commented Jun 27, 2026

Copy link
Copy Markdown

@Stephan-Thomas Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

@gitguardian

gitguardian Bot commented Jun 27, 2026 •

Copy link
Copy Markdown

️✅ There are no secrets present in this pull request anymore.

If these secrets were true positive and are still valid, we highly recommend you to revoke them.
While these secrets were previously flagged, we no longer have a reference to the
specific commits where they were detected. Once a secret has been leaked into a git
repository, you should consider it compromised, even if it was deleted immediately.
Find here more information about risks.


🦉 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 coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
src/core/watch.ts (1)

152-158: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Persist the override before the cache fast-path returns.

When the introspection cache is valid, watchContract exits before this insertContract runs. So watch <id> --poll-interval 300 on an already watched contract reports success but never saves the new override unless the caller also sets forceRefresh.

🤖 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/watch.ts` around lines 152 - 158, Persist the updated poll interval
before the cache fast-path in watchContract, because the current early return
prevents insertContract from running when the introspection cache is valid. Move
the contract persistence logic so it runs before the cached success return, or
otherwise ensure the override is saved regardless of whether forceRefresh is
set. Use watchContract and insertContract as the key symbols to update the flow.
src/daemon/loop.ts (1)

69-80: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

Recompute the daemon interval after overrides change.

effectiveIntervalMs is resolved only once during startup, and the timer is never rescheduled afterward. If a user adds, removes, or edits poll_interval_seconds while the daemon is running, the live loop keeps the old cadence until restart.

Also applies to: 246-257

🤖 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/daemon/loop.ts` around lines 69 - 80, The daemon loop in
scheduledTick/executeCycle is only using resolvePollIntervalMs once at startup,
so changes to poll_interval_seconds are never reflected while the process runs.
Update the timing logic in src/daemon/loop.ts so the active interval is
recalculated after each cycle or when overrides change, and reschedule the
setInterval/timeout based on the new value. Make sure the same fix is applied to
the repeated scheduling path referenced by the Daemon loop so the cadence can
change without restarting.
🤖 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 101-106: The upsert in repositories.ts is preventing
`watchContract` from clearing an existing `poll_interval_seconds` override
because `COALESCE(excluded.poll_interval_seconds,
contracts.poll_interval_seconds)` keeps the old value when `null` is passed.
Update the `ON CONFLICT` handling in the repository write path so an explicit
`null` from `watchContract` actually overwrites the stored value and restores
adaptive polling, while still preserving the existing value only when the field
is omitted. Use the `watchContract` call site and the contracts upsert logic as
the key places to align this behavior.

---

Outside diff comments:
In `@src/core/watch.ts`:
- Around line 152-158: Persist the updated poll interval before the cache
fast-path in watchContract, because the current early return prevents
insertContract from running when the introspection cache is valid. Move the
contract persistence logic so it runs before the cached success return, or
otherwise ensure the override is saved regardless of whether forceRefresh is
set. Use watchContract and insertContract as the key symbols to update the flow.

In `@src/daemon/loop.ts`:
- Around line 69-80: The daemon loop in scheduledTick/executeCycle is only using
resolvePollIntervalMs once at startup, so changes to poll_interval_seconds are
never reflected while the process runs. Update the timing logic in
src/daemon/loop.ts so the active interval is recalculated after each cycle or
when overrides change, and reschedule the setInterval/timeout based on the new
value. Make sure the same fix is applied to the repeated scheduling path
referenced by the Daemon loop so the cadence can change without restarting.
🪄 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: 64940b8b-4368-422b-a3e4-d84fef931f51

📥 Commits

Reviewing files that changed from the base of the PR and between 156d642 and 72d88ab.

📒 Files selected for processing (8)
  • src/core/watch.ts
  • src/daemon/loop.ts
  • src/db/database.ts
  • src/db/repositories.ts
  • src/db/schema.sql
  • tests/core/watch.test.ts
  • tests/daemon/loop.test.ts
  • tests/db/introspection_cache.test.ts
📜 Review details
⚠️ CI failures not shown inline (2)

GitHub Actions: CI Pipeline / build-and-test (22.x): feat(cli): add per-contract polling interval overrides

Conclusion: failure

View job details

2m�[39m
 �[41m�[1m FAIL �[22m�[49m tests/db/alert_delivery.test.ts�[2m > �[22mgetUndeliveredAlerts�[2m > �[22mDelivered filtering�[2m > �[22mdoes not exclude resolved alerts — resolved != delivered
 �[31m�[1mSqliteError�[22m: no such table: contracts�[39m
 �[90m �[2m❯�[22m Database.prepare node_modules/better-sqlite3/lib/methods/wrappers.js:�[2m5:21�[22m�[39m
 �[36m �[2m❯�[22m insertContract src/db/repositories.ts:�[2m98:8�[22m�[39m
     �[90m 96| �[39m// ---------------------------- Database Access Functions For Schema: …
     �[90m 97| �[39mexport function insertContract(db: Database.Database, contract: {id: s…
     �[90m 98| �[39m    db�[33m.�[39m�[34mprepare�[39m(�[32m`
     �[90m   | �[39m       �[31m^�[39m
     �[90m 99| �[39m        INSERT INTO contracts (id, name, network, wasm_hash, tags, pol…
     �[90m100| �[39m        VALUES (`@id`, `@name`, `@network`, `@wasm_hash`, `@tags`, `@poll_interva`…
 �[90m �[2m❯�[22m seedFull tests/db/alert_delivery.test.ts:�[2m41:5�[22m�[39m
 �[90m �[2m❯�[22m tests/db/alert_delivery.test.ts:�[2m245:33�[22m�[39m
 �[31m�[2m⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[204/310]⎯�[22m�[39m
 �[41m�[1m FAIL �[22m�[49m tests/db/alert_delivery.test.ts�[2m > �[22mgetUndeliveredAlerts�[2m > �[22mMultiple alerts per contract/entry�[2m > �[22mreturns one row per alert_fired record, not per contract
 �[31m�[1mSqliteError�[22m: no such table: contracts�[39m
 �[90m �[2m❯�[22m Database.prepare node_modules/better-sqlite3/lib/methods/wrappers.js:�[2m5:21�[22m�[39m
 �[36m �[2m❯�[22m insertContract src/db/repositories.ts:�[2m98:8�[22m�[39m
     �[90m 96| �[39m// ---------------------------- Database Access Functions For Schema: …
     �[90m 97| �[39mexport function insertContract(db: Database.Database, contract: {id: s…
     �[90m 98| �[39m    db�[33m.�[39m�[34mprepare�[39m(�[32m`
     �[90m   | �[39m       �[31m^�[39m
     �[90m 99| �[39m        INSERT INTO contracts (id, name, network, wasm_hash, tags, pol…
     �[90m100| �[39m        VALUES (`@id`, `@name`, `@network`, `@wasm_hash`, ...

GitHub Actions: CI Pipeline / 0_build-and-test (22.x).txt: feat(cli): add per-contract polling interval overrides

Conclusion: failure

View job details

2m�[39m
 �[41m�[1m FAIL �[22m�[49m tests/db/alert_delivery.test.ts�[2m > �[22mgetUndeliveredAlerts�[2m > �[22mDelivered filtering�[2m > �[22mdoes not exclude resolved alerts — resolved != delivered
 �[31m�[1mSqliteError�[22m: no such table: contracts�[39m
 �[90m �[2m❯�[22m Database.prepare node_modules/better-sqlite3/lib/methods/wrappers.js:�[2m5:21�[22m�[39m
 �[36m �[2m❯�[22m insertContract src/db/repositories.ts:�[2m98:8�[22m�[39m
     �[90m 96| �[39m// ---------------------------- Database Access Functions For Schema: …
     �[90m 97| �[39mexport function insertContract(db: Database.Database, contract: {id: s…
     �[90m 98| �[39m    db�[33m.�[39m�[34mprepare�[39m(�[32m`
     �[90m   | �[39m       �[31m^�[39m
     �[90m 99| �[39m        INSERT INTO contracts (id, name, network, wasm_hash, tags, pol…
     �[90m100| �[39m        VALUES (`@id`, `@name`, `@network`, `@wasm_hash`, `@tags`, `@poll_interva`…
 �[90m �[2m❯�[22m seedFull tests/db/alert_delivery.test.ts:�[2m41:5�[22m�[39m
 �[90m �[2m❯�[22m tests/db/alert_delivery.test.ts:�[2m245:33�[22m�[39m
 �[31m�[2m⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[204/310]⎯�[22m�[39m
 �[41m�[1m FAIL �[22m�[49m tests/db/alert_delivery.test.ts�[2m > �[22mgetUndeliveredAlerts�[2m > �[22mMultiple alerts per contract/entry�[2m > �[22mreturns one row per alert_fired record, not per contract
 �[31m�[1mSqliteError�[22m: no such table: contracts�[39m
 �[90m �[2m❯�[22m Database.prepare node_modules/better-sqlite3/lib/methods/wrappers.js:�[2m5:21�[22m�[39m
 �[36m �[2m❯�[22m insertContract src/db/repositories.ts:�[2m98:8�[22m�[39m
     �[90m 96| �[39m// ---------------------------- Database Access Functions For Schema: …
     �[90m 97| �[39mexport function insertContract(db: Database.Database, contract: {id: s…
     �[90m 98| �[39m    db�[33m.�[39m�[34mprepare�[39m(�[32m`
     �[90m   | �[39m       �[31m^�[39m
     �[90m 99| �[39m        INSERT INTO contracts (id, name, network, wasm_hash, tags, pol…
     �[90m100| �[39m        VALUES (`@id`, `@name`, `@network`, `@wasm_hash`, ...
🧰 Additional context used
🪛 GitHub Actions: CI Pipeline / 0_build-and-test (22.x).txt
tests/db/introspection_cache.test.ts

[error] 49-49: Schema assertion failed: expected contracts table_info(PRAGMA table_info(contracts)) column names to include 'last_introspected_at', but received []

src/db/repositories.ts

[error] 98-98: SqliteError: no such table: contracts (failed during insertContract db.prepare INSERT INTO contracts …)


[error] 672-672: SqliteError: no such table: alerts_fired (failed during countUndeliveredAlerts db.prepare SELECT COUNT(*) FROM alerts_fired …)


[error] 495-495: SqliteError: no such table: cost_daily_snapshots (failed during getContractCostSummary db.prepare SELECT … FROM cost_daily_snapshots …)


[error] 556-556: SqliteError: no such table: extension_history (failed during getAverageResourceUsage db.prepare SELECT … FROM extension_history …)


[error] 118-118: SqliteError: no such table: contracts (failed during getContract db.prepare('SELECT * FROM contracts WHERE id = ?').get(id))

🪛 GitHub Actions: CI Pipeline / build-and-test (22.x)
tests/db/introspection_cache.test.ts

[error] 49-51: Assertion failed: expected schema columns to include 'last_introspected_at', but received []. (Underlying DB failure indicates contracts table/schema is missing.)

src/db/repositories.ts

[error] 98-100: SqliteError: no such table: contracts while running insertContract. Attempted query: INSERT INTO contracts (id, name, network, wasm_hash, tags, poll_interval, ...).


[error] 672-672: SqliteError: no such table: alerts_fired while running countUndeliveredAlerts. Attempted query selects COUNT(*) FROM alerts_fired.

🪛 GitHub Check: build-and-test (22.x)
src/db/repositories.ts

[failure] 98-98: tests/alerts/dispatcher.test.ts > deliverPendingAlerts > Delivered flag management > does not re-deliver already-delivered alerts
SqliteError: no such table: contracts
❯ Database.prepare node_modules/better-sqlite3/lib/methods/wrappers.js:5:21
❯ Module.insertContract src/db/repositories.ts:98:8
❯ seedContractWithAlert tests/alerts/dispatcher.test.ts:52:5
❯ tests/alerts/dispatcher.test.ts:253:13

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
Serialized Error: { code: 'SQLITE_ERROR' }


[failure] 98-98: tests/alerts/dispatcher.test.ts > deliverPendingAlerts > Delivered flag management > does NOT mark as delivered when send fails
SqliteError: no such table: contracts
❯ Database.prepare node_modules/better-sqlite3/lib/methods/wrappers.js:5:21
❯ Module.insertContract src/db/repositories.ts:98:8
❯ seedContractWithAlert tests/alerts/dispatcher.test.ts:52:5
❯ tests/alerts/dispatcher.test.ts:241:38

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
Serialized Error: { code: 'SQLITE_ERROR' }


[failure] 98-98: tests/alerts/dispatcher.test.ts > deliverPendingAlerts > Delivered flag management > marks the alert as delivered in the DB after successful send
SqliteError: no such table: contracts
❯ Database.prepare node_modules/better-sqlite3/lib/methods/wrappers.js:5:21
❯ Module.insertContract src/db/repositories.ts:98:8
❯ seedContractWithAlert tests/alerts/dispatcher.test.ts:52:5
❯ tests/alerts/dispatcher.test.ts:229:38

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
Serialized Error: { code: 'SQLITE_ERROR' }


[failure] 98-98: tests/alerts/dispatcher.test.ts > deliverPendingAlerts > Channel routing > calls sendSlackAlert with the correct channel and event
SqliteError: no such table: contracts
❯ Database.prepare node_modules/better-sqlite3/lib/methods/wrappers.js:5:21
❯ Module.insertContract src/db/repositories.ts:98:8
❯ seedContractWithAlert tests/alerts/dispatcher.test.ts:52:5
❯ tests/alerts/dispatcher.test.ts:209:13

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
Serialized Error: { code: 'SQLITE_ERROR' }


[failure] 98-98: tests/alerts/dispatcher.test.ts > deliverPendingAlerts > Channel routing > calls sendWebhookAlert with the correct URL, event payload, and secret
SqliteError: no such table: contracts
❯ Database.prepare node_modules/better-sqlite3/lib/methods/wrappers.js:5:21
❯ Module.insertContract src/db/repositories.ts:98:8
❯ seedContractWithAlert tests/alerts/dispatcher.test.ts:52:5
❯ tests/alerts/dispatcher.test.ts:181:13

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
Serialized Error: { code: 'SQLITE_ERROR' }


[failure] 98-98: tests/alerts/dispatcher.test.ts > deliverPendingAlerts > Channel routing > routes pagerduty alerts to sendPagerDutyAlert
SqliteError: no such table: contracts
❯ Database.prepare node_modules/better-sqlite3/lib/methods/wrappers.js:5:21
❯ Module.insertContract src/db/repositories.ts:98:8
❯ seedContractWithAlert tests/alerts/dispatcher.test.ts:52:5
❯ tests/alerts/dispatcher.test.ts:166:13

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
Serialized Error: { code: 'SQLITE_ERROR' }


[failure] 98-98: tests/alerts/dispatcher.test.ts > deliverPendingAlerts > Channel routing > routes slack alerts to sendSlackAlert
SqliteError: no such table: contracts
❯ Database.prepare node_modules/better-sqlite3/lib/methods/wrappers.js:5:21
❯ Module.insertContract src/db/repositories.ts:98:8
❯ seedContractWithAlert tests/alerts/dispatcher.test.ts:52:5
❯ tests/alerts/dispatcher.test.ts:151:13

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
Serialized Error: { code: 'SQLITE_ERROR' }


[failure] 98-98: tests/alerts/dispatcher.test.ts > deliverPendingAlerts > Channel routing > routes webhook alerts to sendWebhookAlert
SqliteError: no such table: contracts
❯ Database.prepare node_modules/better-sqlite3/lib/methods/wrappers.js:5:21
❯ Module.insertContract src/db/repositories.ts:98:8
❯ seedContractWithAlert tests/alerts/dispatcher.test.ts:52:5
❯ tests/alerts/dispatcher.test.ts:137:13

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
Serialized Error: { code: 'SQLITE_ERROR' }

Comment thread src/db/repositories.ts
@AbdulmalikAlayande

Copy link
Copy Markdown
Collaborator

Hi @Stephan-Thomas, the CI is failing because \src/db/schema.sql\ was accidentally emptied out (0 bytes) in your branch, probably due to encoding corruption or an accidental overwrite.
This causes the tests to fail with
o such table: contracts.
Please restore \src/db/schema.sql\ from the \main\ branch, append your \poll_interval_seconds INTEGER\ column to the \contracts\ table definition, and push the fix!

@Stephan-Thomas

Stephan-Thomas commented Jun 30, 2026 •

Copy link
Copy Markdown
Contributor Author

Done @AbdulmalikAlayande

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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/schema.sql`:
- Around line 7-11: Remove the duplicated poll_interval_seconds column from the
contracts table definition in the schema.sql create statement. Keep only the
existing poll_interval_seconds field already present in the table schema and
ensure the CREATE TABLE contracts block has unique column names throughout.
Verify the schema initialization path that uses contracts no longer triggers a
duplicate column name error.
🪄 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: b5d15ab9-88f2-47b5-855a-38e57afa98b9

📥 Commits

Reviewing files that changed from the base of the PR and between 6ba24cf and f984c1a.

📒 Files selected for processing (1)
  • src/db/schema.sql
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
  • GitHub Check: build-and-test (22.x)
🧰 Additional context used
🪛 SQLFluff (4.2.2)
src/db/schema.sql

[error] 171-172: Too many consecutive blank lines.

(LT15)

Comment thread src/db/schema.sql Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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/database.ts`:
- Around line 68-79: The LIVE_MIGRATIONS try/catch loop is duplicated in
getDatabase and getDatabaseForTesting, so extract the shared migration runner
into a helper such as applyLiveMigrations(db) in src/db/database.ts and call it
from both functions. Move the existing duplicate-column filtering and
db.exec(sql) execution into that helper so the error-handling logic lives in one
place and stays consistent if it changes later.
🪄 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: d144e54f-1413-4f8a-ad7a-7e8ca654ba57

📥 Commits

Reviewing files that changed from the base of the PR and between f984c1a and 3f03ec9.

📒 Files selected for processing (1)
  • src/db/database.ts
📜 Review details
⚠️ CI failures not shown inline (2)

GitHub Actions: CI Pipeline / 0_build-and-test (22.x).txt: feat(cli): add per-contract polling interval overrides

Conclusion: failure

View job details

##[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/keys.test.ts
 ##[warning]   7:10  warning  'KeychainStore' is defined but never used  `@typescript-eslint/no-unused-vars`
 ##[warning]  30:58  warning  'service' is defined but never used        `@typescript-eslint/no-unused-vars`
 /home/runner/work/sorokeep/sorokeep/src/core/discovery.ts
 ##[warning]  268:10  warning  'buildContractDataKey' is defined but never used  `@typescript-eslint/no-unused-vars`
 /home/runner/work/sorokeep/sorokeep/src/rpc/client.ts
 ##[warning]   11:5   warning  'FeeBumpTransaction' is defined but never used  `@typescript-eslint/no-unused-vars`
 ##[warning]   15:10  warning  'CostSummary' is defined but never used         `@typescript-eslint/no-unused-vars`
 ##[error]  797:11  error    Duplicate name 'submitRestore'                  no-dupe-class-members

GitHub Actions: CI Pipeline / build-and-test (22.x): feat(cli): add per-contract polling interval overrides

Conclusion: failure

View job details

##[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/keys.test.ts
 ##[warning]   7:10  warning  'KeychainStore' is defined but never used  `@typescript-eslint/no-unused-vars`
 ##[warning]  30:58  warning  'service' is defined but never used        `@typescript-eslint/no-unused-vars`
 /home/runner/work/sorokeep/sorokeep/src/core/discovery.ts
 ##[warning]  268:10  warning  'buildContractDataKey' is defined but never used  `@typescript-eslint/no-unused-vars`
 /home/runner/work/sorokeep/sorokeep/src/rpc/client.ts
 ##[warning]   11:5   warning  'FeeBumpTransaction' is defined but never used  `@typescript-eslint/no-unused-vars`
 ##[warning]   15:10  warning  'CostSummary' is defined but never used         `@typescript-eslint/no-unused-vars`
 ##[error]  797:11  error    Duplicate name 'submitRestore'                  no-dupe-class-members
🧰 Additional context used
🪛 OpenGrep (1.23.0)
src/db/database.ts

[ERROR] 70-70: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.

(coderabbit.command-injection.exec-js)


[ERROR] 160-160: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.

(coderabbit.command-injection.exec-js)

🔇 Additional comments (3)
src/db/database.ts (3)

70-70: Static analysis false positive: not child_process.exec.

The OpenGrep hint about "dynamic command passed to child_process.exec" is a false positive — db.exec(sql) here is better-sqlite3's Database.prototype.exec, not Node's child_process.exec. No command injection risk since this executes fixed SQL strings against a local SQLite connection, not shell commands.

Also applies to: 160-160


28-46: Centralized LIVE_MIGRATIONS matches PR objectives.

The poll_interval_seconds column addition (Line 33) correctly implements the schema change required by the linked issue, and the channel_accounts table creation preserves prior behavior.


71-78: 🩺 Stability & Availability

No change needed The live migrations already correspond to schema.sql, and getDatabaseForTesting() uses the same migration set.

Comment thread src/db/database.ts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(cli): add per-contract polling interval overrides

2 participants