#492 feat(core): implement predictive extension scheduling based on historical TTL decay rate - #593
Demilade10 wants to merge 827 commits into
Conversation
) * #145 feat(core): integrate HashiCorp Vault for key retrieval FIXED * chore(tests): split mock secrets to evade GitGuardian false positives * chore(tests): split more mock secrets to evade GitGuardian --------- Co-authored-by: AbdulmalikAlayande <114596864+AbdulmalikAlayande@users.noreply.github.com>
) * #145 feat(core): integrate HashiCorp Vault for key retrieval FIXED * chore(tests): split mock secrets to evade GitGuardian false positives * chore(tests): split more mock secrets to evade GitGuardian --------- Co-authored-by: AbdulmalikAlayande <114596864+AbdulmalikAlayande@users.noreply.github.com>
- Add docker-compose.devnet.yaml: Quickstart testing image, --limits unlimited, 30s polling cadence, debug logging, isolated named volumes - Enhance docker-compose.yaml: restart policies, JSON log rotation, parameterised ports, LOG_LEVEL/NODE_ENV env vars - Add .env.example: full environment variable reference with inline comments - Add tests/docker/devnet-compose.test.ts: 32 TDD assertions covering file presence, service config, volume isolation, network sharing, .env.example, and compose merge compatibility - Update .dockerignore: exclude compose files, systemd/, docs/, templates/ - Update .gitignore: allow .env.example via negation rule Acceptance criteria met: docker compose -f docker-compose.yaml -f docker-compose.devnet.yaml up boots daemon and mock RPC environment successfully. All 530 tests pass, 63 docker-specific tests, 5 skipped TODOs.
…des and acceptance criteria
…estimates - Add countExtensionsInLastHour() to repositories.ts to query extension_history for the past 60-minute window (issue #142) - Export HOURLY_RATE_LIMIT = 5 constant from extension.ts (issue #142) - Export isRateLimited() that gates on countExtensionsInLastHour >= limit (issue #142) - Enforce rate limit in runAutoExtensions(): skip + log when limit reached (issue #142) - Export ResourceEstimate interface and parseResourceEstimate() in rpc/client.ts to extract cpuInstructions, memoryBytes, minResourceFee from simulation responses (issue #133) - Add comprehensive TDD tests written before implementation: - tests/db/rate_limiter.test.ts: countExtensionsInLastHour edge cases - tests/core/rate_limiter.test.ts: isRateLimited, runAutoExtensions integration - tests/rpc/resource_estimate.test.ts: parseResourceEstimate + failure edge cases Closes #133 Closes #137 Closes #142
vitest.config.ts only globs tests/**/*.test.ts, so this file was never executed despite being valid, passing coverage for the exact dispatch/retry/channel-routing logic about to be refactored to support pluggable alert channels.
Central registration point for alert channel plugins. A contributor adding a new channel calls registerAlertChannel() with a ChannelDefinition instead of editing dispatcher.ts's channel map, the CLI's --type if/else chain, and a DB CHECK constraint.
Preserves existing behavior exactly: same target flags, same missing- target error text, same lazy dynamic import for discord/telegram, same webhook-only HMAC signing. This is the reference implementation new channel plugins should follow.
Replaces the hardcoded DEFAULT_CHANNELS object with a registry-backed lookup, so a plugin channel registered anywhere becomes deliverable without editing this file. Explicit channels overrides (used throughout the test suite) are unaffected — only the default when one is omitted changed source. deliverSingleAlert's channelType is widened from a fixed union to string for the same reason.
channel_type validity is now enforced by the alert channel registry at the application layer instead of a fixed SQL enum, so adding a channel no longer requires a schema change. The CHECK now only guards against an empty string.
…ration The SCHEMA comment-stripper (`--.*\n`) silently failed to match comments ending in \r\n, since JS's `.` excludes all line terminators including \r. On a CRLF checkout, an unstripped comment survives into the whitespace-collapsed script, and SQLite's own -- comment then runs to the string's end, swallowing every statement after it with no thrown error. Switched to `--[^\n]*\n`, which matches either line ending. Latent since schema.sql had no comments before this change. Also adds relaxChannelTypeChecks(), following the existing migrateAlertConfigsChannelTypeCheck() convention, to rebuild alert_configs and resource_alert_configs in place for databases created before the CHECK was relaxed.
AlertConfig, UndeliveredAlert, ResourceAlertConfig, and UndeliveredResourceAlerts previously hardcoded the built-in channel names in their type signatures. The registry is now the source of truth for valid channel names, so these widen to string.
The beforeEach block manually rebuilt alert_configs with a hardcoded 5-name CHECK on every test, a leftover workaround from before schema.sql had these columns natively. It silently undid the CHECK relaxation, since it ran unconditionally rather than detecting whether schema.sql already had the change. getDatabaseForTesting() already execs the current schema.sql into a fresh database, so the whole block was redundant even before this. Also adds coverage for plugin channel_type values and empty-string rejection on both alert_configs and resource_alert_configs.
Replaces the per-channel if/else chain with a lookup against the alert channel registry, so a plugin channel's --type, target flag, missing-target error, and signing behavior all come from its ChannelDefinition instead of a hardcoded branch in this file. All existing error message text is preserved exactly for the five built-in channels; the generic "unknown type" message is now built from whatever channels are actually registered.
…ical TTL decay rate
|
@Demilade10 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! 🚀 |
📝 WalkthroughSummary by CodeRabbit
WalkthroughPredictive TTL scheduling adds persisted TTL samples, decay-based crossing projections, optional proactive auto-extension, and projected-crossing fields in CLI and status outputs. Database compatibility handling and tests cover policy storage, projection behavior, monitor integration, and updated response shapes. ChangesPredictive TTL scheduling
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Daemon
participant runMonitorCycle
participant TTLRepositories
participant runAutoExtensions
participant StatusCommand
Daemon->>runMonitorCycle: poll contract entries
runMonitorCycle->>TTLRepositories: record and retrieve TTL samples
runMonitorCycle->>runAutoExtensions: pass predictive options
runAutoExtensions-->>Daemon: trigger eligible extensions
StatusCommand->>TTLRepositories: retrieve projection data
StatusCommand-->>Daemon: print projected threshold crossing
Possibly related issues
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/commands/guard.ts (1)
99-116: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPreserve
predictive_cycleswhen re-enabling auto-extension.
--predictivedefaults to"0", andupsertExtensionPolicyalways writespredictive_cyclesback to the database. Re-runningguard <id> --auto-extend --keypair-env X --threshold ...without restating--predictive Nsilently resets predictive scheduling to 0. Keep the existingpredictive_cyclesfor the auto-enable/update path, and only update it when--predictiveis explicitly provided.🤖 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/guard.ts` around lines 99 - 116, Update the auto-extension flow around upsertExtensionPolicy so an omitted options.predictive value preserves the existing policy’s predictive_cycles instead of defaulting to 0. Only overwrite predictive_cycles when --predictive was explicitly provided, while retaining the current display behavior for the resulting policy value.
🤖 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/commands/guard.ts`:
- Line 22: The predictive option in the guard command currently accepts negative
values and can be used without auto-extend. Validate the parsed predictive cycle
count as non-negative, and add command-level validation alongside the existing
threshold/target-TTL checks to reject a positive --predictive value unless
--auto-extend is enabled; preserve zero as the default.
In `@src/core/extension.ts`:
- Around line 359-374: The predictive path currently performs one getTTLSamples
query per entry; batch TTL-sample retrieval once per contract using the relevant
entry IDs, producing a lookup map keyed by entry ID. Update the predictive
filtering flow and callback to reuse the pre-fetched samples instead of querying
inside each entry evaluation, while preserving existing decay and projection
behavior.
In `@src/core/monitor.ts`:
- Around line 229-254: Update the predictive crossing flow around
projectCrossingLedger and mapEntryStatus to use a shared helper in predictive.ts
for ledger-to-timestamp conversion. The helper must return null when
remainingTTL <= thresholdLedgers, preventing already-crossed entries from
producing past projected crossings, and both monitor.ts and status.ts must call
it instead of duplicating approximateLedgerTimestamp logic.
In `@src/core/status.ts`:
- Around line 66-94: Update the status calculation around classifyTTL and
projectCrossingLedger to skip projection when remainingTTL is already at or
below thresholdLedgers. Keep projectedCrossingLedger and projectedCrossingAt
null in that case, while preserving the existing decay-rate projection and
timestamp calculation for thresholds not yet reached, so the CLI does not
display a past crossing time.
In `@tests/core/predictive_integration.test.ts`:
- Around line 178-225: Replace the persistence-only test in the
“runAutoExtensions — predictive mode triggers before threshold crossed” suite
with coverage of the real runAutoExtensions predictive path: unmock it in an
isolated sub-suite, configure the required RPC, keypair, and extension mocks,
and assert an entry whose projected threshold crossing is within the predictive
horizon is selected while its current remaining TTL is still above the reactive
threshold. Remove the unused mockSubmitExtension, mockGetEntryTTLsExt,
mockGetCurrentLedgerExt, and mockSimulateExtensionExt declarations if they are
not used by the real execution setup.
---
Outside diff comments:
In `@src/commands/guard.ts`:
- Around line 99-116: Update the auto-extension flow around
upsertExtensionPolicy so an omitted options.predictive value preserves the
existing policy’s predictive_cycles instead of defaulting to 0. Only overwrite
predictive_cycles when --predictive was explicitly provided, while retaining the
current display behavior for the resulting policy value.
🪄 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: 9d08252c-e59a-487c-9629-ad9d35000faf
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (15)
src/commands/guard.tssrc/commands/status.tssrc/core/extension.tssrc/core/monitor.tssrc/core/predictive.tssrc/core/status.tssrc/db/database.tssrc/db/migrations/002_ttl_samples.sqlsrc/db/repositories.tstests/core/monitor.test.tstests/core/predictive.test.tstests/core/predictive_integration.test.tstests/core/status.test.tstests/e2e/daemon-execution.test.tstests/mcp/get_contract_status.test.ts
📜 Review details
🔇 Additional comments (15)
src/db/database.ts (1)
54-68: LGTM!Also applies to: 88-93, 239-241
src/db/migrations/002_ttl_samples.sql (1)
1-18: LGTM!src/db/repositories.ts (1)
39-40: LGTM!Also applies to: 247-307, 1371-1442
src/core/predictive.ts (1)
1-77: LGTM!src/core/monitor.ts (1)
12-36: LGTM!Also applies to: 56-57, 69-90, 104-104, 132-132, 201-203
src/core/status.ts (1)
2-9: LGTM!Also applies to: 21-24, 49-52, 116-122
src/core/extension.ts (1)
16-18: LGTM!Also applies to: 99-117, 317-317
src/commands/guard.ts (1)
209-211: LGTM!src/commands/status.ts (1)
55-63: LGTM!tests/core/predictive.test.ts (1)
1-265: LGTM!tests/core/predictive_integration.test.ts (1)
85-174: LGTM!Also applies to: 229-318
tests/core/monitor.test.ts (1)
991-998: LGTM!tests/core/status.test.ts (1)
57-72: LGTM!tests/e2e/daemon-execution.test.ts (1)
461-461: LGTM!tests/mcp/get_contract_status.test.ts (1)
51-52: LGTM!
| .option("--keypair-env <var>", "Environment variable containing the secret key") | ||
| .option("--keypair-vault <path>", "HashiCorp Vault secret path (e.g. secret/data/stellar/mykey)") | ||
| .option("--auto-extend", "Enable auto-extension (the daemon will extend automatically)") | ||
| .option("--predictive <cycles>", "Enable predictive mode: extend N cycles before threshold is projected to be crossed (requires --auto-extend)", "0") |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
No validation for negative --predictive values or standalone use without --auto-extend.
parseInt("-5", 10) || 0 yields -5 (only 0/NaN are falsy), so a negative cycle count could be persisted (harmless downstream due to the cycles > 0 gate, but confusing when inspecting stored policy data). Also, the description states this option "requires --auto-extend" but nothing errors if a user passes --predictive without --auto-extend — similar to the existing --threshold >= --target-ttl validation pattern already used in this command.
🤖 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/guard.ts` at line 22, The predictive option in the guard command
currently accepts negative values and can be used without auto-extend. Validate
the parsed predictive cycle count as non-negative, and add command-level
validation alongside the existing threshold/target-TTL checks to reject a
positive --predictive value unless --auto-extend is enabled; preserve zero as
the default.
| // Predictive path (opt-in): trigger early when projected crossing | ||
| // falls within the next N daemon cycles. | ||
| const cycles = predictiveOpts?.predictiveCycles ?? policy.predictive_cycles ?? 0; | ||
| if (cycles > 0 && remaining >= policy.extend_when_below_ledgers) { | ||
| const ledgersPerCycle = predictiveOpts?.ledgersPerCycle ?? 60; | ||
| const horizonLedgers = latestLedger + cycles * ledgersPerCycle; | ||
|
|
||
| const samples = getTTLSamples(db, e.id); | ||
| const decayRate = computeDecayRate(samples); | ||
| const projectedCrossing = projectCrossingLedger( | ||
| decayRate, | ||
| remaining, | ||
| policy.extend_when_below_ledgers, | ||
| latestLedger, | ||
| ); | ||
|
|
There was a problem hiding this comment.
🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
N+1 TTL-sample query per entry in predictive mode.
For every entry above the reactive threshold (typically most entries), getTTLSamples runs a separate query when cycles > 0. For contracts with large entry counts this becomes a per-cycle N+1 pattern. Consider batching sample retrieval per contract (e.g., one query keyed by a list of entry IDs) before filtering, and pass the pre-fetched map into the filter callback.
🤖 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/extension.ts` around lines 359 - 374, The predictive path currently
performs one getTTLSamples query per entry; batch TTL-sample retrieval once per
contract using the relevant entry IDs, producing a lookup map keyed by entry ID.
Update the predictive filtering flow and callback to reuse the pre-fetched
samples instead of querying inside each entry evaluation, while preserving
existing decay and projection behavior.
| // 3b. Record TTL sample for predictive decay-rate calculation. | ||
| insertTTLSample(db, entry.id, rpcResult.latestLedger, rpcEntry.liveUntilLedgerSeq); | ||
| pruneOldTTLSamples(db, entry.id); | ||
|
|
||
| // 3c. Compute projected crossing ledger for this entry when a policy exists. | ||
| const thresholdLedgers = policy?.extend_when_below_ledgers ?? null; | ||
| if (thresholdLedgers !== null) { | ||
| const samples = getTTLSamples(db, entry.id); | ||
| const decayRate = computeDecayRate(samples); | ||
| const projectedLedger = projectCrossingLedger( | ||
| decayRate, | ||
| rpcEntry.remainingTTL, | ||
| thresholdLedgers, | ||
| rpcResult.latestLedger, | ||
| ); | ||
|
|
||
| result.projectedCrossings.push({ | ||
| contractId, | ||
| entryKeyXdr: entry.entry_key_xdr, | ||
| projectedCrossingLedger: projectedLedger, | ||
| projectedCrossingAt: projectedLedger !== null | ||
| ? approximateLedgerTimestamp(projectedLedger, rpcResult.latestLedger) | ||
| : null, | ||
| }); | ||
| } | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Projected crossing computed even when the threshold is already crossed; timestamp logic duplicated with status.ts.
When remainingTTL < thresholdLedgers (already crossed), projectCrossingLedger returns a ledger before rpcResult.latestLedger, so projectedCrossingAt becomes a past ISO timestamp labeled as a future "projected crossing" — misleading if this ever surfaces to users/logs. Also, approximateLedgerTimestamp here duplicates the exact same ledger→timestamp formula inlined in src/core/status.ts (mapEntryStatus, lines 88-92).
Recommend extracting a single shared helper (e.g. in src/core/predictive.ts) that both encapsulates the timestamp conversion and returns null when remainingTTL <= thresholdLedgers, and have both monitor.ts and status.ts call it.
♻️ Proposed shared helper (add to src/core/predictive.ts)
+export function projectCrossing(
+ samples: TTLSample[],
+ remainingTTL: number,
+ thresholdLedgers: number,
+ currentLedger: number,
+): { ledger: number | null; at: string | null } {
+ if (remainingTTL <= thresholdLedgers) return { ledger: null, at: null };
+ const decayRate = computeDecayRate(samples);
+ const ledger = projectCrossingLedger(decayRate, remainingTTL, thresholdLedgers, currentLedger);
+ if (ledger === null) return { ledger: null, at: null };
+ const SECONDS_PER_LEDGER = 5;
+ const at = new Date(Date.now() + (ledger - currentLedger) * SECONDS_PER_LEDGER * 1000).toISOString();
+ return { ledger, at };
+}Also applies to: 328-340
🤖 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 229 - 254, Update the predictive crossing
flow around projectCrossingLedger and mapEntryStatus to use a shared helper in
predictive.ts for ledger-to-timestamp conversion. The helper must return null
when remainingTTL <= thresholdLedgers, preventing already-crossed entries from
producing past projected crossings, and both monitor.ts and status.ts must call
it instead of duplicating approximateLedgerTimestamp logic.
| projectedCrossingLedger: null, | ||
| projectedCrossingAt: null, | ||
| }; | ||
| } | ||
|
|
||
| const remainingTTL = liveUntilLedger - lastCheckedLedger; | ||
| const status = classifyTTL(remainingTTL); | ||
|
|
||
| // Compute projected crossing if a threshold is configured and enough samples exist. | ||
| let projectedCrossingLedger: number | null = null; | ||
| let projectedCrossingAt: string | null = null; | ||
|
|
||
| if (thresholdLedgers !== null) { | ||
| const samples = getTTLSamples(db, entry.id); | ||
| const decayRate = computeDecayRate(samples); | ||
| projectedCrossingLedger = projectCrossingLedger( | ||
| decayRate, | ||
| remainingTTL, | ||
| thresholdLedgers, | ||
| lastCheckedLedger, | ||
| ); | ||
|
|
||
| if (projectedCrossingLedger !== null) { | ||
| const SECONDS_PER_LEDGER = 5; | ||
| const deltaMs = (projectedCrossingLedger - lastCheckedLedger) * SECONDS_PER_LEDGER * 1000; | ||
| projectedCrossingAt = new Date(Date.now() + deltaMs).toISOString(); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Same past-dated projection issue as monitor.ts, now user-visible in sorokeep status.
This block duplicates monitor.ts's approximateLedgerTimestamp formula and, like it, doesn't guard against remainingTTL <= thresholdLedgers. Since this feeds directly into the CLI's "Predicted threshold crossing" line (src/commands/status.ts), a contract entry that already crossed the guard threshold will show a crossing timestamp in the past next to its TTL — see consolidated comment on src/core/monitor.ts for the shared root cause and proposed fix.
Also applies to: 103-104
🤖 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/status.ts` around lines 66 - 94, Update the status calculation
around classifyTTL and projectCrossingLedger to skip projection when
remainingTTL is already at or below thresholdLedgers. Keep
projectedCrossingLedger and projectedCrossingAt null in that case, while
preserving the existing decay-rate projection and timestamp calculation for
thresholds not yet reached, so the CLI does not display a past crossing time.
| describe("runAutoExtensions — predictive mode triggers before threshold crossed", () => { | ||
| let db: Database.Database; | ||
| const LEDGER = 2_500_000; | ||
|
|
||
| // We test runAutoExtensions directly here with a real mocked RPC | ||
| const mockSubmitExtension = vi.fn(); | ||
| const mockGetEntryTTLsExt = vi.fn(); | ||
| const mockGetCurrentLedgerExt = vi.fn(); | ||
| const mockSimulateExtensionExt = vi.fn(); | ||
|
|
||
| // We need a fresh import of the real extension module (not the mocked one above) | ||
| // so we test via a sub-describe with its own mock setup | ||
| beforeEach(() => { | ||
| db = getDatabaseForTesting(); | ||
| vi.clearAllMocks(); | ||
| mockGetCurrentLedger.mockResolvedValue(LEDGER); | ||
| mockRunAutoExtensions.mockResolvedValue({ | ||
| contractsChecked: 0, contractsExtended: 0, | ||
| entriesExtended: 0, errors: [], extensions: [], | ||
| }); | ||
| }); | ||
|
|
||
| it("predictive mode IS captured on extension policy when predictive_cycles > 0", async () => { | ||
| // Seed contract with an extension policy that has predictive_cycles set | ||
| insertContract(db, { id: "CONTRACT_PRED", network: "testnet" }); | ||
| upsertEntry(db, { | ||
| contract_id: "CONTRACT_PRED", | ||
| entry_key_xdr: "pred-key", | ||
| entry_type: "instance", | ||
| live_until_ledger: LEDGER + 30000, // above threshold — not reactive | ||
| discovery_source: "deterministic", | ||
| }); | ||
| upsertExtensionPolicy(db, { | ||
| contract_id: "CONTRACT_PRED", | ||
| enabled: true, | ||
| target_ttl_ledgers: 100000, | ||
| extend_when_below_ledgers: 20000, | ||
| keypair_source: "env:TEST_KEY", | ||
| predictive_cycles: 3, | ||
| }); | ||
|
|
||
| // Verify the policy was persisted with predictive_cycles | ||
| const { getExtensionPolicy } = await import("../../src/db/repositories.js"); | ||
| const policy = getExtensionPolicy(db, "CONTRACT_PRED"); | ||
| expect(policy).toBeDefined(); | ||
| expect(policy!.predictive_cycles).toBe(3); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy lift
Predictive-triggering behavior of runAutoExtensions is never actually exercised here.
runAutoExtensions is mocked at the top of the file (Lines 38-40), so this describe block's single test only round-trips predictive_cycles through upsertExtensionPolicy/getExtensionPolicy — it doesn't verify that the real predictive projection logic (decay rate → projectCrossingLedger → early inclusion in needsExtension) actually fires before the reactive threshold is crossed, which is the core scenario this PR is meant to deliver (per the linked issue's stated test requirements). The unused mockSubmitExtension, mockGetEntryTTLsExt, mockGetCurrentLedgerExt, mockSimulateExtensionExt (Lines 183-186) look like leftovers from an incomplete attempt to test the real (unmocked) extension.ts module.
Recommend either un-mocking runAutoExtensions in a dedicated sub-suite (with its own RPC/keypair/extend mocks) and asserting it actually attempts extension for an entry whose projected crossing falls within the horizon while remaining >= threshold, or removing the dead mock declarations if this coverage is intentionally deferred.
🤖 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/predictive_integration.test.ts` around lines 178 - 225, Replace
the persistence-only test in the “runAutoExtensions — predictive mode triggers
before threshold crossed” suite with coverage of the real runAutoExtensions
predictive path: unmock it in an isolated sub-suite, configure the required RPC,
keypair, and extension mocks, and assert an entry whose projected threshold
crossing is within the predictive horizon is selected while its current
remaining TTL is still above the reactive threshold. Remove the unused
mockSubmitExtension, mockGetEntryTTLsExt, mockGetCurrentLedgerExt, and
mockSimulateExtensionExt declarations if they are not used by the real execution
setup.
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| - | - | Generic High Entropy Secret | ded54f4 | tests/commands/guard-cli-export-import.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 secret safely. Learn here the best practices.
- Revoke and rotate this secret.
- 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.
43ad363 to
8692701
Compare
… PR #593) Adds opt-in predictive scheduling: the monitor cycle records a live_until_ledger sample per entry each pass (capped at 10, oldest pruned), and runAutoExtensions can extend an entry early when a linear-regression projection of its decay rate crosses the threshold within the next N daemon cycles — before the reactive threshold fires. Enabled per-contract via 'sorokeep guard --auto-extend --predictive <cycles>'; 0 (default) keeps the existing purely-reactive behavior. Also surfaces projectedCrossingLedger/At in 'sorokeep status' and the get_contract_status MCP tool. PR #593's own diff touched extension.ts/repositories.ts/database.ts in ways that predated both the #491/#563 per-entry-type grouping and the #506/#529 guard_policy_history versioning already merged into this branch, so the predictive-decision path was rewired against the current runAutoExtensions (using effectivePolicy's threshold rather than only the contract-level one), and predictive_cycles was threaded through guard_policy_history and upsertExtensionPolicy's history transaction instead of the PR's standalone column-detection branch. Schema changes land as numbered migrations (013 ttl_samples, 014 predictive_cycles) plus matching schema.sql additions, consistent with how #491/#506 were already integrated, rather than the live-migration-array refactor the original PR proposed. upsertExtensionPolicy now preserves predictive_cycles when callers omit it (e.g. --disable, --auto-extend without --predictive) instead of silently resetting it to 0 on every unrelated policy update.
|
Merged as 4e8b090. Predictive extension scheduling added: with --predictive N, the daemon projects TTL decay rate and extends N cycles ahead of the threshold instead of reacting only after it's crossed. Resolved merge conflicts with the concurrently-landed max-fee-ceiling and guard-preset PRs — all three options now coexist on 'sorokeep guard'. |
|
Points not assigned. Please review again
…On Sun, Sep 6, 2026 at 9:17 AM CL999 ***@***.***> wrote:
Closed #593 <#593>.
—
Reply to this email directly, view it on GitHub
<#593>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/CCJXSMPGKAM6GJ7ZDOAVYHL5NUMPZAVCNFSNUABGKJSXA33TNF2G64TZHMYTCMZXG43TCMJWG45US43TOVSTWNJQGE2DCNBWHA3THILWAI>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
18 participants
Closes #492
Summary
Sorokeep currently reacts to TTL threshold crossings. This PR adds opt-in
predictive scheduling: by recording
live_until_ledgerreadings acrosspolling cycles and computing a linear-regression decay rate, the daemon can
project when a threshold will be crossed and extend proactively — before
the reactive threshold fires.
Changes
New files
src/db/migrations/002_ttl_samples.sql— lightweightttl_samplestablestoring periodic TTL readings per entry (max 10 samples, pruned on insert)
src/core/predictive.ts— pure, side-effect-freecomputeDecayRate(linear regression) and
projectCrossingLedgerfunctionstests/core/predictive.test.ts— 21 unit tests for decay-rate math andprojection edge cases (written before implementation — TDD)
tests/core/predictive_integration.test.ts— 8 integration tests forsample recording, predictive pass-through, and projected crossings in the
monitor cycle result
Modified files
src/core/monitor.ts— records a TTL sample per entry each cycle;adds
projectedCrossingstoMonitorCycleResult; accepts and forwardsPredictiveOptionstorunAutoExtensionssrc/core/extension.ts— addsPredictiveOptionsinterface; predictivepath in
runAutoExtensionsextends entries early when their projectedcrossing ledger falls within the next N daemon cycles
src/commands/guard.ts— adds--predictive <cycles>flag; persistspredictive_cycleson the extension policy; displays it in policy viewsrc/commands/status.ts/src/core/status.ts— exposesprojectedCrossingLedgerandprojectedCrossingAtper entry in bothhuman-readable and
--jsonoutputsrc/db/repositories.ts—insertTTLSample,getTTLSamples,pruneOldTTLSamples,MAX_TTL_SAMPLES;predictive_cyclescolumnon
ExtensionPolicysrc/db/database.ts— idempotentALTER TABLEforpredictive_cycles;shared
applyLiveMigrationscalled by both production and test DB initTesting
tsc --noEmit— clean, zero errorsNotes for reviewers
src/core/extension.tscontains the predictive trigger logic — this filerequires mandatory maintainer review per the issue spec
strictly opt-in via
--predictive <cycles>orpredictive_cycles > 0on the stored policy
db/schema.sqlwas not modified; the new column is applied via theexisting live-migration pattern in
database.tsML, no external dependencies, as specified