docs: reconstruct CHANGELOG.md from git tag history - #565
Closed
OluRemiFour wants to merge 912 commits into
Closed
OluRemiFour wants to merge 912 commits into
OluRemiFour wants to merge 912 commits into
Conversation
…onsorship feature
# Conflicts: # src/core/extension.ts # src/rpc/client.ts # tests/core/extension.test.ts
Extracts CLI construction into createProgram() (src/cli/program.ts) so scripts/generate-man.ts can introspect the Command tree without running the CLI, generates man/sorokeep.1, and wires build:man into npm run build. Per TegoLabs#384. Trimmed from the original PR before merging: - A full set of unrelated ".claude/agents/kfc/*", ".claude/settings/ kfc-settings.json", and ".claude/system-prompts/spec-workflow- starter.md" files - generic AI-agent workflow scaffolding with no connection to sorokeep or this issue. - getContractsByTag() and related changes to src/commands/status.ts / src/db/repositories.ts - that's issue TegoLabs#379's scope (--tag filtering), a different, unrelated feature. The PR's branch was also quite stale (predated the quiet-hours, contract_groups, rpc/client.ts any-type, and CI audit-scope fixes already on main), which produced a large raw diff and one real merge conflict in src/index.ts (this PR's createProgram() refactor vs. the already-merged --extension-jitter-ms option). Resolved by moving the jitter option registration into createProgram() so both are preserved.
…s#548) Relocates the misplaced test to tests/alerts/ (outside vitest.config.ts's include glob otherwise) and rewrites it against the current SlackChannel class / sendPagerDutyAlert function API - the old copy called a removed sendSlackAlert function.
…ted) Adds src/alerts/matrix.ts (real Matrix Client-Server API, token auth, room-scoped delivery) and its tests, per TegoLabs#310. The PR never registered the channel in builtins.ts, so `--type matrix` was unreachable from the CLI despite the sender being fully implemented and tested. Completed the registration (targetOption: "channel", since the sender's target is a Matrix room ID) plus the matching builtins.test.ts coverage, following the exact pattern used by every other lazily-imported channel (discord/telegram/opsgenie). Verified `alerts add --type matrix` end-to-end at the CLI.
… fixed + completed) Adds src/alerts/teams.ts (Adaptive Card payload, severity coloring per event type) and its tests, per TegoLabs#311. Two things fixed before merging: - CodeRabbit correctly flagged validateWebhookUrl()'s hostname check: `hostname.includes("webhook.office.com")` accepts any hostname that merely contains that substring, e.g. an attacker-controlled "x.webhook.office.com.evil.com" would pass, silently sending real alert content (contract IDs, TTL data) to an attacker-controlled server. Changed to an exact/suffix match on the real hostname, plus an explicit https-only check. Added regression tests for both. - The PR never registered the channel in builtins.ts, so `--type teams` was unreachable from the CLI. Completed the registration (targetOption: "url", matching webhook/discord's pattern) and the matching builtins.test.ts coverage.
…#585, fixed + completed) Adds src/alerts/email.ts (nodemailer SMTP transport, env/config token resolution, password redaction on error) and its tests, per TegoLabs#312. Fixed before merging: - Bumped nodemailer ^7.0.7 -> ^9.0.3 (major version, verified tsc still compiles clean and all tests pass unchanged): the pinned 7.x range had six high-severity advisories, including SMTP/CRLF command injection and an SSRF via the raw-message option. This is a production dependency, so `npm audit --omit=dev` would have failed on it. - The channel was registered in builtins.ts, but src/commands/alerts.ts still had a special-cased `--type email` branch printing "Email alerting is not yet implemented" *before* the registry was ever consulted - making the new channel completely unreachable from the CLI. Removed the special case. - Updated the one existing test that asserted the old "not implemented" behavior, and added a real success-path test (registers a config with --channel <email>). - Fixed a lint error (preserve-caught-error) the right way: the error handler already redacts the SMTP password from the thrown message, but attaching the raw caught error as `cause` would have smuggled the unredacted password back in via the cause chain. Redact the caught error's own message in place before using it as cause, so the guarantee holds through the whole chain.
…s#597, trimmed to scope) Adds src/alerts/googlechat.ts and registers it in builtins.ts, per TegoLabs#313. Registration and the sender itself were already correct. Trimmed from the original PR before merging: a bundled Grafana/Prometheus observability stack (devops/grafana/*, devops/prometheus/*, docker-compose. observability.yml, docs/observability.md) - unrelated to this issue, shared branch lineage with several other open PRs (TegoLabs#594, TegoLabs#595, TegoLabs#596) that also carry the identical bundle. Left tests/docker/docker-compose.test.ts untouched by reverting to main's version. Updated tests/alerts/builtins.test.ts for the 10th channel (the PR's branch predated matrix/teams/email, so its own copy of this file didn't know about them). Note for a follow-up: src/alerts/discord.ts has the same hostname- validation weakness fixed in TegoLabs#557 (`hostname.includes("discord")`, even looser than Teams's check) - pre-existing, not part of this PR, flagging separately.
…egoLabs#318) PR TegoLabs#568 referenced ChannelDefinition.maxRetries in dispatcher.ts but never added the field to the interface, so the branch failed to compile (TS2339). It also shipped a new dispatcher test that called registerAlertChannel("webhook", { maxRetries: 2 }) — a two-argument overload that doesn't exist on the real one-argument, throw-on-duplicate registerAlertChannel API, so the test could never have run against this codebase. - Add optional maxRetries/retryBackoffMs to ChannelDefinition (registry.ts); dispatcher.ts's lookup already existed from the merge and needed no further changes. - Give telegram a maxRetries: 3 default, matching the issue's own rationale (Telegram's Bot API rate limits are stricter than a generic webhook's) — every other channel keeps the global MAX_RETRY_COUNT default, preserving existing behavior exactly. - Rewrite the broken test to swap in a real ChannelDefinition override via the actual registry API, and restore the registry to normal built-in state afterward so it doesn't leak into other tests. Added a companion test asserting webhook has no override by default. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
# Conflicts: # package-lock.json
…nnels' into verify-555
…egoLabs#555) Issue TegoLabs#319's scope explicitly excludes the orphaned src/alerts/*.test.ts files ("reconciling those is phase-8's job, not this issue's") — they aren't in vitest.config.ts's include glob (tests/**/*.test.ts), so any tests added there never actually run. Keeping the docs update and the contract suite applied to webhook/slack, whose real test files live under tests/alerts/. Applying the contract suite to pagerduty/discord/ telegram is deferred to their dedicated orphan-reconciliation issues (TegoLabs#354-TegoLabs#356).
…to-preview-payload-without-sending-FIX' of https://github.com/veloura-dev/sorokeep into verify-582
…b/sorokeep into verify-536 # Conflicts: # src/commands/alerts.ts
…yStats (TegoLabs#536) The PR's src/db/repositories.ts shipped literal backslash-escaped backticks (\`...\`) instead of real template literals in the days-filter branch of getChannelDeliveryStats — invalid TypeScript syntax that made the entire file fail to parse (tsc reported ~30 cascading errors past this point, and CI's build-and-test check was correctly failing). Replaced with proper template literals. Also tightened the query params array from any[] to Array<number | string>. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
package.json (1)
14-14: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winKeep the
manasset in the npm package.Adding
CHANGELOG.mdmust not removeman. The build still generatesman/sorokeep.1, andREADME.mdinstructs users to copy that file after installation. AddCHANGELOG.mdalongsidemanin thefilesarray.🤖 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 `@package.json` at line 14, Update the package.json files array to retain the existing man asset and add CHANGELOG.md alongside it, ensuring the generated man/sorokeep.1 remains included in the npm package.
🤖 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.
Outside diff comments:
In `@package.json`:
- Line 14: Update the package.json files array to retain the existing man asset
and add CHANGELOG.md alongside it, ensuring the generated man/sorokeep.1 remains
included in the npm package.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 47b9584d-3ce6-4fb1-9c83-539bab144d89
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (2)
README.mdpackage.json
📜 Review details
⚠️ CI failures not shown inline (4)
GitHub Actions: CI Pipeline / build-and-test (22.x): docs: reconstruct CHANGELOG.md from git tag history
Conclusion: failure
##[group]Run npm run lint
�[36;1mnpm run lint�[0m
shell: /usr/bin/bash -e {0}
##[endgroup]
> sorokeep@1.0.0 lint
> eslint src/ tests/
/home/runner/work/sorokeep/sorokeep/src/alerts/alerts.test.ts
##[warning] 41:25 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
##[warning] 42:27 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
/home/runner/work/sorokeep/sorokeep/src/alerts/discord.ts
##[warning] 189:18 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
/home/runner/work/sorokeep/sorokeep/src/alerts/instance_scan.test.ts
##[warning] 9:17 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
##[warning] 14:15 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
##[warning] 83:15 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
/home/runner/work/sorokeep/sorokeep/src/alerts/keys.ts
##[warning] 11:19 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
##[warning] 13:28 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
/home/runner/work/sorokeep/sorokeep/src/alerts/pagerduty.ts
##[warning] 123:48 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
/home/runner/work/sorokeep/sorokeep/src/alerts/resource.test.ts
##[warning] 279:38 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
##[warning] 578:112 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
##[warning] 613:123 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
##[warning] 624:119 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
/home/runner/work/sorokeep/sorokeep/src/alerts/slack.ts
##[warning] 15:42 warn...
GitHub Actions: CI Pipeline / 0_build-and-test (24.x).txt: docs: reconstruct CHANGELOG.md from git tag history
Conclusion: failure
##[group]Run npm run lint
�[36;1mnpm run lint�[0m
shell: /usr/bin/bash -e {0}
##[endgroup]
> sorokeep@1.0.0 lint
> eslint src/ tests/
/home/runner/work/sorokeep/sorokeep/src/alerts/alerts.test.ts
##[warning] 41:25 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
##[warning] 42:27 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
/home/runner/work/sorokeep/sorokeep/src/alerts/discord.ts
##[warning] 189:18 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
/home/runner/work/sorokeep/sorokeep/src/alerts/instance_scan.test.ts
##[warning] 9:17 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
##[warning] 14:15 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
##[warning] 83:15 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
/home/runner/work/sorokeep/sorokeep/src/alerts/keys.ts
##[warning] 11:19 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
##[warning] 13:28 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
/home/runner/work/sorokeep/sorokeep/src/alerts/pagerduty.ts
##[warning] 123:48 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
/home/runner/work/sorokeep/sorokeep/src/alerts/resource.test.ts
##[warning] 279:38 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
##[warning] 578:112 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
##[warning] 613:123 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
##[warning] 624:119 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
/home/runner/work/sorokeep/sorokeep/src/alerts/slack.ts
##[warning] 15:42 warn...
GitHub Actions: CI Pipeline / 1_build-and-test (22.x).txt: docs: reconstruct CHANGELOG.md from git tag history
Conclusion: failure
##[group]Run npm run lint
�[36;1mnpm run lint�[0m
shell: /usr/bin/bash -e {0}
##[endgroup]
> sorokeep@1.0.0 lint
> eslint src/ tests/
/home/runner/work/sorokeep/sorokeep/src/alerts/alerts.test.ts
##[warning] 41:25 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
##[warning] 42:27 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
/home/runner/work/sorokeep/sorokeep/src/alerts/discord.ts
##[warning] 189:18 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
/home/runner/work/sorokeep/sorokeep/src/alerts/instance_scan.test.ts
##[warning] 9:17 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
##[warning] 14:15 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
##[warning] 83:15 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
/home/runner/work/sorokeep/sorokeep/src/alerts/keys.ts
##[warning] 11:19 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
##[warning] 13:28 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
/home/runner/work/sorokeep/sorokeep/src/alerts/pagerduty.ts
##[warning] 123:48 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
/home/runner/work/sorokeep/sorokeep/src/alerts/resource.test.ts
##[warning] 279:38 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
##[warning] 578:112 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
##[warning] 613:123 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
##[warning] 624:119 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
/home/runner/work/sorokeep/sorokeep/src/alerts/slack.ts
##[warning] 15:42 warn...
GitHub Actions: CI Pipeline / build-and-test (24.x): docs: reconstruct CHANGELOG.md from git tag history
Conclusion: failure
##[group]Run npm run lint
�[36;1mnpm run lint�[0m
shell: /usr/bin/bash -e {0}
##[endgroup]
> sorokeep@1.0.0 lint
> eslint src/ tests/
/home/runner/work/sorokeep/sorokeep/src/alerts/alerts.test.ts
##[warning] 41:25 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
##[warning] 42:27 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
/home/runner/work/sorokeep/sorokeep/src/alerts/discord.ts
##[warning] 189:18 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
/home/runner/work/sorokeep/sorokeep/src/alerts/instance_scan.test.ts
##[warning] 9:17 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
##[warning] 14:15 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
##[warning] 83:15 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
/home/runner/work/sorokeep/sorokeep/src/alerts/keys.ts
##[warning] 11:19 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
##[warning] 13:28 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
/home/runner/work/sorokeep/sorokeep/src/alerts/pagerduty.ts
##[warning] 123:48 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
/home/runner/work/sorokeep/sorokeep/src/alerts/resource.test.ts
##[warning] 279:38 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
##[warning] 578:112 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
##[warning] 613:123 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
##[warning] 624:119 warning Unexpected any. Specify a different type `@typescript-eslint/no-explicit-any`
/home/runner/work/sorokeep/sorokeep/src/alerts/slack.ts
##[warning] 15:42 warn...
🔇 Additional comments (1)
README.md (1)
21-22: LGTM!
AbdulmalikAlayande
force-pushed
the
main
branch
from
August 2, 2026 04:34
43ad363 to
8692701
Compare
Contributor
Author
|
@AbdulmalikAlayande |
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (13)
src/daemon/loop.ts (4)
201-203: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAlign skipped-cycle behavior with the metric contract.
This early return no longer increments
daemonCyclesSkipped. The test at Lines 489-507 expects an increment. Restore the increment if the metric remains, or remove the metric and test with the metrics removal.🤖 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 201 - 203, Update the cycle-in-flight branch in the daemon loop to increment daemonCyclesSkipped before returning, preserving the metric contract and the existing skip log behavior; alternatively remove the metric and its associated test if that metric is intentionally being eliminated.
30-31: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRemove the ignored
metricsPortoption.Metrics-server startup was removed, but
DaemonOptionsstill acceptsmetricsPortandstartDaemondoes not use it. Callers can provide this option without receiving metrics. Remove the field or restore the metrics-server contract.🤖 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 30 - 31, Remove the unused metricsPort field from the DaemonOptions definition and update startDaemon-related callers or type usage that reference it, ensuring the daemon no longer accepts an option that has no effect.
172-181: 🚀 Performance & Scalability | 🟠 Major | ⚡ Quick winRun cost aggregation once per cycle.
The existing guarded call at Lines 166-171 already runs
aggregateDailyCostSnapshots. This block runs the same seven-day query and upsert transaction again on every cycle. Keep one call. ClosedeliverSpanbefore cost aggregation so each span covers its own step.🤖 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 172 - 181, Remove the duplicate aggregateDailyCostSnapshots call and its surrounding CostAggregation span/try-catch block, retaining the existing guarded invocation earlier in the cycle. Ensure deliverSpan is ended before the retained cost-aggregation step begins so each span covers only its own operation.
123-131: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winComplete the observability import cleanup consistently.
The change removes observability imports but retains tracing and metric references in production and test code. Restore the required imports and package declarations, or remove every dependent call and test.
- src/daemon/loop.ts#L123-L131: restore the tracing/OpenTelemetry imports or remove the tracing setup.
- src/daemon/loop.ts#L144-L147: restore
daemonCycleDurationor remove the observation.- tests/daemon/loop.test.ts#L82-L83: restore the metrics import or delete the resets and metric 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 `@src/daemon/loop.ts` around lines 123 - 131, Complete the observability import cleanup across all affected sites: in src/daemon/loop.ts lines 123-131, restore the imports required by initTracing, getTracer, Span, trace, and context or remove the tracing setup; in src/daemon/loop.ts lines 144-147, restore daemonCycleDuration or remove its observation; in tests/daemon/loop.test.ts lines 82-83, restore the metrics import used by the resets and metric suite or delete those dependent tests.src/core/monitor.ts (4)
104-113: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winEnd
contractSpanon both success and failure.The current code calls
endSpan(contractSpan, error)only in thecatchblock. A successfulprocessContractcall leaves its span open. The daemon creates one span per contract per cycle, so successful cycles accumulate unfinished spans.Move span closure to
finallyand preserve the caught error separately.Proposed lifecycle structure
+ let spanError: unknown; try { await processContract(db, client, contract.id, network, result); } catch (error: unknown) { + spanError = error; // Fault isolation: record the failure, move to next contract. const message = error instanceof Error ? error.message : String(error); const errorEntry = `Error processing contract ${contract.id}: ${message}`; result.errors.push(errorEntry); logger.error(errorEntry, error); - endSpan(contractSpan, error); + } finally { + endSpan(contractSpan, spanError); }🤖 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/monitor.ts` around lines 104 - 113, Update the contract-processing flow around processContract so contractSpan is always closed in a finally block after both successful and failed processing. Preserve the caught error separately for failure logging and pass it to endSpan when present, while retaining the existing error recording and continuation behavior.
307-317: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winUse the fired target when sending resolution notifications.
alertConfighas one primarychannel_type/channel_target, butallTargetscreates onealerts_firedrow per target.resolveAlerts()returns all unresolved fired rows, and the loop sendsdeliverSingleAlert()from the primaryalert_configsdestination each time, so secondary targets never receive their resolution. Use thechannelType/channelTargetfrom each pending fired entry, or stop sending secondary-target resolutions intentionally.🤖 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/monitor.ts` around lines 307 - 317, Update the resolution-notification loop in resolveAlerts() to call deliverSingleAlert() with each pending fired entry’s channelType and channelTarget rather than the primary alertConfig destination, preserving one delivery per alerts_fired row. In src/core/monitor.ts lines 307-317, use the fired entry fields; in src/db/repositories.ts lines 816-817, ensure resolveAlerts() selects and returns channelType and channelTarget for every unresolved fired entry.
251-266: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftMake multi-target alert creation atomic.
recordAlertFiredruns once per target, buthasUnresolvedAlertonly checks whether any unresolved row exists for the alert configuration and entry. If one insert succeeds and a later insert fails, later cycles can skip the remaining targets because the first unresolved row masks the failure. Wrap the complete target batch in one repository transaction, or make retry detection target-aware. Add a regression test for a failure during the second insert.🤖 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/monitor.ts` around lines 251 - 266, Make the complete target batch in the monitor flow atomic: wrap all recordAlertFired calls for allTargets in a single repository transaction so a failure on any target rolls back earlier inserts and allows the batch to retry. Preserve the existing target ordering and fields, and add a regression test covering failure during the second insert.
19-20: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winDeclare
@opentelemetry/apias a direct production dependency.
src/observability/tracing.tsimportsSpanandSpanStatusCodefrom@opentelemetry/api, andmonitor.tsimports those types fromtracing.js. Add it todependenciesand pin a compatible version.🤖 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/monitor.ts` around lines 19 - 20, Declare `@opentelemetry/api` as a direct production dependency in the package manifest, using a pinned version compatible with the existing imports in tracing.ts and monitor.ts.src/db/repositories.ts (1)
445-445: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winRemove the orphaned audit-log import from
src/core/audit_log.ts.
exportAuditLogcannot importgetAuditLogExtensionsif it is removed fromsrc/db/repositories.js; the caller will fail to compile. Keep a replacement/compatibility export, remove the consumer if the feature is dropped, or update this import path.🤖 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` at line 445, Resolve the broken audit-log dependency by updating the consumer of getAuditLogExtensions in exportAuditLog: either retain a compatible export from the repositories module, remove the consumer if the feature is no longer needed, or change the import to the replacement symbol/module. Ensure the audit_log.ts import and exportAuditLog compile successfully.tests/core/monitor.test.ts (2)
1147-1147: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winRetain equivalent tracing coverage for
process-contract.
runMonitorCyclestill starts aprocess-contractspan for each contract and closes it viaendSpanon success or error. The tests no longer cover these paths, while the existing daemon-level span tests do not cover individual contract spans. Keep equivalent coverage for both successful and failed contract processing, including a success-path assertion that detects an unclosedprocess-contractspan.🤖 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/monitor.test.ts` at line 1147, Restore tests around runMonitorCycle that verify each contract creates a process-contract span and calls endSpan on both successful and failed processing; include a success-path assertion that fails when the process-contract span remains open, while preserving the existing daemon-level span coverage.
377-377: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winRestore the
setAlertConfigEnabledimport.Both tests call
setAlertConfigEnabled, but its repository import was removed. The test file will fail with an unresolved identifier.Re-add the import from
src/db/repositories.ts.Also applies to: 400-400
🤖 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/monitor.test.ts` at line 377, Restore the setAlertConfigEnabled import from src/db/repositories.ts in tests/core/monitor.test.ts so both tests can resolve and call the repository function.tests/daemon/loop.test.ts (1)
489-507: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRemove or restore the skipped-cycle assertion.
scheduledTickno longer incrementsdaemonCyclesSkipped, soafterwill not equalbefore + 1. Align this test with the implementation.🤖 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/daemon/loop.test.ts` around lines 489 - 507, Update the test “increments the skipped-cycles counter when a tick is skipped due to an in-flight cycle” to match scheduledTick’s current behavior: remove the skipped-cycle counter setup and before/after assertion, or restore the corresponding implementation increment if that behavior remains required. Keep the test aligned with the chosen implementation contract.src/commands/daemon.ts (1)
60-62: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winFinish the metrics removal as one contract change.
The command, daemon options, scheduled tick, and test assertion do not agree on whether metrics exist.
- src/commands/daemon.ts#L60-L62: remove the stale
metricsPortstatus block.- src/daemon/loop.ts#L30-L31: remove
metricsPortfromDaemonOptions, or restore metrics-server support.- src/daemon/loop.ts#L201-L203: restore
daemonCyclesSkipped.inc(), or remove the metric and related tests.- tests/daemon/loop.test.ts#L489-L507: remove or update the skipped-cycle metric assertion.
🤖 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/commands/daemon.ts` around lines 60 - 62, Complete the metrics removal consistently: in src/commands/daemon.ts lines 60-62 remove the stale metricsPort status block; in src/daemon/loop.ts lines 30-31 remove metricsPort from DaemonOptions; in src/daemon/loop.ts lines 201-203 remove daemonCyclesSkipped and its related metric handling; and in tests/daemon/loop.test.ts lines 489-507 remove or update the skipped-cycle metric assertion to match the resulting contract.
🤖 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 `@README.md`:
- Around line 130-133: Clean up the watch example in the README by removing the
duplicated description and invocation near the existing watch documentation.
Keep a single fenced block for the command output, ensuring the fence remains
open through the output and uses the appropriate text fence instead of closing
prematurely.
In `@src/core/monitor.ts`:
- Around line 139-160: Remove the later duplicate runAutoExtensions invocation
and its associated result assignments, logging, anomaly iteration, and catch
block. Keep the preceding runAutoExtensions phase as the sole invocation with
the existing arguments and result handling.
In `@src/daemon/loop.ts`:
- Line 134: Restore the Monitor child span around the runMonitorCycle call in
src/daemon/loop.ts lines 134-134, and record any monitor error on that span
while preserving existing cycle behavior. Retain the Monitor span and
error-status assertions in tests/daemon/loop.test.ts lines 531-563; no direct
test change is required beyond ensuring they pass with the restored tracing
contract.
- Around line 259-270: Remove the duplicate resolvePollIntervalMs definition in
src/daemon/loop.ts, retaining the existing single implementation and its
behavior for contract overrides and fallback intervals.
- Around line 259-263: Update resolvePollIntervalMs to filter contracts by both
matching network and contract.active === 1 before mapping poll_interval_seconds,
so its interval overrides match the active contracts monitored by
runMonitorCycle.
In `@src/db/repositories.ts`:
- Line 55: Restore the enabled boolean member on the public AlertConfig type in
repositories.ts, including its original SQLite boolean documentation, so the
existing monitor.ts reads and setAlertConfigEnabled writes remain type-safe.
---
Outside diff comments:
In `@src/commands/daemon.ts`:
- Around line 60-62: Complete the metrics removal consistently: in
src/commands/daemon.ts lines 60-62 remove the stale metricsPort status block; in
src/daemon/loop.ts lines 30-31 remove metricsPort from DaemonOptions; in
src/daemon/loop.ts lines 201-203 remove daemonCyclesSkipped and its related
metric handling; and in tests/daemon/loop.test.ts lines 489-507 remove or update
the skipped-cycle metric assertion to match the resulting contract.
In `@src/core/monitor.ts`:
- Around line 104-113: Update the contract-processing flow around
processContract so contractSpan is always closed in a finally block after both
successful and failed processing. Preserve the caught error separately for
failure logging and pass it to endSpan when present, while retaining the
existing error recording and continuation behavior.
- Around line 307-317: Update the resolution-notification loop in
resolveAlerts() to call deliverSingleAlert() with each pending fired entry’s
channelType and channelTarget rather than the primary alertConfig destination,
preserving one delivery per alerts_fired row. In src/core/monitor.ts lines
307-317, use the fired entry fields; in src/db/repositories.ts lines 816-817,
ensure resolveAlerts() selects and returns channelType and channelTarget for
every unresolved fired entry.
- Around line 251-266: Make the complete target batch in the monitor flow
atomic: wrap all recordAlertFired calls for allTargets in a single repository
transaction so a failure on any target rolls back earlier inserts and allows the
batch to retry. Preserve the existing target ordering and fields, and add a
regression test covering failure during the second insert.
- Around line 19-20: Declare `@opentelemetry/api` as a direct production
dependency in the package manifest, using a pinned version compatible with the
existing imports in tracing.ts and monitor.ts.
In `@src/daemon/loop.ts`:
- Around line 201-203: Update the cycle-in-flight branch in the daemon loop to
increment daemonCyclesSkipped before returning, preserving the metric contract
and the existing skip log behavior; alternatively remove the metric and its
associated test if that metric is intentionally being eliminated.
- Around line 30-31: Remove the unused metricsPort field from the DaemonOptions
definition and update startDaemon-related callers or type usage that reference
it, ensuring the daemon no longer accepts an option that has no effect.
- Around line 172-181: Remove the duplicate aggregateDailyCostSnapshots call and
its surrounding CostAggregation span/try-catch block, retaining the existing
guarded invocation earlier in the cycle. Ensure deliverSpan is ended before the
retained cost-aggregation step begins so each span covers only its own
operation.
- Around line 123-131: Complete the observability import cleanup across all
affected sites: in src/daemon/loop.ts lines 123-131, restore the imports
required by initTracing, getTracer, Span, trace, and context or remove the
tracing setup; in src/daemon/loop.ts lines 144-147, restore daemonCycleDuration
or remove its observation; in tests/daemon/loop.test.ts lines 82-83, restore the
metrics import used by the resets and metric suite or delete those dependent
tests.
In `@src/db/repositories.ts`:
- Line 445: Resolve the broken audit-log dependency by updating the consumer of
getAuditLogExtensions in exportAuditLog: either retain a compatible export from
the repositories module, remove the consumer if the feature is no longer needed,
or change the import to the replacement symbol/module. Ensure the audit_log.ts
import and exportAuditLog compile successfully.
In `@tests/core/monitor.test.ts`:
- Line 1147: Restore tests around runMonitorCycle that verify each contract
creates a process-contract span and calls endSpan on both successful and failed
processing; include a success-path assertion that fails when the
process-contract span remains open, while preserving the existing daemon-level
span coverage.
- Line 377: Restore the setAlertConfigEnabled import from src/db/repositories.ts
in tests/core/monitor.test.ts so both tests can resolve and call the repository
function.
In `@tests/daemon/loop.test.ts`:
- Around line 489-507: Update the test “increments the skipped-cycles counter
when a tick is skipped due to an in-flight cycle” to match scheduledTick’s
current behavior: remove the skipped-cycle counter setup and before/after
assertion, or restore the corresponding implementation increment if that
behavior remains required. Keep the test aligned with the chosen implementation
contract.
🪄 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: 580b76af-6260-45d5-8439-f4ff1a59436b
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (7)
README.mdsrc/commands/daemon.tssrc/core/monitor.tssrc/daemon/loop.tssrc/db/repositories.tstests/core/monitor.test.tstests/daemon/loop.test.ts
📜 Review details
⚠️ CI failures not shown inline (4)
GitHub Actions: CI Pipeline / build-and-test (24.x): docs: reconstruct CHANGELOG.md from git tag history
Conclusion: failure
##[group]Run npm audit --omit=dev --audit-level=high
�[36;1mnpm audit --omit=dev --audit-level=high�[0m
shell: /usr/bin/bash -e {0}
##[endgroup]
# npm audit report
fast-uri 3.0.0 - 3.1.4
Severity: high
fast-uri vulnerable to host confusion via backslash authority introducer - https://github.com/advisories/GHSA-7p8r-x3mc-p8w7
fix available via `npm audit fix`
node_modules/fast-uri
ip-address <=10.3.0
Severity: high
ip-address: Address4 decodes leading-zero octets as decimal while resolvers decode them as octal, allowing SSRF and trust-boundary bypass - https://github.com/advisories/GHSA-mwp4-54f8-5fhr
ip-address: a CIDR suffix on the parsed address suppresses special-use classification and can bypass SSRF and trust-boundary checks - https://github.com/advisories/GHSA-4xrf-jv44-h6hh
ip-address: misclassification of IPv4-mapped/NAT64 IPv6 addresses can bypass SSRF and trust-boundary checks - https://github.com/advisories/GHSA-22jq-vg5j-6vgg
fix available via `npm audit fix`
node_modules/ip-address
2 high severity vulnerabilities
To address all issues, run:
npm audit fix
##[error]Process completed with exit code 1.
GitHub Actions: CI Pipeline / build-and-test (22.x): docs: reconstruct CHANGELOG.md from git tag history
Conclusion: failure
##[group]Run npm audit --omit=dev --audit-level=high
�[36;1mnpm audit --omit=dev --audit-level=high�[0m
shell: /usr/bin/bash -e {0}
##[endgroup]
# npm audit report
fast-uri 3.0.0 - 3.1.4
Severity: high
fast-uri vulnerable to host confusion via backslash authority introducer - https://github.com/advisories/GHSA-7p8r-x3mc-p8w7
fix available via `npm audit fix`
node_modules/fast-uri
ip-address <=10.3.0
Severity: high
ip-address: Address4 decodes leading-zero octets as decimal while resolvers decode them as octal, allowing SSRF and trust-boundary bypass - https://github.com/advisories/GHSA-mwp4-54f8-5fhr
ip-address: a CIDR suffix on the parsed address suppresses special-use classification and can bypass SSRF and trust-boundary checks - https://github.com/advisories/GHSA-4xrf-jv44-h6hh
ip-address: misclassification of IPv4-mapped/NAT64 IPv6 addresses can bypass SSRF and trust-boundary checks - https://github.com/advisories/GHSA-22jq-vg5j-6vgg
fix available via `npm audit fix`
node_modules/ip-address
2 high severity vulnerabilities
To address all issues, run:
npm audit fix
##[error]Process completed with exit code 1.
GitHub Actions: CI Pipeline / 0_build-and-test (24.x).txt: docs: reconstruct CHANGELOG.md from git tag history
Conclusion: failure
##[group]Run npm audit --omit=dev --audit-level=high
�[36;1mnpm audit --omit=dev --audit-level=high�[0m
shell: /usr/bin/bash -e {0}
##[endgroup]
# npm audit report
fast-uri 3.0.0 - 3.1.4
Severity: high
fast-uri vulnerable to host confusion via backslash authority introducer - https://github.com/advisories/GHSA-7p8r-x3mc-p8w7
fix available via `npm audit fix`
node_modules/fast-uri
ip-address <=10.3.0
Severity: high
ip-address: Address4 decodes leading-zero octets as decimal while resolvers decode them as octal, allowing SSRF and trust-boundary bypass - https://github.com/advisories/GHSA-mwp4-54f8-5fhr
ip-address: a CIDR suffix on the parsed address suppresses special-use classification and can bypass SSRF and trust-boundary checks - https://github.com/advisories/GHSA-4xrf-jv44-h6hh
ip-address: misclassification of IPv4-mapped/NAT64 IPv6 addresses can bypass SSRF and trust-boundary checks - https://github.com/advisories/GHSA-22jq-vg5j-6vgg
fix available via `npm audit fix`
node_modules/ip-address
2 high severity vulnerabilities
To address all issues, run:
npm audit fix
##[error]Process completed with exit code 1.
GitHub Actions: CI Pipeline / 1_build-and-test (22.x).txt: docs: reconstruct CHANGELOG.md from git tag history
Conclusion: failure
##[group]Run npm audit --omit=dev --audit-level=high
�[36;1mnpm audit --omit=dev --audit-level=high�[0m
shell: /usr/bin/bash -e {0}
##[endgroup]
# npm audit report
fast-uri 3.0.0 - 3.1.4
Severity: high
fast-uri vulnerable to host confusion via backslash authority introducer - https://github.com/advisories/GHSA-7p8r-x3mc-p8w7
fix available via `npm audit fix`
node_modules/fast-uri
ip-address <=10.3.0
Severity: high
ip-address: Address4 decodes leading-zero octets as decimal while resolvers decode them as octal, allowing SSRF and trust-boundary bypass - https://github.com/advisories/GHSA-mwp4-54f8-5fhr
ip-address: a CIDR suffix on the parsed address suppresses special-use classification and can bypass SSRF and trust-boundary checks - https://github.com/advisories/GHSA-4xrf-jv44-h6hh
ip-address: misclassification of IPv4-mapped/NAT64 IPv6 addresses can bypass SSRF and trust-boundary checks - https://github.com/advisories/GHSA-22jq-vg5j-6vgg
fix available via `npm audit fix`
node_modules/ip-address
2 high severity vulnerabilities
To address all issues, run:
npm audit fix
##[error]Process completed with exit code 1.
🔇 Additional comments (6)
README.md (1)
21-22: LGTM!Also applies to: 375-375, 686-686
src/commands/daemon.ts (1)
15-20: LGTM!Also applies to: 33-36, 81-84
tests/daemon/loop.test.ts (1)
514-529: LGTM!Also applies to: 565-571
src/db/repositories.ts (2)
44-44: LGTM!Also applies to: 58-64, 329-349, 719-720
300-305: 🗄️ Data Integrity & IntegrationNo caller update needed.
insertAlertConfigis only called as a statement, and no production path uses its return value to createAlertConfigTarget.alert_config_idvalues.tests/core/monitor.test.ts (1)
1-1: LGTM!
AbdulmalikAlayande
added a commit
that referenced
this pull request
Aug 8, 2026
Adds CHANGELOG.md in Keep a Changelog format covering Unreleased, 1.0.0, 0.1.2, 0.1.1, and 0.1.0, reconstructed from real "bump version" commits in git history. Linked from README.md and added to package.json's files array. The 0.1.0/0.1.1/0.1.2 version numbers are real (confirmed via `git log --all --grep`) but were never tagged as GitHub releases — only v1.0.0 is an actual tag (verified against `git ls-remote --tags origin`). The PR's compare-links for those three versions pointed at tags that don't exist and would 404; dropped those broken links (kept content, which is genuinely commit-history-grounded) and fixed the [1.0.0] entry's date, which didn't match the real tag's commit date. Adjusted two of the PR's own test assertions to match.
Collaborator
|
Merged via 5e29a0e on main. Great reconstruction — the version content is genuinely grounded in real bump-version commits. One issue: the compare-links for 0.1.0/0.1.1/0.1.2 pointed at GitHub tags that don't actually exist (only v1.0.0 is tagged) and would 404. Dropped those three broken links, kept the content, and fixed the 1.0.0 entry's date to match the real tag. |
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
17 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Close: #481
What does this PR do?
Done. Here's what was implemented:
Created:
Edited:
All 65 doc tests pass (4 test files, 0 failures).
Why?
Does this touch secret-key handling or transaction submission?
Checklist
npm test)npx tsc --noEmit)npm run lint)console.login core logic