Skip to content

Measure SessionStart memory budgets on the injected clock - #887

Merged
Inok merged 3 commits into
mainfrom
pavel/gh-885-budget-clock
Sep 11, 2026
Merged

Inok merged 3 commits into
mainfrom
pavel/gh-885-budget-clock

Conversation

@Inok

@Inok Inok commented Sep 11, 2026 •

Copy link
Copy Markdown
Member

Closes #885 — AI-2701

What & why

SessionStartMemoryLeaseStore took a TimeProvider and used it for lease expiry, but every
budget in the subsystem was measured with static Stopwatch — so half of it answered to an
injected clock and half to the process, and a test could fake one while being timed by the other.
TimeProvider carries the monotonic timestamp pair, so the fix keeps Stopwatch semantics rather
than moving elapsed time onto a wall clock.

A budget is only real if the thing it bounds observes the same clock, so both context lanes arm
their expiry on the injected one too, and the hook's bounded wait uses the clock its Remaining
was measured on. ServiceVerify already measured everything through its injected clock; its
suites were the only thing still handing it a real one.

Where to look

Two cases deliberately keep the process clock, and say so at the call site: their losers contend
for the store's file lock, whose retry loop waits on the same clock it measures — a clock nothing
advances never leaves it.

The lease store gains no parameter. Every hook call site already held clock.Time, so the
orchestrator and scope resolver take it without any new dependency flowing.

Verification

Two pins, each checked by mutation. Stopping the budget from running down fails only
A_fetch_that_outlives_its_budget_is_dropped_rather_than_injected; reverting the orchestrator to
the static Stopwatch does not even compile (CS9113 is an error here). Re-arming the lane's
expiry on a real timer fails only Advancing_past_the_budget_abandons_an_in_flight_fetch, at its
30s ceiling — its budget is ten minutes, so nothing but the injected clock can end that fetch.

Suite Before After
SessionStartMemoryFoundationTests 40 tests, real budgets 42 tests, 183ms
ServiceVerify* (Install, Start, Replace) 45 real forward budgets 97 tests, no wall-clock budget

Full CLI unit suite: 4003 passed, 19 skipped, 0 failed.

RepositoryDetection keeps its own Stopwatch seam and is left alone — the scope resolver that
reaches it is stubbed out in every test here, so threading a clock through it belongs with work
that actually exercises that path.

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Measure SessionStart budgets with the injected TimeProvider

🐞 Bug fix 🧪 Tests 🕐 20-40 Minutes

Grey Divider

AI Description

• Routes injected clocks through every SessionStart memory budget measurement.
• Preserves monotonic timing while making memory and ServiceVerify tests deterministic.
• Retains process clocks where file-lock contention requires time to advance.
Diagram

graph TD
  T["Injected Clock"] --> H["Harness Hooks"] --> S["Hook Support"] --> R["Scope Resolver"]
  H --> O["Memory Orchestrator"] --> L["Lease Store"]
  S --> P["Context Provider"]
  O --> P
Loading
High-Level Assessment

The PR’s approach is appropriate: TimeProvider already exposes monotonic timestamp measurement and clock-aware delays, so propagating the existing hook clock keeps lease expiry and all budget calculations coherent. Continuing with static Stopwatch would preserve the split-clock defect, while introducing another timing abstraction would duplicate TimeProvider without improving behavior. Explicitly retaining TimeProvider.System for lock-contention tests is necessary because a stopped fake clock cannot complete the measured retry delay.

Files changed (17) +156 / -116

Bug fix (13) +40 / -39
AntigravityHookCommand.csPass the Antigravity clock into memory processing +2/-2

Pass the Antigravity clock into memory processing

• Supplies the hook’s TimeProvider to composite provider construction and SessionStart memory orchestration, aligning all Antigravity memory budgets.

src/Capacitor.Cli/Commands/Harness/AntigravityHookCommand.cs

ClaudeHookCommand.csInject Claude’s clock into resolver and orchestrator +2/-2

Inject Claude’s clock into resolver and orchestrator

• Constructs the scope resolver and memory orchestrator with Claude’s existing hook clock so their budgets use the same monotonic source as lease expiry.

src/Capacitor.Cli/Commands/Harness/ClaudeHookCommand.cs

CodexHookCommand.csPass the Codex clock through memory orchestration +2/-2

Pass the Codex clock through memory orchestration

• Provides the injected Codex clock to composite provider construction and the memory orchestrator.

src/Capacitor.Cli/Commands/Harness/CodexHookCommand.cs

CopilotHookCommand.csPass the Copilot clock through memory orchestration +2/-2

Pass the Copilot clock through memory orchestration

• Provides the injected Copilot clock to scope resolution and orchestration through the shared hook support path.

src/Capacitor.Cli/Commands/Harness/CopilotHookCommand.cs

CursorHookCommand.csPass the Cursor clock through memory orchestration +2/-2

Pass the Cursor clock through memory orchestration

• Uses Cursor’s hook clock for the composite provider and orchestrator while preserving ownership of the caller-supplied HTTP client.

src/Capacitor.Cli/Commands/Harness/CursorHookCommand.cs

GeminiHookCommand.csPass the Gemini clock through memory orchestration +2/-2

Pass the Gemini clock through memory orchestration

• Supplies Gemini’s injected TimeProvider to the provider factory and memory orchestrator.

src/Capacitor.Cli/Commands/Harness/GeminiHookCommand.cs

KiroHookCommand.csPass the Kiro clock through memory orchestration +2/-2

Pass the Kiro clock through memory orchestration

• Routes Kiro’s hook clock into scope resolution and orchestration so repeated-callback memory work shares one budget clock.

src/Capacitor.Cli/Commands/Harness/KiroHookCommand.cs

OpenCodeHookCommand.csPass the OpenCode clock through memory orchestration +2/-2

Pass the OpenCode clock through memory orchestration

• Provides OpenCode’s injected clock to the shared provider and memory orchestrator.

src/Capacitor.Cli/Commands/Harness/OpenCodeHookCommand.cs

PiHookCommand.csPass the Pi clock through memory orchestration +2/-2

Pass the Pi clock through memory orchestration

• Supplies Pi’s hook clock to composite provider construction and SessionStart memory orchestration.

src/Capacitor.Cli/Commands/Harness/PiHookCommand.cs

SessionStartMemoryHookSupport.csConstruct scope resolvers with the hook clock +2/-1

Construct scope resolvers with the hook clock

• Adds a TimeProvider parameter to CompositeProvider and uses it when creating the default SessionStart memory scope resolver.

src/Capacitor.Cli/SessionStartMemory/SessionStartMemoryHookSupport.cs

SessionStartMemoryLeaseStore.csMeasure lease-store budgets with TimeProvider +14/-15

Measure lease-store budgets with TimeProvider

• Replaces static Stopwatch timestamps throughout acquisition, mutation, sweeping, and capacity checks with the store’s injected TimeProvider. Lock retry delays now also use that provider, keeping waiting and elapsed-time measurement on the same clock.

src/Capacitor.Cli/SessionStartMemory/SessionStartMemoryLeaseStore.cs

SessionStartMemoryOrchestrator.csMeasure orchestration budgets with the injected clock +3/-2

Measure orchestration budgets with the injected clock

• Adds TimeProvider to the orchestrator and uses its monotonic timestamps to calculate the remaining fetch and commit budget.

src/Capacitor.Cli/SessionStartMemory/SessionStartMemoryOrchestrator.cs

SessionStartMemoryScopeResolver.csMeasure scope resolution with the injected clock +3/-3

Measure scope resolution with the injected clock

• Accepts TimeProvider in the resolver constructor and uses it for remaining-budget calculations instead of static Stopwatch calls.

src/Capacitor.Cli/SessionStartMemory/SessionStartMemoryScopeResolver.cs

Tests (4) +116 / -77
ServiceVerifyInstallTests.csStop install verification tests consuming real budgets +27/-23

Stop install verification tests consuming real budgets

• Introduces a stopped FakeTimeProvider and uses it for install and replacement scenarios that resolve on their first probe. This prevents runner stalls from consuming ServiceVerify’s real forward budget.

test/Capacitor.Cli.Tests.Unit/Services/ServiceVerifyInstallTests.cs

ServiceVerifyReplaceTests.csUse a stopped clock in replacement verification tests +12/-8

Use a stopped clock in replacement verification tests

• Replaces system clocks with a documented stopped FakeTimeProvider in immediate-probe replacement scenarios, removing wall-clock budget sensitivity.

test/Capacitor.Cli.Tests.Unit/Services/ServiceVerifyReplaceTests.cs

ServiceVerifyStartTests.csUse a stopped clock in start verification tests +18/-14

Use a stopped clock in start verification tests

• Runs immediate-probe start verification cases against a stopped FakeTimeProvider so scheduler delays cannot exhaust their forward budgets.

test/Capacitor.Cli.Tests.Unit/Services/ServiceVerifyStartTests.cs

SessionStartMemoryFoundationTests.csVerify memory budgets against a shared fake clock +59/-32

Verify memory budgets against a shared fake clock

• Adds a helper that builds the lease store and orchestrator around one FakeTimeProvider, migrates foundation tests away from real budget timing, and proves an over-budget fetch is dropped. Contended lock tests explicitly retain the process clock because stopped-time delays cannot complete.

test/Capacitor.Cli.Tests.Unit/SessionStartMemory/SessionStartMemoryFoundationTests.cs

@qodo-code-review

qodo-code-review Bot commented Sep 11, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Virtual-clock hooks wait for wall time ✓ Resolved 🐞 Bug ☼ Reliability
Description
GetFragmentAsync calculates Remaining() with its injected TimeProvider, but
CompositeProvider forwards that clock only to SessionStartMemoryScopeResolver, both context
providers still use CancelAfter(request.Budget), hook waiters use provider-less
WaitAsync(remaining), and repository detection enforces independent Stopwatch-based command
limits. When a non-system clock advances during scope discovery or an HTTP fetch, the orchestrator's
logical deadline expires while the underlying operation and final wait remain pending until their
real-time timers expire, affecting every caller or test harness that relies on the injected-clock
path.
Code

src/Capacitor.Cli/SessionStartMemory/SessionStartMemoryOrchestrator.cs[R31-33]

+        var started = time.GetTimestamp();
        TimeSpan Remaining() {
-            var value = request.Budget - System.Diagnostics.Stopwatch.GetElapsedTime(started);
+            var value = request.Budget - time.GetElapsedTime(started);
Relevance

●●● Strong

Accepted timeout-consistency findings show this team fixes budget propagation and wall-clock waits,
especially in hook paths.

PR-#397
PR-#790

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The changed construction and orchestration paths show that elapsed time and the remaining budget are
measured with the injected clock, while that clock is forwarded only to the scope resolver and the
orchestrator directly awaits provider work. Both provider implementations still create cancellation
through CancelAfter(TimeSpan), the common and Claude hook paths still call the provider-less
WaitAsync(TimeSpan) overload, and repository detection starts its own Stopwatch and supplies
real-duration command caps; together these citations show that advancing the injected clock cannot
release those waits, even though HookBudget already exposes provider-aware timeout primitives used
elsewhere.

src/Capacitor.Cli/SessionStartMemory/SessionStartMemoryOrchestrator.cs[31-58]
src/Capacitor.Cli/SessionStartMemory/SessionStartMemoryContextProvider.cs[11-18]
src/Capacitor.Cli/SessionStartMemory/SessionStartCompositeContextProvider.cs[25-51]
src/Capacitor.Cli/SessionStartMemory/SessionStartMemoryHookSupport.cs[52-60]
src/Capacitor.Cli/Commands/Harness/ClaudeHookCommand.cs[1147-1152]
src/Capacitor.Cli/Commands/HookBudget.cs[25-32]
src/Capacitor.Cli/SessionStartMemory/SessionStartMemoryHookSupport.cs[29-38]
src/Capacitor.Cli/SessionStartMemory/SessionStartMemoryOrchestrator.cs[31-46]
src/Capacitor.Cli/SessionStartMemory/SessionStartCompositeContextProvider.cs[25-36]
src/Capacitor.Cli/SessionStartMemory/SessionStartMemoryScopeResolver.cs[5-23]
src/Capacitor.Cli/RepositoryDetection.cs[126-161]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The injected `TimeProvider` reaches orchestration budget calculations but not all cancellation timers, final hook waits, or repository-detection deadlines that enforce those budgets. Advancing a fake clock can therefore exhaust the logical deadline without stopping in-flight scope discovery or HTTP work, leaving completion dependent on real time.

## Fix Focus Areas
- src/Capacitor.Cli/SessionStartMemory/SessionStartMemoryOrchestrator.cs[31-33]
- src/Capacitor.Cli/SessionStartMemory/SessionStartMemoryHookSupport.cs[29-38]
- src/Capacitor.Cli/Commands/Harness/ClaudeHookCommand.cs[1133-1137]
- src/Capacitor.Cli/SessionStartMemory/SessionStartMemoryContextProvider.cs[11-18]
- src/Capacitor.Cli/SessionStartMemory/SessionStartCompositeContextProvider.cs[25-36]
- src/Capacitor.Cli/SessionStartMemory/SessionStartMemoryScopeResolver.cs[5-23]
- src/Capacitor.Cli/RepositoryDetection.cs[126-161]

## Recommended Fix
Thread the same injected `TimeProvider` into both context providers and construct provider-aware budget cancellation linked to the request token. Use the provider-aware `Task.WaitAsync` overload in the shared and Claude bounded-wait helpers, and extend repository-detection deadline plumbing to use the same provider or bound and cancel that work with a provider-aware timeout. Add tests proving that advancing `FakeTimeProvider` cancels a blocked scope probe and a blocked HTTP fetch without waiting for real time, so scope work, HTTP cancellation, repository detection, orchestration, and the hook's final wait all observe one clock.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context sources
✅ Compliance rules (platform): 64 rules
✅ Cross-repo context — repo relationships
Review mode: 🧠 Deep: This behavioral timing change spans multiple production paths and 17 files, with many independent call-site and concurrency/timeout interactions that could hide subtle defects.

Grey Divider

Tip of the day
💡 Did you know, you can switch off images and animations for a plain-text comment

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

@Inok
Inok requested a review from alexeyzimarev September 11, 2026 11:01
@Inok
Inok added this pull request to stack #888 September 11, 2026 11:02
@Inok
Inok force-pushed the pavel/gh-885-budget-clock branch from 4e30c0c to 5e5b26e Compare September 11, 2026 11:11
@Inok
Inok force-pushed the pavel/gh-885-budget-clock branch from 5e5b26e to 3e6e491 Compare September 11, 2026 11:15
@Inok
Inok force-pushed the pavel/gh-885-budget-clock branch from 3e6e491 to c13d785 Compare September 11, 2026 11:40
@Inok
Inok disabled the stack merge September 11, 2026 12:15
@Inok
Inok disabled the stack merge September 11, 2026 12:17
Base automatically changed from pavel/gh-870-injected-machine-auth to main September 11, 2026 12:17
Inok and others added 3 commits September 11, 2026 14:17
Lease expiry was already injectable while every budget stayed on the process
clock, so a test could fake one and still be timed by the other. TimeProvider
carries the monotonic pair, so elapsed time does not move to a wall clock.

The two contended cases keep the process clock: their losers wait on the
store's file lock, and a clock nothing advances never leaves that retry loop.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ServiceVerify already measured through its injected clock; only the suites
handed it the process one, so a runner stalled between entry and the first
probe spent the whole 20s budget and the install failed as a timeout.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The budget arithmetic moved to the injected clock while the cancellation
enforcing it stayed on a real timer, so a frozen clock bounded nothing and the
fetch ran until wall time caught up. Both lanes and the bounded hook wait now
arm against the clock the remaining was measured on.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@Inok
Inok force-pushed the pavel/gh-885-budget-clock branch from c13d785 to 0cb5083 Compare September 11, 2026 12:17
@Inok
Inok merged commit 4a3382f into main Sep 11, 2026
7 of 8 checks passed
@Inok
Inok deleted the pavel/gh-885-budget-clock branch September 11, 2026 12:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Measure SessionStart memory budgets on the injected clock

2 participants