test(core): add policy precedence matrix tests for extension entry-type overrides - #598
Open
Demilade10 wants to merge 828 commits into
Open
Demilade10 wants to merge 828 commits into
Demilade10 wants to merge 828 commits into
Conversation
…FIXED (TegoLabs#270) * TegoLabs#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>
…FIXED (TegoLabs#270) * TegoLabs#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 TegoLabs#142) - Export HOURLY_RATE_LIMIT = 5 constant from extension.ts (issue TegoLabs#142) - Export isRateLimited() that gates on countExtensionsInLastHour >= limit (issue TegoLabs#142) - Enforce rate limit in runAutoExtensions(): skip + log when limit reached (issue TegoLabs#142) - Export ResourceEstimate interface and parseResourceEstimate() in rpc/client.ts to extract cpuInstructions, memoryBytes, minResourceFee from simulation responses (issue TegoLabs#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 TegoLabs#133 Closes TegoLabs#137 Closes TegoLabs#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
…hen multiple policies could apply
|
@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
WalkthroughThe PR adds predictive TTL scheduling: it stores bounded TTL samples, estimates decay, projects threshold crossings, persists predictive cycle settings, triggers auto-extensions within a forecast horizon, and exposes projections through monitor, status, and CLI output. ChangesPredictive TTL scheduling
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Warning |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 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: Validate the --predictive option in the command setup like
--target-ttl and --threshold, rejecting non-numeric or negative values and
rejecting its use without --auto-extend. Parse the validated value once, reuse
that value in the guard configuration and all predictive branches, and remove
the duplicated parseInt(options.predictive ?? "0", 10) expressions.
In `@src/core/extension.ts`:
- Around line 366-373: Refactor the filtering flow around getTTLSamples so TTL
samples are loaded in one batched database query for all relevant entry IDs
before iterating entries, then reuse the per-entry results when computing
decayRate and projectedCrossing. Preserve the existing cycles === 0 shortcut and
avoid any synchronous DB read inside the per-entry filter.
In `@src/core/monitor.ts`:
- Around line 328-341: The projection-to-timestamp logic is duplicated between
monitor and status, including the ledger interval constant. In
src/core/monitor.ts lines 328-341, extract approximateLedgerTimestamp and, where
appropriate, the surrounding projection flow into a shared helper in
src/core/predictive.ts; update the monitor caller to use it. In
src/core/status.ts lines 74-94, replace the local decay, projection, and
timestamp calculations with the same shared helper and remove its duplicate
constant, ensuring both callers share one ledger-close-time assumption and
projection strategy.
In `@src/db/database.ts`:
- Around line 88-93: Replace the blanket catch around the migration loop in the
database initialization flow with handling that ignores only the expected
“column already exists” condition and rethrows or surfaces all other migration
errors. Ensure failures in adding predictive_cycles cannot be silently hidden
before upsertExtensionPolicy relies on the column.
In `@src/db/repositories.ts`:
- Around line 247-301: Update upsertExtensionPolicy so an omitted
policy.predictive_cycles preserves the existing database value during ON
CONFLICT updates instead of defaulting to 0. Adjust the predictive_cycles
assignment and bound value in the hasPredictiveCycles branch, using the same
COALESCE-style existing-value preservation pattern as insertContract; retain 0
only for genuinely new rows.
In `@tests/core/extension.test.ts`:
- Around line 1053-1056: Add tests in the “Policy precedence interaction” suite
covering the predictive eligibility branch in src/core/extension.ts: precedence
between predictiveOpts.predictiveCycles and policy.predictive_cycles, the
ledgersPerCycle horizon, and projections both within and outside that horizon.
Use the existing RPC setup helpers and assertions in this test file, preserving
current entry-type precedence coverage.
- Around line 1459-1479: Make the cross-type isolation assertions in the test
unconditional by asserting that both instanceCall and wasmCall are defined
before checking their target values. Preserve the existing key-filtering
predicates and expected 75000/300000 targets, but remove the conditional guards
so missing calls fail the test.
- Around line 11-15: Prevent the imports of upsertEntryTypePolicy and
getEffectivePolicyForEntry in extension.test.ts from causing the existing
extension tests to fail before execution. Either gate the pending per-entry-type
policy suites with describe.skip/describe.todo and load those helpers
dynamically, or export both helpers from the repositories module while
preserving the current tests.
- Line 1: Remove the UTF-8 BOM from the beginning of
tests/core/extension.test.ts and replace the corrupted characters at the
specified lines with proper em dashes. Re-save the file using UTF-8 encoding
without a BOM, preserving all other content.
In `@tests/core/predictive_integration.test.ts`:
- Around line 178-225: The predictive-mode test must exercise the real
runAutoExtensions implementation rather than only verifying policy persistence.
Replace the top-level mock setup for this sub-describe with an isolated
import/mock configuration that uses the real runAutoExtensions and the declared
RPC mocks, invoke it with an entry still above extend_when_below_ledgers, and
assert that predictive_cycles causes an extension to be submitted before the
reactive threshold; remove any unused mock declarations or wire them into the
scenario.
🪄 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: 93dd91e3-8637-4adf-8818-c6c721ba34c0
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (16)
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/extension.test.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 (16)
src/core/predictive.ts (1)
1-77: LGTM!src/db/repositories.ts (1)
30-41: LGTM!Also applies to: 303-307, 1371-1442
tests/core/predictive.test.ts (1)
1-265: LGTM!src/core/monitor.ts (1)
12-36: LGTM!Also applies to: 56-57, 69-90, 104-104, 132-132, 201-203, 229-254
tests/core/predictive_integration.test.ts (1)
1-177: LGTM!Also applies to: 229-318
tests/core/status.test.ts (1)
60-61: LGTM!Also applies to: 71-72
tests/mcp/get_contract_status.test.ts (1)
51-52: LGTM!src/db/migrations/002_ttl_samples.sql (1)
8-17: LGTM!src/db/database.ts (1)
54-68: LGTM!Also applies to: 239-241
src/commands/status.ts (1)
55-63: LGTM!src/core/extension.ts (2)
16-18: LGTM!Also applies to: 99-116, 317-317
361-364: 🎯 Functional CorrectnessNo current caller passes
predictiveOptswith a defaultpredictiveCycles.The daemon does not provide
predictiveOptstorunMonitorCycle, so this precedence issue does not affect the current code path.> Likely an incorrect or invalid review comment.src/commands/guard.ts (1)
110-116: LGTM!Also applies to: 209-211
tests/core/extension.test.ts (1)
831-1038: LGTM!Also applies to: 1057-1102
tests/core/monitor.test.ts (1)
991-998: LGTM!tests/e2e/daemon-execution.test.ts (1)
461-461: 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
Validate --predictive and compute it once.
parseInt(options.predictive ?? "0", 10) || 0 is duplicated on Lines 106 and 109, and unlike --target-ttl/--threshold there is no validation: a non-numeric value is silently coerced to 0, and a negative value (--predictive -5) is persisted as-is into predictive_cycles. The help text also claims it "requires --auto-extend", but the flag is silently ignored on the other paths rather than rejected.
🛡️ Proposed fix
+ const predictiveCycles = parseInt(options.predictive ?? "0", 10);
+ if (isNaN(predictiveCycles) || predictiveCycles < 0) {
+ console.error(chalk.red("--predictive must be a non-negative number"));
+ process.exit(1);
+ }
+ if (predictiveCycles > 0 && !options.autoExtend) {
+ console.error(chalk.red("--predictive requires --auto-extend"));
+ process.exit(1);
+ }
@@
- predictive_cycles: parseInt(options.predictive ?? "0", 10) || 0,
+ predictive_cycles: predictiveCycles,
});
-
- const predictiveCycles = parseInt(options.predictive ?? "0", 10) || 0;Also applies to: 106-109
🤖 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, Validate the --predictive option in the
command setup like --target-ttl and --threshold, rejecting non-numeric or
negative values and rejecting its use without --auto-extend. Parse the validated
value once, reuse that value in the guard configuration and all predictive
branches, and remove the duplicated parseInt(options.predictive ?? "0", 10)
expressions.
Comment on lines
+366
to
+373
| 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 | 💤 Low value
Per-entry synchronous DB read inside the filter.
getTTLSamples runs a query for every entry above threshold on every cycle, for every contract, on the same thread as the concurrent extension work. For contracts with many entries this becomes N queries per cycle. Consider a single batched query keyed by entry_id IN (...) (or skipping the lookup entirely when cycles === 0, which it already does) and resolving samples once before the filter.
🤖 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 366 - 373, Refactor the filtering flow
around getTTLSamples so TTL samples are loaded in one batched database query for
all relevant entry IDs before iterating entries, then reuse the per-entry
results when computing decayRate and projectedCrossing. Preserve the existing
cycles === 0 shortcut and avoid any synchronous DB read inside the per-entry
filter.
Comment on lines
+328
to
341
| } | ||
|
|
||
| // ─── Private helpers ────────────────────────────────────────────────────────── | ||
|
|
||
| /** | ||
| * Approximate a wall-clock ISO-8601 timestamp for a future ledger. | ||
| * Stellar closes a ledger roughly every 5 seconds. | ||
| */ | ||
| function approximateLedgerTimestamp(targetLedger: number, currentLedger: number): string { | ||
| const SECONDS_PER_LEDGER = 5; | ||
| const deltaLedgers = targetLedger - currentLedger; | ||
| const deltaMs = deltaLedgers * SECONDS_PER_LEDGER * 1000; | ||
| return new Date(Date.now() + deltaMs).toISOString(); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Duplicated projection-to-timestamp logic (and magic constant) across monitor.ts and status.ts. Both files independently implement "load samples → computeDecayRate → projectCrossingLedger → convert ledger delta to ISO timestamp via SECONDS_PER_LEDGER = 5". This should be a single shared helper (e.g. in src/core/predictive.ts) to avoid the two copies drifting if the ledger-close-time assumption or projection strategy ever changes.
src/core/monitor.ts#L328-L341: extractapproximateLedgerTimestamp(and ideally the whole projection block at lines 234-253) into a shared helper insrc/core/predictive.tsthat both files import.src/core/status.ts#L74-L94: replace the inlined decay/projection/timestamp logic with the same shared helper instead of a second copy with its ownSECONDS_PER_LEDGERconstant.
📍 Affects 2 files
src/core/monitor.ts#L328-L341(this comment)src/core/status.ts#L74-L94
🤖 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 328 - 341, The projection-to-timestamp
logic is duplicated between monitor and status, including the ledger interval
constant. In src/core/monitor.ts lines 328-341, extract
approximateLedgerTimestamp and, where appropriate, the surrounding projection
flow into a shared helper in src/core/predictive.ts; update the monitor caller
to use it. In src/core/status.ts lines 74-94, replace the local decay,
projection, and timestamp calculations with the same shared helper and remove
its duplicate constant, ensuring both callers share one ledger-close-time
assumption and projection strategy.
Comment on lines
+88
to
93
| // issue #492 — predictive scheduling: add predictive_cycles to extension_policies | ||
| `ALTER TABLE extension_policies ADD COLUMN predictive_cycles INTEGER NOT NULL DEFAULT 0`, | ||
| ]; | ||
| for (const sql of migrations) { | ||
| try { db.exec(sql); } catch { /* column already exists — no-op */ } | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win
Narrow the swallowed error so real migration failures aren't hidden.
The blanket catch {} treats every failure as "column already exists". If ALTER TABLE extension_policies ADD COLUMN predictive_cycles fails for another reason (missing table, locked DB), the column is silently absent and upsertExtensionPolicy quietly falls back to the non-predictive branch (src/db/repositories.ts:240-301), so --predictive is dropped with no signal.
♻️ Suggested refactor
for (const sql of migrations) {
- try { db.exec(sql); } catch { /* column already exists — no-op */ }
+ try {
+ db.exec(sql);
+ } catch (err: unknown) {
+ const msg = err instanceof Error ? err.message : String(err);
+ if (!/duplicate column name|already exists/i.test(msg)) {
+ throw err;
+ }
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // issue #492 — predictive scheduling: add predictive_cycles to extension_policies | |
| `ALTER TABLE extension_policies ADD COLUMN predictive_cycles INTEGER NOT NULL DEFAULT 0`, | |
| ]; | |
| for (const sql of migrations) { | |
| try { db.exec(sql); } catch { /* column already exists — no-op */ } | |
| } | |
| // issue `#492` — predictive scheduling: add predictive_cycles to extension_policies | |
| `ALTER TABLE extension_policies ADD COLUMN predictive_cycles INTEGER NOT NULL DEFAULT 0`, | |
| ]; | |
| for (const sql of migrations) { | |
| try { | |
| db.exec(sql); | |
| } catch (err: unknown) { | |
| const msg = err instanceof Error ? err.message : String(err); | |
| if (!/duplicate column name|already exists/i.test(msg)) { | |
| throw err; | |
| } | |
| } | |
| } |
🧰 Tools
🪛 OpenGrep (1.26.0)
[ERROR] 92-92: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.
(coderabbit.command-injection.exec-js)
🤖 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/database.ts` around lines 88 - 93, Replace the blanket catch around
the migration loop in the database initialization flow with handling that
ignores only the expected “column already exists” condition and rethrows or
surfaces all other migration errors. Ensure failures in adding predictive_cycles
cannot be silently hidden before upsertExtensionPolicy relies on the column.
Comment on lines
+247
to
301
| /** Number of daemon cycles ahead to project TTL crossing. 0 = disabled (default). */ | ||
| predictive_cycles?: number; | ||
| }): void { | ||
| db.prepare(` | ||
| INSERT INTO extension_policies (contract_id, enabled, target_ttl_ledgers, extend_when_below_ledgers, keypair_public, keypair_source) | ||
| VALUES (@contract_id, @enabled, @target_ttl_ledgers, @extend_when_below_ledgers, @keypair_public, @keypair_source) | ||
| ON CONFLICT(contract_id) DO UPDATE SET | ||
| enabled = @enabled, | ||
| target_ttl_ledgers = @target_ttl_ledgers, | ||
| extend_when_below_ledgers = @extend_when_below_ledgers, | ||
| keypair_public = @keypair_public, | ||
| keypair_source = @keypair_source | ||
| `).run({ | ||
| contract_id: policy.contract_id, | ||
| enabled: policy.enabled !== false ? 1 : 0, | ||
| target_ttl_ledgers: policy.target_ttl_ledgers, | ||
| extend_when_below_ledgers: policy.extend_when_below_ledgers, | ||
| keypair_public: policy.keypair_public ?? null, | ||
| keypair_source: policy.keypair_source ?? null, | ||
| }); | ||
| // Detect whether the predictive_cycles column exists (added via live migration). | ||
| // Databases loaded directly from schema.sql (e.g. in some tests) may not have it yet. | ||
| const hasPredictiveCycles = (() => { | ||
| try { | ||
| const info = db.prepare("PRAGMA table_info(extension_policies)").all() as Array<{ name: string }>; | ||
| return info.some(col => col.name === "predictive_cycles"); | ||
| } catch { | ||
| return false; | ||
| } | ||
| })(); | ||
|
|
||
| if (hasPredictiveCycles) { | ||
| db.prepare(` | ||
| INSERT INTO extension_policies (contract_id, enabled, target_ttl_ledgers, extend_when_below_ledgers, keypair_public, keypair_source, predictive_cycles) | ||
| VALUES (@contract_id, @enabled, @target_ttl_ledgers, @extend_when_below_ledgers, @keypair_public, @keypair_source, @predictive_cycles) | ||
| ON CONFLICT(contract_id) DO UPDATE SET | ||
| enabled = @enabled, | ||
| target_ttl_ledgers = @target_ttl_ledgers, | ||
| extend_when_below_ledgers = @extend_when_below_ledgers, | ||
| keypair_public = @keypair_public, | ||
| keypair_source = @keypair_source, | ||
| predictive_cycles = @predictive_cycles | ||
| `).run({ | ||
| contract_id: policy.contract_id, | ||
| enabled: policy.enabled !== false ? 1 : 0, | ||
| target_ttl_ledgers: policy.target_ttl_ledgers, | ||
| extend_when_below_ledgers: policy.extend_when_below_ledgers, | ||
| keypair_public: policy.keypair_public ?? null, | ||
| keypair_source: policy.keypair_source ?? null, | ||
| predictive_cycles: policy.predictive_cycles ?? 0, | ||
| }); | ||
| } else { | ||
| // Fallback for databases that don't have the predictive_cycles column yet. | ||
| db.prepare(` | ||
| INSERT INTO extension_policies (contract_id, enabled, target_ttl_ledgers, extend_when_below_ledgers, keypair_public, keypair_source) | ||
| VALUES (@contract_id, @enabled, @target_ttl_ledgers, @extend_when_below_ledgers, @keypair_public, @keypair_source) | ||
| ON CONFLICT(contract_id) DO UPDATE SET | ||
| enabled = @enabled, | ||
| target_ttl_ledgers = @target_ttl_ledgers, | ||
| extend_when_below_ledgers = @extend_when_below_ledgers, | ||
| keypair_public = @keypair_public, | ||
| keypair_source = @keypair_source | ||
| `).run({ | ||
| contract_id: policy.contract_id, | ||
| enabled: policy.enabled !== false ? 1 : 0, | ||
| target_ttl_ledgers: policy.target_ttl_ledgers, | ||
| extend_when_below_ledgers: policy.extend_when_below_ledgers, | ||
| keypair_public: policy.keypair_public ?? null, | ||
| keypair_source: policy.keypair_source ?? null, | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Partial upsertExtensionPolicy calls silently reset predictive_cycles to 0.
predictive_cycles is optional in the input type, but the SQL always writes predictive_cycles = @predictive_cycles`` (defaulted to 0 when omitted) on `ON CONFLICT`. Any caller that upserts a policy without re-specifying `predictive_cycles` (e.g. the disable-policy call in `guard.ts`: `upsertExtensionPolicy(db, { contract_id: contractId, enabled: false, target_ttl_ledgers: targetTTL, extend_when_below_ledgers: threshold })`) will silently wipe a previously configured predictive setting back to disabled. Contrast with `insertContract`'s `poll_interval_seconds`, which uses `COALESCE(excluded.x, contracts.x)` to preserve existing values when omitted — the same pattern should apply here.
🛠️ Proposed fix — preserve existing predictive_cycles when omitted
if (hasPredictiveCycles) {
db.prepare(`
INSERT INTO extension_policies (contract_id, enabled, target_ttl_ledgers, extend_when_below_ledgers, keypair_public, keypair_source, predictive_cycles)
- VALUES (`@contract_id`, `@enabled`, `@target_ttl_ledgers`, `@extend_when_below_ledgers`, `@keypair_public`, `@keypair_source`, `@predictive_cycles`)
+ VALUES (`@contract_id`, `@enabled`, `@target_ttl_ledgers`, `@extend_when_below_ledgers`, `@keypair_public`, `@keypair_source`, COALESCE(`@predictive_cycles`, 0))
ON CONFLICT(contract_id) DO UPDATE SET
enabled = `@enabled`,
target_ttl_ledgers = `@target_ttl_ledgers`,
extend_when_below_ledgers = `@extend_when_below_ledgers`,
keypair_public = `@keypair_public`,
keypair_source = `@keypair_source`,
- predictive_cycles = `@predictive_cycles`
+ predictive_cycles = COALESCE(`@predictive_cycles`, extension_policies.predictive_cycles)
`).run({
contract_id: policy.contract_id,
enabled: policy.enabled !== false ? 1 : 0,
target_ttl_ledgers: policy.target_ttl_ledgers,
extend_when_below_ledgers: policy.extend_when_below_ledgers,
keypair_public: policy.keypair_public ?? null,
keypair_source: policy.keypair_source ?? null,
- predictive_cycles: policy.predictive_cycles ?? 0,
+ predictive_cycles: policy.predictive_cycles ?? null,
});
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| /** Number of daemon cycles ahead to project TTL crossing. 0 = disabled (default). */ | |
| predictive_cycles?: number; | |
| }): void { | |
| db.prepare(` | |
| INSERT INTO extension_policies (contract_id, enabled, target_ttl_ledgers, extend_when_below_ledgers, keypair_public, keypair_source) | |
| VALUES (@contract_id, @enabled, @target_ttl_ledgers, @extend_when_below_ledgers, @keypair_public, @keypair_source) | |
| ON CONFLICT(contract_id) DO UPDATE SET | |
| enabled = @enabled, | |
| target_ttl_ledgers = @target_ttl_ledgers, | |
| extend_when_below_ledgers = @extend_when_below_ledgers, | |
| keypair_public = @keypair_public, | |
| keypair_source = @keypair_source | |
| `).run({ | |
| contract_id: policy.contract_id, | |
| enabled: policy.enabled !== false ? 1 : 0, | |
| target_ttl_ledgers: policy.target_ttl_ledgers, | |
| extend_when_below_ledgers: policy.extend_when_below_ledgers, | |
| keypair_public: policy.keypair_public ?? null, | |
| keypair_source: policy.keypair_source ?? null, | |
| }); | |
| // Detect whether the predictive_cycles column exists (added via live migration). | |
| // Databases loaded directly from schema.sql (e.g. in some tests) may not have it yet. | |
| const hasPredictiveCycles = (() => { | |
| try { | |
| const info = db.prepare("PRAGMA table_info(extension_policies)").all() as Array<{ name: string }>; | |
| return info.some(col => col.name === "predictive_cycles"); | |
| } catch { | |
| return false; | |
| } | |
| })(); | |
| if (hasPredictiveCycles) { | |
| db.prepare(` | |
| INSERT INTO extension_policies (contract_id, enabled, target_ttl_ledgers, extend_when_below_ledgers, keypair_public, keypair_source, predictive_cycles) | |
| VALUES (@contract_id, @enabled, @target_ttl_ledgers, @extend_when_below_ledgers, @keypair_public, @keypair_source, @predictive_cycles) | |
| ON CONFLICT(contract_id) DO UPDATE SET | |
| enabled = @enabled, | |
| target_ttl_ledgers = @target_ttl_ledgers, | |
| extend_when_below_ledgers = @extend_when_below_ledgers, | |
| keypair_public = @keypair_public, | |
| keypair_source = @keypair_source, | |
| predictive_cycles = @predictive_cycles | |
| `).run({ | |
| contract_id: policy.contract_id, | |
| enabled: policy.enabled !== false ? 1 : 0, | |
| target_ttl_ledgers: policy.target_ttl_ledgers, | |
| extend_when_below_ledgers: policy.extend_when_below_ledgers, | |
| keypair_public: policy.keypair_public ?? null, | |
| keypair_source: policy.keypair_source ?? null, | |
| predictive_cycles: policy.predictive_cycles ?? 0, | |
| }); | |
| } else { | |
| // Fallback for databases that don't have the predictive_cycles column yet. | |
| db.prepare(` | |
| INSERT INTO extension_policies (contract_id, enabled, target_ttl_ledgers, extend_when_below_ledgers, keypair_public, keypair_source) | |
| VALUES (@contract_id, @enabled, @target_ttl_ledgers, @extend_when_below_ledgers, @keypair_public, @keypair_source) | |
| ON CONFLICT(contract_id) DO UPDATE SET | |
| enabled = @enabled, | |
| target_ttl_ledgers = @target_ttl_ledgers, | |
| extend_when_below_ledgers = @extend_when_below_ledgers, | |
| keypair_public = @keypair_public, | |
| keypair_source = @keypair_source | |
| `).run({ | |
| contract_id: policy.contract_id, | |
| enabled: policy.enabled !== false ? 1 : 0, | |
| target_ttl_ledgers: policy.target_ttl_ledgers, | |
| extend_when_below_ledgers: policy.extend_when_below_ledgers, | |
| keypair_public: policy.keypair_public ?? null, | |
| keypair_source: policy.keypair_source ?? null, | |
| }); | |
| } | |
| } | |
| /** Number of daemon cycles ahead to project TTL crossing. 0 = disabled (default). */ | |
| predictive_cycles?: number; | |
| }): void { | |
| // Detect whether the predictive_cycles column exists (added via live migration). | |
| // Databases loaded directly from schema.sql (e.g. in some tests) may not have it yet. | |
| const hasPredictiveCycles = (() => { | |
| try { | |
| const info = db.prepare("PRAGMA table_info(extension_policies)").all() as Array<{ name: string }>; | |
| return info.some(col => col.name === "predictive_cycles"); | |
| } catch { | |
| return false; | |
| } | |
| })(); | |
| if (hasPredictiveCycles) { | |
| db.prepare(` | |
| INSERT INTO extension_policies (contract_id, enabled, target_ttl_ledgers, extend_when_below_ledgers, keypair_public, keypair_source, predictive_cycles) | |
| VALUES (`@contract_id`, `@enabled`, `@target_ttl_ledgers`, `@extend_when_below_ledgers`, `@keypair_public`, `@keypair_source`, COALESCE(`@predictive_cycles`, 0)) | |
| ON CONFLICT(contract_id) DO UPDATE SET | |
| enabled = `@enabled`, | |
| target_ttl_ledgers = `@target_ttl_ledgers`, | |
| extend_when_below_ledgers = `@extend_when_below_ledgers`, | |
| keypair_public = `@keypair_public`, | |
| keypair_source = `@keypair_source`, | |
| predictive_cycles = COALESCE(`@predictive_cycles`, extension_policies.predictive_cycles) | |
| `).run({ | |
| contract_id: policy.contract_id, | |
| enabled: policy.enabled !== false ? 1 : 0, | |
| target_ttl_ledgers: policy.target_ttl_ledgers, | |
| extend_when_below_ledgers: policy.extend_when_below_ledgers, | |
| keypair_public: policy.keypair_public ?? null, | |
| keypair_source: policy.keypair_source ?? null, | |
| predictive_cycles: policy.predictive_cycles ?? null, | |
| }); | |
| } else { | |
| // Fallback for databases that don't have the predictive_cycles column yet. | |
| db.prepare(` | |
| INSERT INTO extension_policies (contract_id, enabled, target_ttl_ledgers, extend_when_below_ledgers, keypair_public, keypair_source) | |
| VALUES (`@contract_id`, `@enabled`, `@target_ttl_ledgers`, `@extend_when_below_ledgers`, `@keypair_public`, `@keypair_source`) | |
| ON CONFLICT(contract_id) DO UPDATE SET | |
| enabled = `@enabled`, | |
| target_ttl_ledgers = `@target_ttl_ledgers`, | |
| extend_when_below_ledgers = `@extend_when_below_ledgers`, | |
| keypair_public = `@keypair_public`, | |
| keypair_source = `@keypair_source` | |
| `).run({ | |
| contract_id: policy.contract_id, | |
| enabled: policy.enabled !== false ? 1 : 0, | |
| target_ttl_ledgers: policy.target_ttl_ledgers, | |
| extend_when_below_ledgers: policy.extend_when_below_ledgers, | |
| keypair_public: policy.keypair_public ?? null, | |
| keypair_source: policy.keypair_source ?? null, | |
| }); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/db/repositories.ts` around lines 247 - 301, Update upsertExtensionPolicy
so an omitted policy.predictive_cycles preserves the existing database value
during ON CONFLICT updates instead of defaulting to 0. Adjust the
predictive_cycles assignment and bound value in the hasPredictiveCycles branch,
using the same COALESCE-style existing-value preservation pattern as
insertContract; retain 0 only for genuinely new rows.
| @@ -1,4 +1,4 @@ | |||
| import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; | |||
| import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; | |||
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd -t f 'extension.test.ts' -x sh -c 'head -c3 "$1" | xxd' _ {}
rg -n $'\ufeff|\ufffd' tests/core/extension.test.tsRepository: AbdulmalikAlayande/sorokeep
Length of output: 681
Remove the UTF-8 BOM and replace corrupted em dashes.
tests/core/extension.test.ts starts with a BOM, and lines 1162, 1262, 1390, 1503, and 1531 contain replacement characters where em dashes were lost. Re-save the file as UTF-8 without BOM.
Also applies to: 1162, 1262, 1390, 1503, 1531
🤖 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/extension.test.ts` at line 1, Remove the UTF-8 BOM from the
beginning of tests/core/extension.test.ts and replace the corrupted characters
at the specified lines with proper em dashes. Re-save the file using UTF-8
encoding without a BOM, preserving all other content.
Comment on lines
+11
to
+15
| // Per-entry-type policy helpers (added by the sibling per-entry-type policy | ||
| // issue in this phase). These imports will cause a compile-time failure until | ||
| // that feature lands — that is intentional TDD behaviour. | ||
| upsertEntryTypePolicy, | ||
| getEffectivePolicyForEntry, |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP 'export function (upsertEntryTypePolicy|getEffectivePolicyForEntry)\b' --type=ts
rg -nP 'entry_type_policies|entryTypePolic' --type=ts -g '!tests/**' -C2Repository: AbdulmalikAlayande/sorokeep
Length of output: 165
🏁 Script executed:
#!/bin/bash
set -e
printf 'Tracked files matching extension.repos/test:\n'
git ls-files | rg '(^|/)extension\.(test\.)?ts$|(^|/)repositories\.(ts|tsx)$|src/db/repositories|tests/core/extension' || true
printf '\nImport references in tests/core/extension.test.ts if it exists:\n'
if git ls-files | grep -q '^tests/core/extension.test.ts$'; then
sed -n '1,35p' tests/core/extension.test.ts | cat -n
printf '\nRelevant export/type declarations in src/db/repositories.ts if it exists:\n'
if git ls-files | grep -q '^src/db/repositories.ts$'; then
rg -n 'upsertEntryTypePolicy|getEffectivePolicyForEntry|entryTypePolicy|entry_type_policy|EntryTypePolicy|entry_type_policies|entryTypePolic' src/db/repositories.ts -C 3 || true
else
printf 'src/db/repositories.ts not found\n'
fi
else
printf 'tests/core/extension.test.ts not found\n'
fi
printf '\nSearch for symbols with relaxed grep:\n'
rg -n -i 'upsertEntryTypePolicy|getEffectivePolicyForEntry|entryTypePolicy|EntryTypePolicy|entry_type_policy' . -g '*.ts' -g '!tests/**' || trueRepository: AbdulmalikAlayande/sorokeep
Length of output: 2058
Gate the intentional red imports before failing extension.test.ts.
upsertEntryTypePolicy and getEffectivePolicyForEntry are not exported from src/db/repositories.js, so this module import will fail before any existing extension tests run. Wrap the pending suites behind describe.skip/describe.todo with a dynamic guard, or land both helpers in src/db/repositories.ts.
🤖 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/extension.test.ts` around lines 11 - 15, Prevent the imports of
upsertEntryTypePolicy and getEffectivePolicyForEntry in extension.test.ts from
causing the existing extension tests to fail before execution. Either gate the
pending per-entry-type policy suites with describe.skip/describe.todo and load
those helpers dynamically, or export both helpers from the repositories module
while preserving the current tests.
Comment on lines
+1053
to
+1056
| describe("Policy precedence interaction", () => { | ||
|
|
||
| // -- RPC setup helper ------------------------------------------------- | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
No coverage for the predictive path added in this cohort.
This file is the designated test surface for src/core/extension.ts, yet the new suites exercise only entry-type policy precedence. The predictive eligibility branch (predictiveOpts.predictiveCycles vs policy.predictive_cycles, ledgersPerCycle horizon, projection within/outside horizon) is untested here. Want me to draft those cases?
🤖 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/extension.test.ts` around lines 1053 - 1056, Add tests in the
“Policy precedence interaction” suite covering the predictive eligibility branch
in src/core/extension.ts: precedence between predictiveOpts.predictiveCycles and
policy.predictive_cycles, the ledgersPerCycle horizon, and projections both
within and outside that horizon. Use the existing RPC setup helpers and
assertions in this test file, preserving current entry-type precedence coverage.
Comment on lines
+1459
to
+1479
| const instanceCall = mockSubmitExtension.mock.calls.find( | ||
| (call: unknown[]) => | ||
| Array.isArray(call[0]) && | ||
| (call[0] as string[]).includes("instance-cross-type-key") && | ||
| !(call[0] as string[]).includes("wasm-cross-type-key"), | ||
| ); | ||
| if (instanceCall) { | ||
| expect(instanceCall[1]).toBe(75000); | ||
| expect(instanceCall[1]).not.toBe(300000); | ||
| } | ||
|
|
||
| // wasm entry must use the OVERRIDE target (300000) | ||
| const wasmCall = mockSubmitExtension.mock.calls.find( | ||
| (call: unknown[]) => | ||
| Array.isArray(call[0]) && | ||
| (call[0] as string[]).includes("wasm-cross-type-key") && | ||
| !(call[0] as string[]).includes("instance-cross-type-key"), | ||
| ); | ||
| if (wasmCall) { | ||
| expect(wasmCall[1]).toBe(300000); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Cross-type isolation assertions are vacuous — this test cannot fail.
Both expectations sit inside if (instanceCall) / if (wasmCall). runAutoExtensions batches all of a contract's eligible entry keys into a single extendEntries call, so no call matches "includes instance key AND excludes wasm key"; both finds return undefined, both blocks are skipped, and the test passes green while asserting nothing. Assert the calls exist.
💚 Proposed fix
- if (instanceCall) {
- expect(instanceCall[1]).toBe(75000);
- expect(instanceCall[1]).not.toBe(300000);
- }
+ expect(instanceCall).toBeDefined();
+ expect(instanceCall![1]).toBe(75000);
@@
- if (wasmCall) {
- expect(wasmCall[1]).toBe(300000);
- }
+ expect(wasmCall).toBeDefined();
+ expect(wasmCall![1]).toBe(300000);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const instanceCall = mockSubmitExtension.mock.calls.find( | |
| (call: unknown[]) => | |
| Array.isArray(call[0]) && | |
| (call[0] as string[]).includes("instance-cross-type-key") && | |
| !(call[0] as string[]).includes("wasm-cross-type-key"), | |
| ); | |
| if (instanceCall) { | |
| expect(instanceCall[1]).toBe(75000); | |
| expect(instanceCall[1]).not.toBe(300000); | |
| } | |
| // wasm entry must use the OVERRIDE target (300000) | |
| const wasmCall = mockSubmitExtension.mock.calls.find( | |
| (call: unknown[]) => | |
| Array.isArray(call[0]) && | |
| (call[0] as string[]).includes("wasm-cross-type-key") && | |
| !(call[0] as string[]).includes("instance-cross-type-key"), | |
| ); | |
| if (wasmCall) { | |
| expect(wasmCall[1]).toBe(300000); | |
| } | |
| const instanceCall = mockSubmitExtension.mock.calls.find( | |
| (call: unknown[]) => | |
| Array.isArray(call[0]) && | |
| (call[0] as string[]).includes("instance-cross-type-key") && | |
| !(call[0] as string[]).includes("wasm-cross-type-key"), | |
| ); | |
| expect(instanceCall).toBeDefined(); | |
| expect(instanceCall![1]).toBe(75000); | |
| expect(instanceCall![1]).not.toBe(300000); | |
| // wasm entry must use the OVERRIDE target (300000) | |
| const wasmCall = mockSubmitExtension.mock.calls.find( | |
| (call: unknown[]) => | |
| Array.isArray(call[0]) && | |
| (call[0] as string[]).includes("wasm-cross-type-key") && | |
| !(call[0] as string[]).includes("instance-cross-type-key"), | |
| ); | |
| expect(wasmCall).toBeDefined(); | |
| expect(wasmCall![1]).toBe(300000); |
🤖 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/extension.test.ts` around lines 1459 - 1479, Make the cross-type
isolation assertions in the test unconditional by asserting that both
instanceCall and wasmCall are defined before checking their target values.
Preserve the existing key-filtering predicates and expected 75000/300000
targets, but remove the conditional guards so missing calls fail the test.
Comment on lines
+178
to
+225
| 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
Test doesn't actually verify predictive triggering via runAutoExtensions.
This describe block's title and the file's docstring (line 7: "Predictive mode triggers extension BEFORE threshold is actually crossed") claim to cover early-trigger behavior, but runAutoExtensions is mocked at the top of the file, and the single test here only checks that predictive_cycles persists on the policy row via getExtensionPolicy — it never invokes runAutoExtensions or asserts an extension actually fires early. The unused mockSubmitExtension/mockGetEntryTTLsExt/mockGetCurrentLedgerExt/mockSimulateExtensionExt declarations (lines 183-186) suggest a fuller test was intended but not completed. The predictive-eligibility branch in runAutoExtensions (the core acceptance criterion for issue #492) is therefore untested in this integration suite.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/core/predictive_integration.test.ts` around lines 178 - 225, The
predictive-mode test must exercise the real runAutoExtensions implementation
rather than only verifying policy persistence. Replace the top-level mock setup
for this sub-describe with an isolated import/mock configuration that uses the
real runAutoExtensions and the declared RPC mocks, invoke it with an entry still
above extend_when_below_ledgers, and assert that predictive_cycles causes an
extension to be submitted before the reactive threshold; remove any unused mock
declarations or wire them into the scenario.
|
| 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.
AbdulmalikAlayande
force-pushed
the
main
branch
from
August 2, 2026 04:34
43ad363 to
8692701
Compare
Collaborator
Author
|
Seems #491 has been resolved
|
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
18 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.
Closes #505
Summary
Adds comprehensive test coverage for the policy precedence rules that govern
which extension policy wins when a contract has both a contract-level default
and per-entry-type overrides simultaneously.
Only
tests/core/extension.test.tswas modified, as scoped by the issue.What was added
Section 5 —
getEffectivePolicyForEntryunit tests (11 tests)Direct unit tests of the resolution helper, covering every cell of the
precedence matrix in isolation (no RPC, no auto-extension loop):
undefinedundefinedundefinedPlus cross-type isolation: wasm override does not bleed into instance, instance
override does not bleed into wasm, and all four entry types resolve their own
overrides independently on the same contract.
Section 6 —
Policy precedence interactionintegration tests (10 tests)End-to-end tests through
runAutoExtensions, verifying that the correcttarget_ttl_ledgersreaches the RPC call for each matrix cell and thatdisabled policies at any level never submit a transaction.
Test results