Skip to content

fix(core): bound foreign session timestamps - #3835

Merged
Astro-Han merged 1 commit into
apache:mainfrom
hydraxman:fix/foreign-session-timeclip
Aug 27, 2026
Merged

fix(core): bound foreign session timestamps#3835
Astro-Han merged 1 commit into
apache:mainfrom
hydraxman:fix/foreign-session-timeclip

Conversation

@hydraxman

Copy link
Copy Markdown
Contributor

Summary

Prevent corrupt or future-schema foreign-session timestamps outside ECMAScript's Date range from crashing resume digest rendering. Store normalization now rejects values outside the TimeClip bound, while the prompt renderer independently falls back to updated_at=unknown for an invalid digest.

Fixes #3822

Verification

  • RED: npm --workspace @maka/core test — 656 passed, 2 failed before the production change (out-of-range normalization and render RangeError)
  • npm --workspace @maka/core test — 658 passed
  • npm test with Python 3.11 and canonical macOS TMPDIR — core, storage, MCP, runtime, eval, computer-use, UI, CLI, and desktop passed; runtime-host encountered an unrelated shared-cache ENOTEMPTY
  • HOME=<isolated> npm --workspace @maka/runtime-host test — 1,214 passed
  • npm run typecheck — all workspaces passed
  • npm run lint — passed (2,757 files)
  • npm run format:check — passed (1,652 files)
  • npx knip --workspace apps/desktop — passed
  • npx knip --workspace packages/ui — passed
  • npm run build — passed
  • git diff --check — passed

AI use

Select exactly one:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: OpenAI GPT-5.6 assisted with the implementation and regression tests; the contributor of record retains responsibility for final review and the merge decision.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update on a68af7b: GO — no P0-P3. Adds TimeClip-bounded timestamp handling with dual-layer fail-open, hosted checks none yet — needs CI green.

简体中文该头无阻断,待 CI。

@M4n5ter
M4n5ter force-pushed the fix/foreign-session-timeclip branch from a68af7b to f0942d7 Compare August 26, 2026 08:54

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bounds foreign session timestamps to the ECMAScript TimeClip range so a corrupt store row can't crash the digest render.

Re-reviewed at 4485ba3c9 (force-pushed since my a68af7b19 note). Verified: FOREIGN_SESSION_MAX_EPOCH_MS = 8_640_000_000_000_000 is the correct TimeClip bound, Math.abs(value) <= MAX matches the spec's > 8.64e15 → NaN rule, and :642 is the only new Date() construction in the file — so with it guarded, no finite input can reach toISOString() out of range. The normalizeEpochMs return type was already number | undefined, so returning undefined for out-of-range values is type-safe, and the test asserts the caller's 0 fallback. No P0–P3.

One note, not graded — it's currently unreachable, but it's the seed of the next one. This PR establishes isValidForeignEpochMs as the single validity authority, and normalizeEpochMs now routes through it. parseTimestampMs (:193-200) is the other timestamp entry point in this file and still checks only Number.isFinite, with no upper bound. Today that's harmless because :642 is the only new Date() and it's guarded — nothing reachable can throw.

The reason it's worth one line anyway: #1512 adds toIsoOrUnknown plus two more new Date() call sites fed from parseTimestampMs, at which point a recorded 1e300 throws RangeError and fails the whole handoff. Routing parseTimestampMs through isValidForeignEpochMs here closes that before it opens, and leaves #1512 with one validator to reuse instead of a second one to write.

I've asked #1512 to land after this one for exactly that reason — it currently conflicts with this file and re-introduces the Number.isFinite guard you just proved insufficient.


AI-assisted review: I read the full diff and verified the TimeClip bound, the single new Date() call site, and the remaining bare isFinite checks against this head myself. No tests run. AI review is not independent human review.

简体中文

4485ba3c9 复审(自 a68af7b19 后有 force-push)。TimeClip 上界取值正确,:642 是文件中唯一的 new Date() 且已被保护,因此无可达的 RangeError。无 P0–P3。

一条不定级的提醒:本 PR 把 isValidForeignEpochMs 立为唯一有效性权威,但 parseTimestampMs(:193-200)这条并行入口仍只查 Number.isFinite。当前无害,但 #1512 会在这条路径上新增两个 new Date() 调用点,届时 1e300 会抛 RangeError 并让整个交接失败。顺手接上一行,既提前关闭该问题,也让 #1512 有一个现成校验器可复用。我已要求 #1512 排在本 PR 之后。

@Astro-Han
Astro-Han merged commit a5c2631 into apache:main Aug 27, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/S Under 100 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(core): foreign-session digest render throws RangeError on finite out-of-range timestamps

3 participants