Wave 3: runtime package - #3
Conversation
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 252 |
| Duplication | 2 |
AI Reviewer: first review requested successfully. AI can make mistakes. Always validate suggestions.
TIP This summary will be updated as you push new changes.
There was a problem hiding this comment.
Pull Request Overview
The @sverka/runtime package successfully implements the core execution engine features, including topological sorting, resource pooling, and retry logic. However, the PR is currently not up to standards due to critical issues in the Scheduler implementation.
A high-severity logic bug was identified in the launchReady method where the internal cursor can jump past pending operations, likely causing plan execution to hang or terminate prematurely. Furthermore, the execute method is a high-complexity 'God Method' (complexity 40) that lacks re-entrancy protection, creating risks for race conditions and undefined behavior if an instance is reused. These structural concerns, alongside non-alignment with the implementation plan regarding error codes and significant code duplication in cancellation logic, must be addressed before merging.
About this PR
- The
Schedulerclass exhibits significant complexity and unsafe state-sharing. The use of shared internal properties likecancelledand theResourcePoolacrossexecute()calls without re-entrancy checks makes the class unsafe for concurrent use. A systemic refactor to decompose the 'God Method' orchestration into discrete, testable private methods is required to ensure long-term maintainability.
Test suggestions
- Linear topological execution (a -> b -> c)
- Diamond topological execution (a -> {b, c} -> d)
- Enforcement of
maxConcurrentlimits - Resource pooling for CPU and Memory limits
-
SchedulerError(INSUFFICIENT_RESOURCES) when an operation exceeds total pool capacity - Cancellation of transitive dependents on fatal failure
-
continueOnErrorallowing independent branches to proceed - Cache hit skipping execution and marking
fromCache: true - State persistence and skipping completed operations on resume
- Re-running operations marked 'running' in persisted state upon resume
- Executor routing based on
canExecutecriteria - Handling of operations with no matching executor (NO_EXECUTOR)
- Detection of cycles in the Plan DAG (CYCLE_DETECTED)
- Retry policy application with backoff delays
- Retry specifically on timeout errors when configured
- Collection of logs and artifacts from multiple operations
TIP Improve review quality by adding custom instructions
TIP How was this review? Give us feedback
MergerNeeds Review PR exceeds the merge-gate context budget (69228 tokens); escalating to a human reviewer. Commit |
100dcb3 to
caa6e03
Compare
🤖 CodeAnt AI — Review Status
|
📝 WalkthroughSummary by CodeRabbit
WalkthroughChangesRuntime scheduler
Repository tooling
Estimated code review effort: 5 (Critical) | ~90 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant Scheduler
participant CacheBackend
participant Executor
participant StateStore
Client->>Scheduler: execute(plan)
Scheduler->>StateStore: load(planId)
Scheduler->>CacheBackend: get(cacheKey)
alt cache miss
Scheduler->>Executor: execute(request)
Executor-->>Scheduler: ExecuteResult
Scheduler->>CacheBackend: put(cacheEntry)
else cache hit
CacheBackend-->>Scheduler: cached outputs
end
Scheduler->>StateStore: save(execution state)
Scheduler-->>Client: ExecutionResult
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
8eca5d4 to
31204dd
Compare
31204dd to
8eca5d4
Compare
|
Skipping CodeAnt AI review — this PR changes more than 100 files, which usually means a migration, codemod, or vendored drop. Line-level review on diffs this large produces duplicate findings on the same rewrite pattern and drowns out anything that actually matters. If you still want a review, comment |
ee02376 to
4379e03
Compare
There was a problem hiding this comment.
Actionable comments posted: 25
🤖 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 @.evidence/2026-08-09/sv-wtn/gates-green/claim.json:
- Line 4: Update the claim in the recorded evidence to state only the observed
lint failure from the IR configuration and the command executed in packages/ir;
remove the unsupported pre-existing repo-wide and non-regression causal
assertions. If retaining that causal conclusion, add baseline and candidate
commit identifiers plus equivalent lint results for both revisions and relevant
packages.
In @.gc:
- Line 1: Replace the tracked .gc symlink target with a supported
repository-relative locator; do not retain the machine-specific /home/pepl path.
If a local runtime link is still required, keep it untracked and document the
setup step.
In @.opencode/plugins/gascity.js:
- Around line 146-163: Scope cached prime and injected session context by
sessionID: update buildPrefix and its callers to accept sessionID, store prime
values per session, and pass providerSessionEnv(sessionID) to every stateful
command, including handoff --auto. Ensure both prompt hooks cannot inject the
same prefix twice by retaining one injection path or making the operation
idempotent.
In @.opencode/skills/.gc-skill-ownership.json:
- Line 1: Replace the developer-local absolute paths in the targets map with
stable repository-relative targets, or generate the cache links during
bootstrap. Update the ownership-map generation or bootstrap logic associated
with the targets configuration so the resulting links resolve in clean checkouts
and across machines without embedding cache hashes.
In `@packages/runtime/src/__tests__/cache.test.ts`:
- Around line 71-75: Extend the cache-hit test around the existing assertions
for outcome "a" to verify that cache.restored is called. Keep the current
success, fromCache, and skipped-execution assertions unchanged, and assert the
restored output behavior exposed by cache.restored.
In `@packages/runtime/src/__tests__/parse.test.ts`:
- Around line 19-22: Extend the parseCpu tests to reject hexadecimal and
exponent-form strings such as "0x10" and "1e3". Update parseCpu to validate the
input against the supported decimal-string grammar before calling Number, while
preserving existing RangeError behavior for invalid values.
- Around line 37-41: Extend the malformed-input coverage around parseMemory with
an oversized digits-only value that converts to Infinity, and update parseMemory
to reject non-finite byte counts before returning. Preserve existing RangeError
behavior for all invalid memory strings and ensure ResourcePool.fromConfig
cannot receive an unlimited capacity from parsing.
In `@packages/runtime/src/cache.ts`:
- Around line 6-21: Preserve the full cache identity in the put contract by
passing the complete CacheKey, including inputs, alongside the CacheEntry or
embedding it in CacheEntry; update CacheBackend implementations, scheduler call
sites, and cache tests to use the composite identity consistently so get, store,
and put associate matching inputs.
In `@packages/runtime/src/internal/parse.ts`:
- Around line 7-13: Update parseCpu to validate the input string against the
documented positive decimal format before calling Number, rejecting hexadecimal,
exponent notation, surrounding whitespace, Infinity, and other malformed values
while accepting values such as "2", "0.5", and "1.5". Preserve the existing
RangeError behavior and positive-value requirement.
- Around line 15-42: Align resource zero-value handling across isValidCpuString,
parseCpu, isValidMemoryString, and parseMemory: reject the exact zero value
consistently while preserving the existing memory grammar for binary-suffixed
values and rejecting decimal/SI forms. Ensure validated plans cannot accept
resources that runtime parsing rejects.
In `@packages/runtime/src/internal/retry.ts`:
- Line 79: Replace substring-based timeout detection in both retry call sites
around isTimeout with an explicit executor-provided timeout discriminator, such
as ExecuteResult.status or timedOut, and update the executor contract
accordingly. Ensure timeout outcomes are consistently marked regardless of
error-message wording, while ordinary command output containing “timeout”
remains a normal failure; use a dedicated TimeoutError for the throw path.
- Around line 62-113: Replace the positional arguments of executeAttempt and
handleExecutorThrow with a shared parameter object, using RetryContext or a
local AttemptContext to group the stable retry values attempt, maxAttempts,
retryOn, backoffSeconds, and start while preserving the current per-attempt
behavior. Update both call sites and destructuring accordingly, and reuse the
shared shouldRetry/backoff handling rather than duplicating that sequence.
In `@packages/runtime/src/internal/scheduler-helpers.ts`:
- Around line 93-104: Replace the 10 ms polling loop in cancellableSleep with a
single delay raced against the scheduler’s cancellation signal, preserving
immediate resolution when cancellation occurs or the timeout elapses. Prefer
wiring an AbortController through Scheduler.cancel and updating retry helper
call sites to use its signal, removing the isCancelled closure where applicable.
- Around line 55-65: Update shouldRetry to classify the outcome as "timeout"
when isTimeout is true, otherwise "failure", then check retryOn membership for
only that category while preserving the attempt < maxAttempts limit.
In `@packages/runtime/src/internal/state-persist.ts`:
- Around line 41-72: Update clearPersistedState and persistState to accept and
use the injectable logger from SchedulerConfig instead of console.warn, passing
it through the Scheduler call sites with the existing failure context. Also
propagate persistState save failures to the caller through the scheduler result
or an explicit callback, and update Scheduler.persist so callers can detect when
resume state was not saved.
In `@packages/runtime/src/internal/topo.ts`:
- Line 121: Update the cycle construction in the topological traversal so a
self-loop returns the node only once rather than duplicating it; adjust the
logic around stack slicing and `next` in the cycle-detection path. Add or update
the corresponding assertion in the topo tests to expect `["a"]` for a self-loop.
- Around line 55-57: Validate every dependency referenced by the plan before
scheduling begins, rather than ignoring unknown IDs in topoSort. Update
Scheduler.execute to reject plans with an UNKNOWN_DEPENDENCY error before
invoking any executor, and add a Scheduler test confirming the error is returned
and no executor runs.
In `@packages/runtime/src/scheduler.ts`:
- Around line 148-155: Update the runError handling in execute() and the
corresponding path around the method at lines 250–263 so it marks execution as
cancelled, awaits Promise.allSettled(ctx.inflight), and only then rethrows
ctx.runError. Ensure every thrown runError path drains in-flight operations
before returning control to the caller.
- Around line 189-201: Update checkResourceFeasibility so parseCpu and
parseMemory failures for an operation are caught and converted into
SchedulerError instances carrying the offending operationId. Preserve the
existing INSUFFICIENT_RESOURCES error for valid values that exceed pool
capacity, ensuring all plan-level resource faults use SchedulerError rather than
leaking RangeError.
- Around line 157-160: Update the scheduler’s execution lifecycle around
cancel() to own an AbortController, pass its signal through ExecuteRequest into
the executor, and call abort() when cancel() is invoked. Extend ExecuteRequest
and the executor call path so already-running container or remote executions
receive and honor the AbortSignal, while preserving the existing partial-result
behavior and retry cancellation checks.
- Around line 495-513: The resource acquisition flow in runOp/acquireResources
must not consume a concurrency slot while waiting or poll with sleep. Acquire
CPU and memory before adding the operation to ctx.running, and update
ResourcePool.release to notify waiting acquirers so acquireResources waits on
that notification and retries until successful or cancelled, preserving the
existing cancellation outcome.
- Around line 458-478: Update tryCacheHit to distinguish cache get failures from
restore failures: continue returning false when cache lookup fails or misses,
but do not silently fall through after config.cache.restore fails. Propagate the
restore error as an operation failure, or clean all partially restored workspace
paths before returning false so re-execution never uses dirty cache content.
- Around line 83-123: Extend validateConfig to validate config.totalMemory
alongside maxConcurrent and totalCpu, using parseMemory to detect invalid values
and converting any parse failure into SchedulerError with code "INVALID_CONFIG".
Ensure buildPool can continue using parseMemory after validation without
exposing a bare RangeError, while preserving the existing handling for undefined
totalMemory.
- Around line 265-281: Ensure terminal scheduler execution marks all remaining
pending operations as cancelled, including after fatal failures and other
non-cancellation exits. Update the flow around shouldContinue,
markBlockedAsCancelled, cleanupCancelled, and the loop teardown so
markBlockedAsCancelled runs after the loop for every terminal path, before
buildOutcomes and snapshotState consume operation state.
In `@specs/03-runtime/spec.md`:
- Around line 361-365: Update the resource exhaustion rule and all other
scheduler error rules in specs/03-runtime/spec.md to state that SchedulerError
uses code: "SCHEDULER_ERROR" and exposes the specific discriminator as
context.code, including context.code: "INSUFFICIENT_RESOURCES" for insufficient
resources. Ensure consumers are directed to branch on context.code rather than
code.
🪄 Autofix
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: 4e25cc9e-bb38-461b-bab5-cbe749cc17b0
📒 Files selected for processing (44)
.beads/identity.toml.codacy/codacy.yaml.devin/config.json.devin/hide-gascity-session.sh.evidence/2026-08-09/sv-wtn/gates-green/claim.json.gc.opencode/plugins/gascity.js.opencode/skills/.gc-skill-ownership.json.opencode/skills/core.gc-agents.opencode/skills/core.gc-city.opencode/skills/core.gc-dashboard.opencode/skills/core.gc-dispatch.opencode/skills/core.gc-mail.opencode/skills/core.gc-rigs.opencode/skills/core.gc-workengdocs/adr/ADR-007-runtime-scheduler-design.mdengdocs/architecture/wave-03-runtime-plan.mdpackages/runtime/package.jsonpackages/runtime/project.jsonpackages/runtime/src/__tests__/cache.test.tspackages/runtime/src/__tests__/errors.test.tspackages/runtime/src/__tests__/helpers/fixtures.tspackages/runtime/src/__tests__/parse.test.tspackages/runtime/src/__tests__/public-api.test.tspackages/runtime/src/__tests__/resource-limits.test.tspackages/runtime/src/__tests__/resource-pool.test.tspackages/runtime/src/__tests__/retry.test.tspackages/runtime/src/__tests__/scheduler.test.tspackages/runtime/src/__tests__/state-store.test.tspackages/runtime/src/__tests__/topo.test.tspackages/runtime/src/cache.tspackages/runtime/src/errors.tspackages/runtime/src/executor.tspackages/runtime/src/index.tspackages/runtime/src/internal/parse.tspackages/runtime/src/internal/resource-pool.tspackages/runtime/src/internal/retry.tspackages/runtime/src/internal/scheduler-helpers.tspackages/runtime/src/internal/state-persist.tspackages/runtime/src/internal/topo.tspackages/runtime/src/result.tspackages/runtime/src/scheduler.tspackages/runtime/src/state-store.tsspecs/03-runtime/spec.md
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Codacy Static Code Analysis
🧰 Additional context used
🪛 ast-grep (0.45.1)
.opencode/plugins/gascity.js
[warning] 137-137: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFile(tmp, JSON.stringify({ info, messages }, null, 2))
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 16-16: Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { execFile } from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process)
🪛 GitHub Check: SonarCloud Code Analysis
packages/runtime/src/internal/retry.ts
[warning] 92-92: Async function 'handleExecutorThrow' has too many parameters (9). Maximum allowed is 7.
[warning] 62-62: Async function 'executeAttempt' has too many parameters (9). Maximum allowed is 7.
packages/runtime/src/scheduler.ts
[failure] 522-522: Refactor this function to reduce its Cognitive Complexity from 17 to the 15 allowed.
🪛 LanguageTool
engdocs/architecture/wave-03-runtime-plan.md
[style] ~21-~21: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...; optional in config). - CacheBackend interface + CacheKey / CacheEntry (optional i...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
specs/03-runtime/spec.md
[grammar] ~31-~31: Ensure spelling is correct
Context: ...ryPolicy` (maxAttempts, backoffSeconds, retryOn). - Collects logs and artifacts from e...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 markdownlint-cli2 (0.23.2)
engdocs/architecture/wave-03-runtime-plan.md
[warning] 63-63: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 95-95: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 102-102: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 103-103: Ordered list item prefix
Expected: 1; Actual: 4; Style: 1/2/3
(MD029, ol-prefix)
[warning] 105-105: Ordered list item prefix
Expected: 2; Actual: 5; Style: 1/2/3
(MD029, ol-prefix)
[warning] 106-106: Ordered list item prefix
Expected: 3; Actual: 6; Style: 1/2/3
(MD029, ol-prefix)
[warning] 107-107: Ordered list item prefix
Expected: 4; Actual: 7; Style: 1/2/3
(MD029, ol-prefix)
[warning] 108-108: Ordered list item prefix
Expected: 5; Actual: 8; Style: 1/2/3
(MD029, ol-prefix)
[warning] 109-109: Ordered list item prefix
Expected: 6; Actual: 9; Style: 1/2/3
(MD029, ol-prefix)
[warning] 111-111: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 112-112: Ordered list item prefix
Expected: 1; Actual: 10; Style: 1/2/3
(MD029, ol-prefix)
[warning] 115-115: Ordered list item prefix
Expected: 2; Actual: 11; Style: 1/2/3
(MD029, ol-prefix)
[warning] 119-119: Ordered list item prefix
Expected: 3; Actual: 12; Style: 1/2/3
(MD029, ol-prefix)
[warning] 124-124: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 125-125: Ordered list item prefix
Expected: 1; Actual: 13; Style: 1/2/3
(MD029, ol-prefix)
[warning] 128-128: Ordered list item prefix
Expected: 2; Actual: 14; Style: 1/2/3
(MD029, ol-prefix)
[warning] 133-133: Ordered list item prefix
Expected: 3; Actual: 15; Style: 1/2/3
(MD029, ol-prefix)
[warning] 149-149: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 150-150: Ordered list item prefix
Expected: 1; Actual: 16; Style: 1/2/3
(MD029, ol-prefix)
[warning] 153-153: Ordered list item prefix
Expected: 2; Actual: 17; Style: 1/2/3
(MD029, ol-prefix)
[warning] 160-160: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 161-161: Ordered list item prefix
Expected: 1; Actual: 18; Style: 1/2/3
(MD029, ol-prefix)
[warning] 164-164: Ordered list item prefix
Expected: 2; Actual: 19; Style: 1/2/3
(MD029, ol-prefix)
[warning] 168-168: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 169-169: Ordered list item prefix
Expected: 1; Actual: 20; Style: 1/2/3
(MD029, ol-prefix)
[warning] 172-172: Ordered list item prefix
Expected: 2; Actual: 21; Style: 1/2/3
(MD029, ol-prefix)
[warning] 177-177: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 178-178: Ordered list item prefix
Expected: 1; Actual: 22; Style: 1/2/3
(MD029, ol-prefix)
[warning] 180-180: Ordered list item prefix
Expected: 2; Actual: 23; Style: 1/2/3
(MD029, ol-prefix)
[warning] 184-184: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 185-185: Ordered list item prefix
Expected: 1; Actual: 24; Style: 1/2/3
(MD029, ol-prefix)
[warning] 186-186: Ordered list item prefix
Expected: 2; Actual: 25; Style: 1/2/3
(MD029, ol-prefix)
[warning] 187-187: Ordered list item prefix
Expected: 3; Actual: 26; Style: 1/2/3
(MD029, ol-prefix)
specs/03-runtime/spec.md
[warning] 441-441: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
🪛 OpenGrep (1.26.0)
packages/runtime/src/internal/parse.ts
[ERROR] 31-31: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.
(coderabbit.command-injection.exec-js)
🔇 Additional comments (38)
packages/runtime/src/__tests__/errors.test.ts (1)
1-54: LGTM!.opencode/skills/core.gc-agents (1)
1-1: Duplicate of the ownership-map issue.This skill link uses the same non-portable
/home/pepl/.gc/cache/...target. The link-generation mechanism must produce a portable target or regenerate this file during bootstrap..opencode/skills/core.gc-city (1)
1-1: Duplicate of the ownership-map issue.This skill link depends on the developer-local
/home/pepl/.gc/cache/...path and will not resolve in other environments..opencode/skills/core.gc-dashboard (1)
1-1: Duplicate of the ownership-map issue.This skill link depends on the developer-local
/home/pepl/.gc/cache/...path and will not resolve in other environments..opencode/skills/core.gc-dispatch (1)
1-1: Duplicate of the ownership-map issue.This skill link depends on the developer-local
/home/pepl/.gc/cache/...path and will not resolve in other environments..opencode/skills/core.gc-mail (1)
1-1: Duplicate of the ownership-map issue.This skill link depends on the developer-local
/home/pepl/.gc/cache/...path and will not resolve in other environments..opencode/skills/core.gc-rigs (1)
1-1: Duplicate of the ownership-map issue.This skill link depends on the developer-local
/home/pepl/.gc/cache/...path and will not resolve in other environments..opencode/skills/core.gc-work (1)
1-1: Duplicate of the ownership-map issue.This skill link depends on the developer-local
/home/pepl/.gc/cache/...path and will not resolve in other environments..beads/identity.toml (1)
1-5: LGTM!.codacy/codacy.yaml (1)
1-15: LGTM!.devin/config.json (1)
1-15: LGTM!.devin/hide-gascity-session.sh (1)
1-26: LGTM!.opencode/plugins/gascity.js (1)
186-201: 🚀 Performance & ScalabilityKeep the hooks separate.
chat.messageruns after system prompt construction, whileexperimental.chat.system.transformmutates the system prompt for the LLM request. These hooks do not duplicate the prefix in the same system prompt.> Likely an incorrect or invalid review comment.engdocs/adr/ADR-007-runtime-scheduler-design.md (1)
1-91: LGTM!engdocs/architecture/wave-03-runtime-plan.md (1)
1-273: LGTM!packages/runtime/package.json (1)
5-11: LGTM!Also applies to: 21-23
packages/runtime/project.json (1)
11-17: LGTM!packages/runtime/src/internal/topo.ts (1)
1-54: LGTM!Also applies to: 58-120, 122-187
packages/runtime/src/__tests__/parse.test.ts (1)
1-18: LGTM!Also applies to: 23-36, 42-43
packages/runtime/src/__tests__/resource-pool.test.ts (1)
1-46: LGTM!packages/runtime/src/__tests__/helpers/fixtures.ts (1)
1-146: LGTM!packages/runtime/src/errors.ts (1)
18-23: The hardcoded"SCHEDULER_ERROR"code conflicts with the callers.packages/runtime/src/scheduler.tspasses discriminators such asINVALID_CONFIG,CYCLE_DETECTED,INSUFFICIENT_RESOURCES, andNO_EXECUTORinsidecontext.code, which shadows the meaning of the class-levelcode. This matches the earlier review note about accepting the specific code as a constructor parameter.packages/runtime/src/scheduler.ts (3)
75-81:execute()is not reentrant.cancelledandpoolare instance state, and Line 133 resetscancelledon every call. A secondexecute()on the same instance while a run is active clears the cancellation flag of the first run and shares the resource pool. This repeats the earlier review note about adding anactiveguard.
522-552:indexstill skips pending operations, and SonarCloud fails the complexity gate.Line 532 and Line 542 set
ctx.index = i + 1wheni > ctx.index. Every pending operation between the oldctx.indexandiis then never revisited, because the next scan starts after them. This repeats the earlier review note about advancingindexonly past operations that left thependingstate. SonarCloud additionally fails this function for cognitive complexity 17 against the limit of 15; a cursor that advances only while the head entry is terminal removes both problems.Source: Linters/SAST tools
394-414: LGTM!Also applies to: 433-456
packages/runtime/src/executor.ts (1)
6-40: LGTM!packages/runtime/src/state-store.ts (1)
20-24: LGTM!packages/runtime/src/result.ts (1)
1-35: LGTM!packages/runtime/src/internal/resource-pool.ts (1)
8-50: LGTM!packages/runtime/src/internal/scheduler-helpers.ts (1)
5-52: LGTM!packages/runtime/src/internal/retry.ts (1)
24-60: LGTM!packages/runtime/src/internal/state-persist.ts (1)
18-38: LGTM!packages/runtime/src/index.ts (1)
3-12: LGTM!packages/runtime/src/__tests__/public-api.test.ts (1)
2-32: LGTM!packages/runtime/src/__tests__/scheduler.test.ts (1)
1-358: LGTM!packages/runtime/src/__tests__/resource-limits.test.ts (1)
1-124: LGTM!packages/runtime/src/__tests__/retry.test.ts (1)
1-131: LGTM!packages/runtime/src/__tests__/state-store.test.ts (1)
1-194: LGTM!
7b6b618 to
af20326
Compare
af20326 to
02962a0
Compare
Backend-agnostic runtime: Executor interface (canExecute/execute), Scheduler with topological sort, concurrent execution within maxConcurrent + optional CPU/memory resource pool, dependent cancellation on fatal failure, continueOnError, StateStore persistence for resume, CacheBackend reuse, RetryPolicy, log/artifact collection. <details> - 64 vitest tests pass - typecheck clean - build green - fixes sv-iqe: ResourcePool now created when either totalCpu or totalMemory is set (independent optionals per spec), defaulting the unset dimension to Infinity </details> Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
state-store.ts: - Document ReadonlyMap serialization requirement for JSON-backed impls - Document lifecycle: scheduler calls clear() on success, retains on partial scheduler.ts: - Validate maxConcurrent: must be positive finite integer (INVALID_CONFIG) - Validate totalCpu: must be positive finite number when provided - Serialize concurrent persist() calls via promise chain to prevent overlap - Handle cancelled outcomes in aggregate status (partial, not success) when executor returns cancelled without cancel() being called - Call stateStore.clear() after fully successful completion Tests: 64 → 74 (+10: 8 config validation, 1 cancelled aggregate, 1 state retention) Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
- canonical.ts: extract emitScalar/emitArray to reduce emit() cognitive complexity; use String.raw for escape sequences; convert for-loop to for-of; add explicit UTF-16 compare function for keys.sort() - validate.ts: remove unused PlanOperation import; use \d in regex; refactor validatePlan into focused helpers to reduce cognitive complexity - scheduler.ts: extract RunContext and private methods to reduce execute/runOp/launchReady/executeWithRetry cognitive complexity; use optional chaining; extract computeFinalStatus for nested ternary - topo.ts: extract buildEdges to reduce topoSort cognitive complexity - parse.ts: use \d in regex - test files: use toHaveLength assertions; fix pre-existing typecheck errors in state-store.test.ts
- scheduler.ts: extract helpers to internal/scheduler-helpers.ts to reduce file size; further split execute/runOp/launchReady/executeWithRetry into smaller methods (prepareContext, finalizeResult, runOpAttempt, handleOpFailure, tryLaunchNextReady, executeAttempt, handleExecutorThrow) to meet Codacy limits (50 lines/method, complexity 10) - validate.ts: split validateTopLevelFields into validateApiVersion, validateId, validatePlanIdIfShapeOk, checkShapeOk, validateOperationsShape; extract countIds/isDuplicateId from collectDuplicateIds - topo.ts: extract kahnSort/processDependents from topoSort; restructure dfs cycle detection; extract buildDependentMap/bfsDependents/enqueueDependents from dependentsOf
…dules Extract executeWithRetry/buildRequest/executeAttempt/handleExecutorThrow to internal/retry.ts and loadState/persist/clearState to internal/state-persist.ts to reduce scheduler.ts file length below Codacy threshold
Co-Authored-By: Petr Plenkov <petr.plenkov@gmail.com>
…facts and ignore them Co-Authored-By: Petr Plenkov <petr.plenkov@gmail.com>
02962a0 to
949a861
Compare
|



User description
Summary
Test plan
Stacked on #2
Generated with Devin
CodeAnt-AI Description
Add a backend-independent runtime scheduler for reliable plan execution
What Changed
Impact
✅ Concurrent plan execution✅ Resumable interrupted runs✅ Fewer repeated operations through cache reuse✅ Clearer scheduler and executor errors🔄 Retrigger CodeAnt AI Review
💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.