fix: add run store retention

[JAIVIGNESH2002]

Summary

• Add TTL and max-size retention to in-memory baseline and upgrade run stores.
• Evict completed runs on store reads/writes so command output does not remain in memory indefinitely.
• Keep running runs available while pruning oldest completed runs first.

Agent Changes

• src/lib/baseline-run-store.ts: adds TTL, max-size retention, stale-running timeout, and active-run admission control while preserving fresh running records for polling.
• src/lib/upgrade-run-store.ts: applies the same retention and active-run policy for upgrade verification runs.
• src/lib/run-store-retention.ts: centralizes strict positive integer env parsing for retention settings.

Verification

• npm run lint passed
• npm run typecheck passed
• npm run test passed: 20 test files, 130 tests
• npm run build passed

Remaining Risk

• This is still an in-memory store; retention bounds memory lifetime, but cross-process durability remains future work.

[qodo-code-review]

PR Summary by Qodo

Bound in-memory run stores with configurable retention

🐞 Bug fix 🧪 Tests ⚙️ Configuration changes 🕐 20-40 Minutes

AI Description

• Adds configurable TTL and capacity limits to both in-memory run stores.
• Evicts expired or oldest completed runs while preserving active runs.
• Covers time-based and size-based retention with deterministic timer tests.

Diagram

graph TD
  A["Store access"] --> B["Baseline store"] --> D["Retention pruning"] --> E{"Completed run?"} -->|Yes| F{"Expired or excess?"} -->|Yes| G["Evict oldest"]
  A --> C["Upgrade store"] --> D
  E -->|No| H["Keep run"]
  F -->|No| H

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Shared retained-map utility

• ➕ Eliminates duplicated environment parsing and pruning logic.
• ➕ Keeps retention semantics consistent across future run stores.
• ➖ Introduces a generic abstraction for only two small stores.
• ➖ Requires careful typing around each store's status and timestamp fields.

2. Durable external run store

• ➕ Supports cross-process polling and restart durability.
• ➕ Can enforce retention centrally with database expiration policies.
• ➖ Adds infrastructure, serialization, and operational complexity.
• ➖ Substantially exceeds the scope of bounding current process memory.

Recommendation: Keep the lazy, access-triggered in-memory pruning for this fix: it bounds completed-run lifetime without timers or infrastructure and preserves running work. Extract a shared utility if another store adopts the policy, and pursue durable storage separately when cross-process continuity is required.

Files changed (4) +208 / -18

Bug fix (2) +115 / -12

baseline-run-store.tsAdd retention pruning to baseline runs +56/-5

Add retention pruning to baseline runs

• Tracks each record's last update and prunes completed runs on reads and writes. Retention uses configurable TTL and maximum-count limits with one-hour and 100-run defaults while exempting running records.

src/lib/baseline-run-store.ts

upgrade-run-store.tsAdd retention pruning to upgrade runs +59/-7

Add retention pruning to upgrade runs

• Records update timestamps across run creation, completion, and pull-request updates. Prunes completed upgrade records on store reads and writes using configurable TTL and capacity limits while preserving running runs.

src/lib/upgrade-run-store.ts

Tests (2) +93 / -6

baseline-run-store.test.tsCover baseline run TTL and capacity eviction +41/-6

Cover baseline run TTL and capacity eviction

• Adds deterministic tests for expiration and oldest-completed eviction, including environment cleanup between tests. Extends the repository inspection fixture to produce unsupported Yarn runs that complete immediately.

src/lib/baseline-run-store.test.ts

upgrade-run-store.test.tsCover upgrade run TTL and capacity eviction +52/-0

Cover upgrade run TTL and capacity eviction

• Adds fake-timer coverage proving expired completed upgrades and the oldest completed run are evicted. Restores stubbed environment variables after each test.

src/lib/upgrade-run-store.test.ts

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

1. Running runs bypass size cap ✓ Resolved 🐞 Bug ☼ Reliability

Description

Both new pruners count running records toward map size but only allow completed records to be
evicted, so concurrent or stalled runs can grow either map beyond UPGRADEPILOT_MAX_RUNS without
bound. Because starts store a running record before launching unawaited verification, an unresolved
verification makes this a concrete memory-retention path that defeats the max-size safeguard.

Code

src/lib/baseline-run-store.ts[R182-185]

+    .filter((record) => record.status === "completed")
+    .sort((left, right) => left.updatedAtMs - right.updatedAtMs);
+
+  while (baselineRuns.size > maxRuns && completedRuns.length > 0) {

Relevance

●●● Strong

The proposed size cap fails its stated purpose when unresolved running records exhaust eviction
candidates.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

Baseline starts insert running records and launch completion without awaiting it; the tests
demonstrate that verification may remain unresolved. Both pruners derive eviction candidates
exclusively from completed records and stop once that list is empty, so their module-level maps can
exceed the configured limit indefinitely.

src/lib/baseline-run-store.ts[64-84]
src/lib/baseline-run-store.test.ts[24-35]
src/lib/baseline-run-store.ts[175-190]
src/lib/upgrade-run-store.ts[160-197]
src/lib/upgrade-run-store.ts[404-419]

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 max-size pruning policy cannot enforce `UPGRADEPILOT_MAX_RUNS` when records are running, allowing stalled or highly concurrent runs to accumulate indefinitely.

## Issue Context
Running runs must remain pollable, so do not silently evict active records. Add admission control, a separate bounded active-run policy, or a reliable stale-running timeout, and apply the same policy to both stores.

## Fix Focus Areas
- src/lib/baseline-run-store.ts[162-191]
- src/lib/upgrade-run-store.ts[391-420]

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

---

2. Agent change summary missing ✗ Dismissed 📘 Rule violation ⚙ Maintainability

Description

Application code in both run stores is modified, but the PR description has no clearly labeled
agent-change section that enumerates those components and their purposes. This prevents reviewers
from identifying the application changes attributed to the agent as required.

Code

src/lib/upgrade-run-store.ts[R391-393]

+function setUpgradeRunRecord(record: UpgradeRunRecord) {
+  upgradeRuns.set(record.id, record);
+  pruneUpgradeRuns();

Relevance

●●● Strong

The stated repository rule explicitly requires an agent-change summary for modified application
code.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

Rule 2919025 requires an explicit agent-change summary whenever application code is modified. The
cited production modules contain newly added retention implementation, while the supplied PR
description contains only generic Summary, Verification, and Remaining Risk sections and no
agent-change section enumerating the modified components.

Rule 2919025: Document agent-made application code changes in pull request description
src/lib/baseline-run-store.ts[162-204]
src/lib/upgrade-run-store.ts[391-433]

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 PR description lacks a clearly labeled `Agent Changes` section for the modified application modules.

## Issue Context
Add a non-empty section that explicitly lists both run-store modules and briefly states that each now applies TTL and maximum-size retention while preserving running records.

## Fix Focus Areas
- src/lib/baseline-run-store.ts[162-204]
- src/lib/upgrade-run-store.ts[391-433]

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

---

3. Full quality gate not rerun ✗ Dismissed 📘 Rule violation ▣ Testability

Description

The PR verification lists only targeted tests and typechecking, but the documented full quality gate
also requires npm run lint, the full npm run test, and npm run build. The change therefore
lacks evidence that all required verification commands were rerun after these application-code
modifications.

Code

src/lib/baseline-run-store.ts[R162-164]

+function setBaselineRunRecord(record: BaselineRunRecord) {
+  baselineRuns.set(record.id, record);
+  pruneBaselineRuns();

Relevance

●●● Strong

Explicit workflow requires full lint, test, and build verification after application changes.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

Rule 2918973 requires all relevant configured verification commands to be rerun after code changes.
The repository's contribution workflow defines a four-command full quality gate at
CONTRIBUTING.md[10-17], while the cited changed application code implements the new retention
behavior and the supplied PR description documents neither lint, full-suite, nor build execution.

Rule 2918973: Re-run automated verification commands after modifying code under review
CONTRIBUTING.md[10-17]
src/lib/baseline-run-store.ts[162-204]

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 PR does not document execution of the repository's complete required quality gate after the run-store changes.

## Issue Context
`CONTRIBUTING.md` requires `npm run lint`, `npm run typecheck`, `npm run test`, and `npm run build`; the PR verification currently names only targeted tests and typechecking. Run every required command against the latest branch state and update the PR verification section with the results.

## Fix Focus Areas
- CONTRIBUTING.md[10-17]
- src/lib/baseline-run-store.ts[162-204]
- src/lib/upgrade-run-store.ts[391-433]

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

---

4. Malformed limits are accepted ✓ Resolved 🐞 Bug ☼ Reliability

Description

readPositiveIntegerEnv uses parseInt, so malformed values such as 10foo, 1000ms, or 1.5
are silently applied as 10, 1000, or 1 instead of falling back. A configuration typo can therefore
unexpectedly evict runs early or retain far more data than intended in both stores.

Code

src/lib/baseline-run-store.ts[R201-203]

+  const parsed = Number.parseInt(rawValue, 10);
+
+  return Number.isInteger(parsed) && parsed > 0 ? parsed : fallback;

Relevance

●●● Strong

Malformed numeric prefixes are accepted by parseInt, creating a deterministic
configuration-validation bug.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

Both newly added helpers call Number.parseInt(rawValue, 10) and validate only the parsed result,
not whether the entire source string was consumed; JavaScript parseInt therefore accepts a valid
numeric prefix followed by invalid characters or a fractional suffix.

src/lib/baseline-run-store.ts[194-203]
src/lib/upgrade-run-store.ts[423-432]

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 retention environment parser accepts numeric prefixes of otherwise invalid values, silently applying unintended limits.

## Issue Context
Require the entire trimmed environment value to represent a positive base-10 integer before converting it; otherwise use the fallback (or fail configuration explicitly). Keep behavior consistent in both stores, preferably through one shared helper.

## Fix Focus Areas
- src/lib/baseline-run-store.ts[194-203]
- src/lib/upgrade-run-store.ts[423-432]

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

---

Context sources

✅ Compliance rules (platform): 40 rules

Review mode: ⚖️ Balanced: This changes runtime retention and eviction behavior across two asynchronous in-memory stores, with multiple lifecycle paths and configuration semantics that warrant a complete single-pass review.

Tip of the day

💡 Did you know, you can enable the Remediation agent and Qodo fixes findings in a dedicated fix PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗