Skip to content

Commit 307573b

Browse files
taofeeq-deruMastra Code (anthropic/claude-opus-4-7)
andauthored
chore(core): address serialize-evented-wflows PR #17836 review follow-ups (#18385)
## Summary Follow-up to #17836. Picks up three reviewer comments that landed after the merge: - **`Mastra` TTL sweep warning** — When the run-scoped workflow TTL sweep evicts a registration that was still live, log a warning with `runId` / `ageMs` / `ttlMs` so abandoned suspended runs are visible in operator logs instead of disappearing silently. (Comment: P3 follow-up.) - **`codec.ts` regex-injection suppression** — The `// lgtm[js/regex-injection]` comment in `decodeRegExpEnvelope` now spells out the three structural gates that justify the suppression — `isEnvelope`'s source-length cap + flag whitelist, the local re-narrowing in `decodeRegExpEnvelope`, and the `try/catch` fallback — plus an explicit "do not 'fix' by escaping" warning. A future reader looking only at that screen no longer has to scroll back up to `isEnvelope` to understand why this is safe. (#17836 (comment)) - **`evented-unix-pubsub` scenario test** — Allocates the `UnixSocketPubSub` socket under a short `/tmp/aim-*` directory instead of `os.tmpdir()`, so the path stays under macOS's 104-byte `sun_path` limit. (Nit follow-up.) The three other reviewer comments (`DefaultStepResult` codec registration, `MAX_REGEXP_SOURCE_LENGTH` constant use, `hydrate-run-scope` skip-list comment, and the `__m_codec__` reservation doc) were already handled in the merged PR. ## Test plan - `pnpm --filter ./packages/core exec vitest run src/mastra/run-scope.test.ts src/loop/test-utils/aimock/scenarios/evented-unix-pubsub.scenario.test.ts src/events/codec` → 70 passed, no type errors - `pnpm --filter ./packages/core check` → clean 🤖 Generated with [Mastra Code](https://mastra.ai) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## ELI5 This PR adds three improvements to Mastra's workflow system: (1) it now tells operators when old workflow runs are automatically cleaned up instead of silently disappearing, (2) it adds better documentation explaining why certain security-risky code patterns are actually safe, and (3) it fixes socket file paths to work properly on macOS where there's a strict limit on how long paths can be. --- ## Changes ### TTL Sweep Logging Enhancement Added warning log messages to `Mastra#sweepStaleRunScopedWorkflows` that are emitted when the TTL sweep evicts stale run-scoped workflow registrations. The warning includes the `runId`, computed `ageMs`, and `ttlMs` value, making abandoned suspended runs visible in operator logs. A corresponding test case was added to verify this logging behavior. ### Regex-Injection Suppression Documentation Expanded the inline documentation in `decodeRegExpEnvelope` within `codec.ts` to explicitly document the three structural safety gates that justify the `// lgtm[js/regex-injection]` suppression: - `isEnvelope`'s source-length cap and flag whitelist - Local re-narrowing of `source` and `flags` within `decodeRegExpEnvelope` - The `try/catch` fallback mechanism The documentation also includes an explicit warning against attempting to fix the issue by escaping characters. ### Unix Socket Path Adjustment Modified the `evented-unix-pubsub` scenario test to allocate Unix sockets under a short `/tmp/aim-*` directory created at test time, instead of using `os.tmpdir()`. This ensures socket paths stay within macOS's 104-byte `sun_path` limit. The test now uses `mkdtempSync` and `rmSync` to manage temporary directories, with cleanup occurring in `afterEach`. ### Changeset Documentation Added a patch-level changeset for `@mastra/core` documenting all three follow-up items. --- ## Testing - 70 tests passed with no type errors - Package check completed cleanly <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Mastra Code (anthropic/claude-opus-4-7) <noreply@mastra.ai>
1 parent 1556acd commit 307573b

5 files changed

Lines changed: 59 additions & 4 deletions

File tree

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
---
2+
'@mastra/core': patch
3+
---
4+
5+
Address post-merge review follow-ups from the evented-workflow serialization work:
6+
7+
- `Mastra` now logs a warning when the TTL sweep evicts a run-scoped workflow that was still registered, surfacing abandoned suspended runs instead of dropping them silently.
8+
- The `RegExp` codec's `// lgtm[js/regex-injection]` suppression now spells out all three structural gates (`isEnvelope` length cap + flag whitelist, local re-narrowing in `decodeRegExpEnvelope`, `try/catch` fallback) at the suppression site so future readers don't reintroduce escaping.
9+
- The `evented-unix-pubsub` scenario test now allocates its Unix socket under a short `/tmp/aim-*` directory instead of `os.tmpdir()` so it stays under the macOS 104-byte `sun_path` limit.

packages/core/src/events/codec/codec.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -147,8 +147,15 @@ function decodeRegExpEnvelope(v: unknown): RegExp {
147147
if (!/^[dgimsuvy]*$/.test(flags)) return /(?:)/;
148148
if (new Set(flags).size !== flags.length) return /(?:)/;
149149
try {
150-
// lgtm[js/regex-injection] -- payload is narrowed (length cap, flag whitelist,
151-
// try/catch); see helper docstring for why escaping `source` would be wrong.
150+
// lgtm[js/regex-injection] -- safe because of three structural gates that
151+
// run before this line: (1) `isEnvelope` rejects payloads whose `source`
152+
// exceeds `MAX_REGEXP_SOURCE_LENGTH` and whose `flags` fall outside the
153+
// spec flag whitelist `[dgimsuvy]` (no duplicates); (2) the local checks
154+
// above re-narrow `source`/`flags` so the constructor input is bounded at
155+
// this single call site; (3) this `try/catch` swallows any constructor
156+
// failure and returns the empty regex. Do NOT "fix" this by escaping
157+
// `source` — see the helper docstring above for why that would break the
158+
// round-trip contract.
152159
return new RegExp(candidate.source, flags);
153160
} catch {
154161
return /(?:)/;

packages/core/src/loop/test-utils/aimock/scenarios/evented-unix-pubsub.scenario.test.ts

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { randomUUID } from 'node:crypto';
2-
import { tmpdir } from 'node:os';
2+
import { mkdtempSync, rmSync } from 'node:fs';
33
import { join } from 'node:path';
44
import { stepCountIs } from '@internal/ai-sdk-v5';
55
import { afterEach, describe, expect, it } from 'vitest';
@@ -24,13 +24,23 @@ import { runLoopScenario, useLoopScenarioAimock } from '../aimock-scenario';
2424
describe('AIMock loop scenario: evented + UnixSocketPubSub', () => {
2525
const getMock = useLoopScenarioAimock();
2626
const pubsubs: UnixSocketPubSub[] = [];
27+
const socketDirs: string[] = [];
2728

2829
afterEach(async () => {
2930
await Promise.allSettled(pubsubs.splice(0).map(p => p.close()));
31+
for (const dir of socketDirs.splice(0)) {
32+
rmSync(dir, { recursive: true, force: true });
33+
}
3034
});
3135

3236
it('round-trips a tool result containing Date/Map/Error across the evented pubsub', async () => {
33-
const socketPath = join(tmpdir(), `aimock-evented-${randomUUID()}.sock`);
37+
// Keep this path short. macOS caps `sun_path` at 104 bytes — the default
38+
// `os.tmpdir()` on macOS (`/var/folders/...`) already burns ~50 chars
39+
// before we add a UUID + suffix, so we anchor under `/tmp` and trim the
40+
// UUID to make the test portable on every host.
41+
const socketDir = mkdtempSync('/tmp/aim-');
42+
socketDirs.push(socketDir);
43+
const socketPath = join(socketDir, `${randomUUID().slice(0, 8)}.sock`);
3444
const pubsub = new UnixSocketPubSub(socketPath);
3545
pubsubs.push(pubsub);
3646

packages/core/src/mastra/index.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2771,6 +2771,15 @@ export class Mastra<
27712771
// time — never parse it back out of the composite key, since callers
27722772
// can pass runIds that contain ':'.
27732773
this.#releaseRunScope(entry.runId);
2774+
// Surface the eviction so operators can investigate long-suspended
2775+
// runs that never resumed. The refcounted lifecycle in
2776+
// `__createRunScope`/`__releaseRunScope` covers the happy path; this
2777+
// branch only fires when a registration was abandoned past the TTL.
2778+
this.#logger.warn('Evicted stale run-scoped workflow after TTL expired', {
2779+
runId: entry.runId,
2780+
ageMs: now - entry.registeredAt,
2781+
ttlMs: Mastra.INTERNAL_WORKFLOW_TTL_MS,
2782+
});
27742783
}
27752784
}
27762785
}

packages/core/src/mastra/run-scope.test.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -216,6 +216,26 @@ describe('Mastra runScope lifecycle', () => {
216216
expect(m.__getRunScope('fresh-run')).toBeDefined();
217217
});
218218

219+
it('emits a warning when the sweep evicts a still-registered run', () => {
220+
const warn = vi.fn();
221+
const m = new Mastra({
222+
logger: { warn, info: vi.fn(), debug: vi.fn(), error: vi.fn(), trackException: vi.fn() } as any,
223+
});
224+
m.__registerInternalWorkflow(makeWorkflow('agentic-loop'), 'abandoned-run');
225+
226+
vi.setSystemTime(Date.now() + Mastra.INTERNAL_WORKFLOW_TTL_MS + 1000);
227+
m.__registerInternalWorkflow(makeWorkflow('agentic-loop'), 'fresh-run');
228+
229+
expect(warn).toHaveBeenCalledWith(
230+
'Evicted stale run-scoped workflow after TTL expired',
231+
expect.objectContaining({
232+
runId: 'abandoned-run',
233+
ttlMs: Mastra.INTERNAL_WORKFLOW_TTL_MS,
234+
ageMs: expect.any(Number),
235+
}),
236+
);
237+
});
238+
219239
it('releases the correct scope when the runId contains a colon', () => {
220240
const m = makeMastra();
221241
// runIds are caller-controlled; nothing prevents a custom id from

0 commit comments

Comments
 (0)