Repository navigation
Optimize telemetry flush startup after package dogfood audit - #84
Merged
Merged
Conversation
- add a cross-process telemetry flush spawn gate so command_start and command_finish do not both pay detached child startup while a flush is already booting - ignore and untrack the metadata collections runtime cache so generated tracker/cache data is not committed - refresh the project pm-changelog package to 2026.5.30 and regenerate CHANGELOG.md through the package-owned path - record pm-qmx3 audit evidence, close/release the concrete audit slice, and preserve pm-3mal for remaining bundling startup work Verification: pm test pm-qmx3 --run --progress; node scripts/release/run-gates.mjs --json --telemetry-mode required --max-sentry-critical 10 --max-sentry-high 20 --max-telemetry-error-rate 10 --max-telemetry-missing-error-rows 0; PM_DOGFOOD_SEMANTIC=1 PM_OLLAMA_MODEL=qwen3-embedding:0.6b node scripts/dogfood-package-first.mjs; pnpm changelog:pm:check; node scripts/check-secrets.mjs; pnpm quality:static; git diff --check
Reviewer's GuideIntroduces a cross-process spawn gate for telemetry flush child processes to avoid redundant detached child startups, wires it into the flush scheduling flow with appropriate lock lifecycle handling and tests, and refreshes the pm-changelog extension/versioning and audit metadata while dropping the generated metadata collections cache from source control. Sequence diagram for telemetry flush spawn gate and process lock interactionsequenceDiagram
participant CliProcess
participant TelemetryRuntime
participant ChildProcess
CliProcess->>TelemetryRuntime: scheduleTelemetryFlush(globalPmRoot, endpoint, retentionDays)
TelemetryRuntime->>TelemetryRuntime: acquireTelemetryFlushSpawnGate(globalPmRoot)
alt spawn gate acquired
TelemetryRuntime->>CliProcess: spawn(process.execPath, [telemetryFlushRunnerPath()])
CliProcess-->>ChildProcess: start telemetry-flush.js (detached)
else spawn gate not acquired
TelemetryRuntime-->>CliProcess: return (skip spawn)
end
ChildProcess->>TelemetryRuntime: flushQueueWithProcessLock(globalPmRoot, endpoint, retentionDays)
TelemetryRuntime->>TelemetryRuntime: releaseTelemetryFlushSpawnGate(globalPmRoot)
TelemetryRuntime->>TelemetryRuntime: acquireTelemetryFlushLock(globalPmRoot)
alt flush lock acquired
TelemetryRuntime-->>ChildProcess: proceed with flush
else flush lock not acquired
TelemetryRuntime-->>ChildProcess: return (another flush active)
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Warning Review limit reached
More reviews will be available in 55 minutes and 10 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThis PR implements a spawn-gate mechanism for telemetry flushing that prevents multiple concurrent "spawn flush" worker scheduling attempts. The core logic uses directory-lock-based synchronous operations in the telemetry runtime, integrates deduplication logic into the scheduler, and includes comprehensive unit tests validating lock freshness, staleness, and multi-process interactions. ChangesTelemetry Spawn Gate Implementation
Sequence DiagramsequenceDiagram
participant Scheduler as scheduleTelemetryFlush
participant SpawnGate as Spawn Gate Lock
participant Filesystem as Filesystem (mkdirSync/statSync/rmSync)
participant Worker as Telemetry Worker
Scheduler->>SpawnGate: acquireTelemetryFlushSpawnGate()
SpawnGate->>Filesystem: statSync(flushSpawnLockPath) to check freshness
alt Lock is fresh (recent)
Filesystem-->>SpawnGate: Lock exists and not stale
SpawnGate-->>Scheduler: Gate is fresh, skip spawn
else Lock is stale or missing
Filesystem-->>SpawnGate: Lock missing or stale
SpawnGate->>Filesystem: mkdirSync(flushSpawnLockPath, {recursive:true})
Filesystem-->>SpawnGate: Lock created
SpawnGate-->>Scheduler: Gate acquired, proceed
Scheduler->>Worker: spawn detached telemetry worker
Worker-->>Scheduler: child process spawning
Scheduler->>SpawnGate: releaseTelemetryFlushSpawnGate()
SpawnGate->>Filesystem: rmSync(flushSpawnLockPath, best-effort)
Filesystem-->>SpawnGate: Lock removed
end
🎯 3 (Moderate) | ⏱️ ~18 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 3 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/core/telemetry/runtime.ts" line_range="1327-1336" />
<code_context>
+ }
+}
+
+function acquireTelemetryFlushSpawnGate(globalPmRoot: string): boolean {
+ if (isFreshDirectoryLock(flushLockPath(globalPmRoot), TELEMETRY_FLUSH_LOCK_STALE_MS)) {
+ return false;
+ }
+
+ const lockPath = flushSpawnLockPath(globalPmRoot);
+ if (isFreshDirectoryLock(lockPath, TELEMETRY_FLUSH_SPAWN_LOCK_STALE_MS)) {
+ return false;
+ }
+ removeDirectoryLockBestEffort(lockPath);
+
+ try {
+ mkdirSync(path.dirname(lockPath), { recursive: true });
+ mkdirSync(lockPath);
+ return true;
+ } catch (error: unknown) {
+ if (typeof error === "object" && error !== null && "code" in error && (error as { code?: unknown }).code === "EEXIST") {
+ return false;
+ }
</code_context>
<issue_to_address>
**issue (bug_risk):** Returning `true` on unexpected `mkdirSync` errors can incorrectly signal that the spawn gate was acquired.
The `catch` block currently returns `true` for any non-`EEXIST` error. If `mkdirSync(lockPath)` fails with `EPERM`, `ENOSPC`, or similar, the caller will think the gate was acquired and may spawn concurrent flush processes without a lock directory, breaking the throttling semantics.
Instead, consider returning `false` (or rethrowing) for unexpected errors so the gate is only treated as acquired when the lock directory is known to exist.
</issue_to_address>
### Comment 2
<location path="tests/unit/telemetry-runtime.spec.ts" line_range="892" />
<code_context>
+ });
+ });
+
+ it("clears a parent-created spawn gate once the flush child enters the process lock", async () => {
+ await withTempGlobalRoot(async (globalRoot) => {
+ await fs.mkdir(path.dirname(telemetryFlushSpawnLockPath(globalRoot)), { recursive: true });
</code_context>
<issue_to_address>
**suggestion (testing):** Add a test for the case where the flush process lock is *not* acquired, to ensure the spawn gate is still released
Currently we only cover the case where the child acquires the lock and the parent-created spawn gate is removed, which indirectly exercises `releaseTelemetryFlushSpawnGate`. We’re missing coverage for when `flushQueueWithProcessLock` fails to acquire the lock (e.g., lock already held): the implementation still calls `releaseTelemetryFlushSpawnGate`, and we should assert that the spawn gate is cleared in that scenario too. You could pre-create a `flush.lock` directory to simulate a held lock, then invoke the same path used by `flushTelemetryQueueNow` and assert that the spawn gate directory is removed even when lock acquisition fails.
</issue_to_address>
### Comment 3
<location path="tests/unit/telemetry-runtime.spec.ts" line_range="37-41" />
<code_context>
return path.join(globalRoot, "runtime", "telemetry", "events.jsonl");
}
+function telemetryFlushLockPath(globalRoot: string): string {
+ return path.join(globalRoot, "runtime", "telemetry", "flush.lock");
+}
+
+function telemetryFlushSpawnLockPath(globalRoot: string): string {
+ return path.join(globalRoot, "runtime", "telemetry", "flush.spawn.lock");
+}
</code_context>
<issue_to_address>
**suggestion:** Avoid duplicating lock-path logic in tests and reuse the runtime’s `_testOnly` helpers instead
Since these paths are already exposed via `_testOnly.flushLockPath` and `_testOnly.flushSpawnLockPath` in the runtime, please call those helpers from the tests instead of redefining the paths here. This avoids duplication and prevents tests from drifting if the directory structure changes.
Suggested implementation:
```typescript
const originalTelemetryIngestKey = process.env.PM_TELEMETRY_INGEST_KEY;
const DAY_MS = 24 * 60 * 60 * 1000;
return path.join(globalRoot, "runtime", "telemetry", "events.jsonl");
}
async function withTempGlobalRoot(run: (globalRoot: string) => Promise<void>): Promise<void> {
```
You’ll also need to update all usages of `telemetryFlushLockPath(globalRoot)` and `telemetryFlushSpawnLockPath(globalRoot)` in this file to call the runtime’s `_testOnly` helpers instead. For example, wherever you currently have:
- `telemetryFlushLockPath(globalRoot)` → replace with something like `runtime._testOnly.flushLockPath(globalRoot)`
- `telemetryFlushSpawnLockPath(globalRoot)` → replace with something like `runtime._testOnly.flushSpawnLockPath(globalRoot)`
Use the actual import / variable name for your runtime object as it is defined in this spec (e.g. `runtime`, `telemetryRuntime`, etc.).
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| }); | ||
| }); | ||
|
|
||
| it("clears a parent-created spawn gate once the flush child enters the process lock", async () => { |
There was a problem hiding this comment.
suggestion (testing): Add a test for the case where the flush process lock is not acquired, to ensure the spawn gate is still released
Currently we only cover the case where the child acquires the lock and the parent-created spawn gate is removed, which indirectly exercises releaseTelemetryFlushSpawnGate. We’re missing coverage for when flushQueueWithProcessLock fails to acquire the lock (e.g., lock already held): the implementation still calls releaseTelemetryFlushSpawnGate, and we should assert that the spawn gate is cleared in that scenario too. You could pre-create a flush.lock directory to simulate a held lock, then invoke the same path used by flushTelemetryQueueNow and assert that the spawn gate directory is removed even when lock acquisition fails.
Comment on lines
+37
to
+41
| function telemetryFlushLockPath(globalRoot: string): string { | ||
| return path.join(globalRoot, "runtime", "telemetry", "flush.lock"); | ||
| } | ||
|
|
||
| function telemetryFlushSpawnLockPath(globalRoot: string): string { |
There was a problem hiding this comment.
suggestion: Avoid duplicating lock-path logic in tests and reuse the runtime’s _testOnly helpers instead
Since these paths are already exposed via _testOnly.flushLockPath and _testOnly.flushSpawnLockPath in the runtime, please call those helpers from the tests instead of redefining the paths here. This avoids duplication and prevents tests from drifting if the directory structure changes.
Suggested implementation:
const originalTelemetryIngestKey = process.env.PM_TELEMETRY_INGEST_KEY;
const DAY_MS = 24 * 60 * 60 * 1000;
return path.join(globalRoot, "runtime", "telemetry", "events.jsonl");
}
async function withTempGlobalRoot(run: (globalRoot: string) => Promise<void>): Promise<void> {You’ll also need to update all usages of telemetryFlushLockPath(globalRoot) and telemetryFlushSpawnLockPath(globalRoot) in this file to call the runtime’s _testOnly helpers instead. For example, wherever you currently have:
telemetryFlushLockPath(globalRoot)→ replace with something likeruntime._testOnly.flushLockPath(globalRoot)telemetryFlushSpawnLockPath(globalRoot)→ replace with something likeruntime._testOnly.flushSpawnLockPath(globalRoot)
Use the actual import / variable name for your runtime object as it is defined in this spec (e.g. runtime, telemetryRuntime, etc.).
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a cross-process telemetry flush spawn gate to optimize CLI startup performance by preventing redundant child process spawns. It also updates the .gitignore to ignore the metadata collections runtime cache and bumps the pm-changelog extension version. Feedback on these changes suggests optimizing the spawn gate implementation to avoid unnecessary synchronous filesystem operations on the CLI hot path.
Comment on lines
+1327
to
+1348
| function acquireTelemetryFlushSpawnGate(globalPmRoot: string): boolean { | ||
| if (isFreshDirectoryLock(flushLockPath(globalPmRoot), TELEMETRY_FLUSH_LOCK_STALE_MS)) { | ||
| return false; | ||
| } | ||
|
|
||
| const lockPath = flushSpawnLockPath(globalPmRoot); | ||
| if (isFreshDirectoryLock(lockPath, TELEMETRY_FLUSH_SPAWN_LOCK_STALE_MS)) { | ||
| return false; | ||
| } | ||
| removeDirectoryLockBestEffort(lockPath); | ||
|
|
||
| try { | ||
| mkdirSync(path.dirname(lockPath), { recursive: true }); | ||
| mkdirSync(lockPath); | ||
| return true; | ||
| } catch (error: unknown) { | ||
| if (typeof error === "object" && error !== null && "code" in error && (error as { code?: unknown }).code === "EEXIST") { | ||
| return false; | ||
| } | ||
| return true; | ||
| } | ||
| } |
There was a problem hiding this comment.
Performance Optimization: Avoid Unnecessary Synchronous I/O on CLI Hot Path
In the current implementation of acquireTelemetryFlushSpawnGate, several synchronous filesystem operations (statSync, rmSync, and mkdirSync with { recursive: true }) are executed on every single command invocation (during both command_start and command_finish telemetry events). Since this is a CLI tool where startup latency is critical, we can optimize this hot path:
- Avoid redundant
rmSynccalls: Only callremoveDirectoryLockBestEffortif the spawn lock directory actually exists and is stale. If it doesn't exist,statSyncwill throw, and we can safely skip thermSynccall. - Avoid eager recursive directory creation: Try creating the lock directory directly first. In 99% of cases, the parent directory (
globalPmRoot/runtime/telemetry) already exists. IfmkdirSyncfails withENOENT, only then recursively create the parent directory and retry.
This optimization significantly reduces the number of synchronous system calls on the hot path of every command execution.
function acquireTelemetryFlushSpawnGate(globalPmRoot: string): boolean {
if (isFreshDirectoryLock(flushLockPath(globalPmRoot), TELEMETRY_FLUSH_LOCK_STALE_MS)) {
return false;
}
const lockPath = flushSpawnLockPath(globalPmRoot);
try {
const stats = statSync(lockPath);
if (Date.now() - stats.mtimeMs < TELEMETRY_FLUSH_SPAWN_LOCK_STALE_MS) {
return false;
}
removeDirectoryLockBestEffort(lockPath);
} catch {
// Lock directory does not exist, no need to remove it.
}
try {
mkdirSync(lockPath);
return true;
} catch (error: unknown) {
if (typeof error === "object" && error !== null && "code" in error) {
const code = (error as { code?: string }).code;
if (code === "EEXIST") {
return false;
}
if (code === "ENOENT") {
try {
mkdirSync(path.dirname(lockPath), { recursive: true });
mkdirSync(lockPath);
return true;
} catch (innerError: unknown) {
if (typeof innerError === "object" && innerError !== null && "code" in innerError && (innerError as { code?: string }).code === "EEXIST") {
return false;
}
}
}
}
return true;
}
}![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.agents/pm/tasks/pm-qmx3.toon (1)
1-57:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse the pm system-of-record workflow instead of directly editing managed
.agents/pmartifacts.This change modifies a file under
.agents/pm/**, which should not be edited directly. Please regenerate/update this task record through the pm workflow/tooling and commit the generated result instead of manual file edits.As per coding guidelines,
.agents/pm/**: "pmis the system of record. Do not edit.agents/pmfiles directly."🤖 Prompt for 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. In @.agents/pm/tasks/pm-qmx3.toon around lines 1 - 57, The task record under .agents/pm was edited directly (task id pm-qmx3) which violates the policy that "pm is the system of record"; revert the manual edits to .agents/pm/tasks/pm-qmx3.toon and regenerate/update this task using the pm system-of-record workflow/tooling (use the pm command or UI that creates/updates task id pm-qmx3) so the managed artifacts (.agents/pm/extensions, .agents/pm/tasks entries, and .gitignore changes) are produced by the pm tool and commit that generated output instead of hand-editing the file.
🧹 Nitpick comments (1)
tests/unit/telemetry-runtime.spec.ts (1)
37-43: 💤 Low valueReuse the exported lock-path helpers instead of redefining them.
_testOnlyalready exposesflushLockPathandflushSpawnLockPath(the sameruntime/telemetry/flush.lock/flush.spawn.lockjoins). Redefining them here duplicates the path contract, so a future change to the runtime layout would silently diverge from these tests.♻️ Drop the local duplicates and use the exported helpers
-function telemetryFlushLockPath(globalRoot: string): string { - return path.join(globalRoot, "runtime", "telemetry", "flush.lock"); -} - -function telemetryFlushSpawnLockPath(globalRoot: string): string { - return path.join(globalRoot, "runtime", "telemetry", "flush.spawn.lock"); -} -Then reference
_testOnly.flushLockPath(globalRoot)/_testOnly.flushSpawnLockPath(globalRoot)in the new tests.🤖 Prompt for 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. In `@tests/unit/telemetry-runtime.spec.ts` around lines 37 - 43, Remove the local helper functions telemetryFlushLockPath and telemetryFlushSpawnLockPath and update tests to call the exported helpers _testOnly.flushLockPath(globalRoot) and _testOnly.flushSpawnLockPath(globalRoot) instead; locate the duplicated functions in telemetry-runtime.spec.ts, delete their definitions, and replace any references to telemetryFlushLockPath(...) / telemetryFlushSpawnLockPath(...) with _testOnly.flushLockPath(globalRoot) / _testOnly.flushSpawnLockPath(globalRoot).
🤖 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 @.agents/pm/chores/pm-3mal.toon:
- Around line 9-19: You edited a protected tracker under the pm system (pm-3mal)
directly; revert those manual edits and re-submit the change through the pm
system-of-record flow instead. Undo the direct changes made to the tracker
metadata in the .agents/pm area, recreate the intended changes as a normal code
commit that updates the implementation in the runtime telemetry code and tests
(target the telemetry runtime change and its test: symbols referencing
scheduleTelemetryFlush / spawn gate logic and the telemetry-runtime.spec tests),
and open the change via the official pm workflow (use the pm tool/CLI to create
the tracker/pr) so the .agents/pm metadata is produced by the system rather than
manual edits. Ensure no further direct edits are made under the protected
.agents/pm namespace.
In @.agents/pm/history/pm-3mal.jsonl:
- Around line 6-11: You directly edited the PM history record "pm-3mal" (the
system-of-record) which is forbidden; revert the changes that modify the pm-3mal
history entry (undo the commit(s) that added/edited metadata/files, tests,
comments, release ops) and regenerate this history through the PM workflow
instead—remove any diffs targeting the pm-3mal history object, then re-run the
official PM recording process (the project’s PM workflow/automation) to produce
the intended metadata/comments/tests so the system-of-record is updated
correctly.
In @.agents/pm/history/pm-th6y.jsonl:
- Line 55: The change directly edits a pm system-of-record file
(.agents/pm/history/pm-th6y.jsonl) which is forbidden; revert this manual edit
and re-apply the update using the pm workflow/tooling that publishes to the
metadata/comments and metadata/updated_at fields (i.e., make the change by
invoking the pm tool that appends the new comment entry under metadata.comments
and updates metadata.updated_at) rather than committing JSON edits by hand so
the pm system remains authoritative.
---
Outside diff comments:
In @.agents/pm/tasks/pm-qmx3.toon:
- Around line 1-57: The task record under .agents/pm was edited directly (task
id pm-qmx3) which violates the policy that "pm is the system of record"; revert
the manual edits to .agents/pm/tasks/pm-qmx3.toon and regenerate/update this
task using the pm system-of-record workflow/tooling (use the pm command or UI
that creates/updates task id pm-qmx3) so the managed artifacts
(.agents/pm/extensions, .agents/pm/tasks entries, and .gitignore changes) are
produced by the pm tool and commit that generated output instead of hand-editing
the file.
---
Nitpick comments:
In `@tests/unit/telemetry-runtime.spec.ts`:
- Around line 37-43: Remove the local helper functions telemetryFlushLockPath
and telemetryFlushSpawnLockPath and update tests to call the exported helpers
_testOnly.flushLockPath(globalRoot) and _testOnly.flushSpawnLockPath(globalRoot)
instead; locate the duplicated functions in telemetry-runtime.spec.ts, delete
their definitions, and replace any references to telemetryFlushLockPath(...) /
telemetryFlushSpawnLockPath(...) with _testOnly.flushLockPath(globalRoot) /
_testOnly.flushSpawnLockPath(globalRoot).
🪄 Autofix (Beta)
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: Repository UI (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: d3721e5c-b90a-4b19-b257-799c68a4a3b8
⛔ Files ignored due to path filters (2)
.agents/pm/extensions/pm-changelog/dist/extension.jsis excluded by!**/dist/**.agents/pm/extensions/pm-changelog/dist/extension.js.mapis excluded by!**/dist/**,!**/*.map
📒 Files selected for processing (16)
.agents/pm/chores/pm-3mal.toon.agents/pm/epics/pm-th6y.toon.agents/pm/extensions/.managed-extensions.json.agents/pm/extensions/pm-changelog/manifest.json.agents/pm/extensions/pm-changelog/package.json.agents/pm/features/pm-59gj.toon.agents/pm/history/pm-3mal.jsonl.agents/pm/history/pm-59gj.jsonl.agents/pm/history/pm-qmx3.jsonl.agents/pm/history/pm-th6y.jsonl.agents/pm/runtime/metadata-cache-collections.json.agents/pm/tasks/pm-qmx3.toon.gitignoreCHANGELOG.mdsrc/core/telemetry/runtime.tstests/unit/telemetry-runtime.spec.ts
Comment on lines
+9
to
+19
| updated_at: "2026-05-30T06:42:12.568Z" | ||
| author: maintainer-agent | ||
| comments[2]{created_at,author,text}: | ||
| comments[3]{created_at,author,text}: | ||
| "2026-05-29T22:59:24.881Z",maintainer-agent,"Addressed the highest-frequency sub-item: scheduleTelemetryFlush now spawns the detached flush child AT MOST ONCE per process (in-process guard). Previously command_start + command_finish each spawned a child (~6.5ms fork/exec each, deduped only by lock AFTER paying Node startup). One child drains the whole queue. DEFERRED with rationale: (a) PM_TOOL_PARAMETERS_SCHEMA lazy-build — it is public SDK surface (sdk/index.ts re-exports it, contract-tested in sdk-index.spec) and the eager build is only ~3ms (~1% of a ~300ms command); churning the public API for 1% is not worth it. (b) completion.ts flag-string constants are not on the bundled CLI hot path. The dominant per-command cost is ESM module-load/bundle, tracked separately under pm-ss1d (esbuild --splitting --packages=external). Keeping open for the bundling lever." | ||
| "2026-05-30T05:07:04.484Z",maintainer-agent,"CORRECTION: reverted the in-process flush spawn-once guard. A flush child boots (~50-100ms) before even a normal command finishes (~300ms), so spawning only at command_start would systematically defer the command_finish event to the next invocation — a real-time telemetry timeliness regression not worth ~6.5ms. The correct optimization is a cheap cross-process 'flush already in flight' check before spawn (skip redundant spawns while preserving timeliness), tracked as future work. No telemetry runtime change ships in this PR; pm-3mal stays open for the bundling lever (pm-ss1d) + the in-flight cross-process flush guard." | ||
| "2026-05-30T06:15:14.974Z",maintainer-agent,"Partial implementation evidence 2026-05-30: added the future-work cross-process telemetry flush spawn guard. scheduleTelemetryFlush now creates a cheap spawn-gate directory before forking, skips duplicate command_finish spawns while a command_start child is booting, also skips when a fresh flush.lock is already owned by another process, and clears the spawn gate once the child enters the process lock. This preserves real-time command_finish flushing without paying duplicate child process startup. Bundling lever remains open under this chore; not closing pm-3mal in this pass." | ||
| files[2]{path,scope,note}: | ||
| src/core/telemetry/runtime.ts,project,Cross-process telemetry flush spawn gate | ||
| tests/unit/telemetry-runtime.spec.ts,project,Telemetry spawn gate regression coverage | ||
| tests[1]{command,path,scope,timeout_seconds,pm_context_mode,env_set,env_clear,shared_host_safe,assert_stdout_contains,assert_stdout_regex,assert_stderr_contains,assert_stderr_regex,assert_stdout_min_lines,assert_json_field_equals,assert_json_field_gte,note}: | ||
| node scripts/run-tests.mjs test -- tests/unit/telemetry-runtime.spec.ts,null,project,300,null,null,null,null,null,null,null,null,null,null,null,Telemetry flush spawn gate regression |
There was a problem hiding this comment.
Avoid direct edits under .agents/pm/**.
These tracker changes are direct modifications in a protected path; they should be produced through the pm system-of-record flow instead of manual file edits.
As per coding guidelines, .agents/pm/**: pm is the system of record. Do not edit .agents/pm files directly.
🤖 Prompt for 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.
In @.agents/pm/chores/pm-3mal.toon around lines 9 - 19, You edited a protected
tracker under the pm system (pm-3mal) directly; revert those manual edits and
re-submit the change through the pm system-of-record flow instead. Undo the
direct changes made to the tracker metadata in the .agents/pm area, recreate the
intended changes as a normal code commit that updates the implementation in the
runtime telemetry code and tests (target the telemetry runtime change and its
test: symbols referencing scheduleTelemetryFlush / spawn gate logic and the
telemetry-runtime.spec tests), and open the change via the official pm workflow
(use the pm tool/CLI to create the tracker/pr) so the .agents/pm metadata is
produced by the system rather than manual edits. Ensure no further direct edits
are made under the protected .agents/pm namespace.
Comment on lines
+6
to
+11
| {"ts":"2026-05-30T06:15:00.644Z","author":"maintainer-agent","op":"files_add","patch":[{"op":"replace","path":"/metadata/updated_at","value":"2026-05-30T06:15:00.644Z"},{"op":"add","path":"/metadata/files","value":[{"path":"src/core/telemetry/runtime.ts","scope":"project","note":"Cross-process telemetry flush spawn gate"},{"path":"tests/unit/telemetry-flush-spawn.spec.ts","scope":"project","note":"Telemetry spawn gate regression coverage"}]}],"before_hash":"e2f02bae12220b98bf6eedba544bec57071a295ef473ee6b7bf9b9cbbc1ecf49","after_hash":"230265440cff8fe93ad627aa07dbb9668e1490fff5856165c8c834901900a441"} | ||
| {"ts":"2026-05-30T06:15:07.935Z","author":"maintainer-agent","op":"tests_add","patch":[{"op":"replace","path":"/metadata/updated_at","value":"2026-05-30T06:15:07.935Z"},{"op":"add","path":"/metadata/tests","value":[{"command":"node scripts/run-tests.mjs test -- tests/unit/telemetry-flush-spawn.spec.ts tests/unit/telemetry-runtime.spec.ts","scope":"project","timeout_seconds":300,"note":"Telemetry flush spawn gate regression"}]}],"before_hash":"230265440cff8fe93ad627aa07dbb9668e1490fff5856165c8c834901900a441","after_hash":"371a126d05a31c09d339864cdda49f967a0e9773a6571e4525bba9b625d5da10"} | ||
| {"ts":"2026-05-30T06:15:14.974Z","author":"maintainer-agent","op":"comment_add","patch":[{"op":"add","path":"/metadata/comments/2","value":{"created_at":"2026-05-30T06:15:14.974Z","author":"maintainer-agent","text":"Partial implementation evidence 2026-05-30: added the future-work cross-process telemetry flush spawn guard. scheduleTelemetryFlush now creates a cheap spawn-gate directory before forking, skips duplicate command_finish spawns while a command_start child is booting, also skips when a fresh flush.lock is already owned by another process, and clears the spawn gate once the child enters the process lock. This preserves real-time command_finish flushing without paying duplicate child process startup. Bundling lever remains open under this chore; not closing pm-3mal in this pass."}},{"op":"replace","path":"/metadata/updated_at","value":"2026-05-30T06:15:14.974Z"}],"before_hash":"371a126d05a31c09d339864cdda49f967a0e9773a6571e4525bba9b625d5da10","after_hash":"c33ba2f362df45688acdc0167ba71f060baa719503fefaed6ac910258edaace3"} | ||
| {"ts":"2026-05-30T06:16:52.364Z","author":"maintainer-agent","op":"files_add","patch":[{"op":"replace","path":"/metadata/files/1/path","value":"tests/unit/telemetry-runtime.spec.ts"},{"op":"replace","path":"/metadata/updated_at","value":"2026-05-30T06:16:52.364Z"}],"before_hash":"c33ba2f362df45688acdc0167ba71f060baa719503fefaed6ac910258edaace3","after_hash":"0306c461f17cb4a021645275d2914324eedbf6000ce347fc2b873ddc16338426"} | ||
| {"ts":"2026-05-30T06:17:11.478Z","author":"maintainer-agent","op":"tests_add","patch":[{"op":"replace","path":"/metadata/tests/0/command","value":"node scripts/run-tests.mjs test -- tests/unit/telemetry-runtime.spec.ts"},{"op":"replace","path":"/metadata/updated_at","value":"2026-05-30T06:17:11.478Z"}],"before_hash":"0306c461f17cb4a021645275d2914324eedbf6000ce347fc2b873ddc16338426","after_hash":"19872442374fe0aa031cc7e2049fc3c0f7e27a54bc9921d0844c26bf8c93144a"} | ||
| {"ts":"2026-05-30T06:42:12.568Z","author":"maintainer-agent","op":"release","patch":[{"op":"remove","path":"/metadata/assignee"},{"op":"replace","path":"/metadata/updated_at","value":"2026-05-30T06:42:12.568Z"}],"before_hash":"19872442374fe0aa031cc7e2049fc3c0f7e27a54bc9921d0844c26bf8c93144a","after_hash":"2cdf00aec1b42e092cdb146e217682246a984630c638b35ab714ba60355a9acf"} |
There was a problem hiding this comment.
Do not directly modify .agents/pm history records.
Lines 6-11 directly edit .agents/pm/history/pm-3mal.jsonl. This violates the repository rule for system-of-record files and should be regenerated through the PM workflow instead of direct file edits.
As per coding guidelines, “.agents/pm/**: pm is the system of record. Do not edit .agents/pm files directly.”
🤖 Prompt for 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.
In @.agents/pm/history/pm-3mal.jsonl around lines 6 - 11, You directly edited
the PM history record "pm-3mal" (the system-of-record) which is forbidden;
revert the changes that modify the pm-3mal history entry (undo the commit(s)
that added/edited metadata/files, tests, comments, release ops) and regenerate
this history through the PM workflow instead—remove any diffs targeting the
pm-3mal history object, then re-run the official PM recording process (the
project’s PM workflow/automation) to produce the intended
metadata/comments/tests so the system-of-record is updated correctly.
| {"ts":"2026-05-29T16:04:39.577Z","author":"codex-audit-20260529","op":"tests_add","patch":[{"op":"replace","path":"/metadata/tests/10/timeout_seconds","value":600},{"op":"replace","path":"/metadata/tests/10/command","value":"pnpm changelog:pm:check"},{"op":"replace","path":"/metadata/tests/9/timeout_seconds","value":300},{"op":"replace","path":"/metadata/tests/9/command","value":"pnpm build"},{"op":"replace","path":"/metadata/tests/8/timeout_seconds","value":900},{"op":"replace","path":"/metadata/tests/8/command","value":"PM_DOGFOOD_SEMANTIC=1 PM_OLLAMA_MODEL=qwen3-embedding:0.6b node scripts/dogfood-package-first.mjs"},{"op":"replace","path":"/metadata/tests/7/timeout_seconds","value":600},{"op":"replace","path":"/metadata/tests/7/command","value":"pm health --check-only --json"},{"op":"replace","path":"/metadata/tests/6/command","value":"node scripts/run-tests.mjs test -- tests/unit/get-append-command.spec.ts tests/integration/help-runtime.spec.ts tests/unit/completion-command.spec.ts tests/unit/error-guidance.spec.ts"},{"op":"replace","path":"/metadata/tests/5/command","value":"node scripts/run-tests.mjs test -- tests/unit/extension-command.spec.ts tests/integration/cli.integration.spec.ts -t 'installs runtime dependencies for packed npm package extensions|normalizes npm file-alias package specs|resolves existing cwd-relative npm package specs'"},{"op":"replace","path":"/metadata/tests/4/timeout_seconds","value":300},{"op":"replace","path":"/metadata/tests/4/command","value":"node scripts/run-tests.mjs test -- tests/unit/drift-scan.spec.ts -t 'detects broken chains'"},{"op":"replace","path":"/metadata/tests/3/timeout_seconds","value":900},{"op":"replace","path":"/metadata/tests/3/command","value":"node scripts/run-tests.mjs test -- tests/integration/release-readiness-runtime.spec.ts tests/integration/release-automation-contract.spec.ts tests/unit/extension-command.spec.ts tests/integration/cli.integration.spec.ts tests/unit/get-append-command.spec.ts tests/integration/help-runtime.spec.ts tests/unit/completion-command.spec.ts tests/unit/error-guidance.spec.ts tests/unit/drift-scan.spec.ts"},{"op":"replace","path":"/metadata/tests/2/timeout_seconds","value":1800},{"op":"replace","path":"/metadata/tests/2/command","value":"node scripts/release/run-gates.mjs --json --telemetry-mode required --max-sentry-critical 10 --max-sentry-high 20 --max-telemetry-error-rate 10 --max-telemetry-missing-error-rows 0"},{"op":"replace","path":"/metadata/tests/1/timeout_seconds","value":600},{"op":"replace","path":"/metadata/tests/1/command","value":"mise exec node@20 -- pnpm smoke:npx"},{"op":"add","path":"/metadata/tests/11","value":{"command":"pnpm typecheck","scope":"project","timeout_seconds":300}},{"op":"replace","path":"/metadata/updated_at","value":"2026-05-29T16:04:39.577Z"}],"before_hash":"8ee4a294ed5d88f82ef92cb49215d40327a55e6c96e4c7ab6949b8aa613005c3","after_hash":"d947e5dc1a097e138e5079edbeff0a6a824f20915d4eef3fce6e5ba14fb1370b"} | ||
| {"ts":"2026-05-29T16:09:22.065Z","author":"codex-audit-20260529","op":"comment_add","patch":[{"op":"add","path":"/metadata/comments/20","value":{"created_at":"2026-05-29T16:09:22.065Z","author":"codex-audit-20260529","text":"Final CI follow-up verification 2026-05-29: full local release gate re-run passed after the forward fix: build, typecheck, docs/skills, static quality, coverage 1979 tests at 100%, version policy, secret scan, npx smoke, package-first dogfood, npm-pack dry-run, compatibility, and required Sentry/telemetry gate. Sentry threshold remains critical=0 high=4; telemetry finish_error_rate_pct=1.36 and missing error-code rows=0."}},{"op":"replace","path":"/metadata/updated_at","value":"2026-05-29T16:09:22.065Z"}],"before_hash":"d947e5dc1a097e138e5079edbeff0a6a824f20915d4eef3fce6e5ba14fb1370b","after_hash":"b6e82d0490100e610fb9daa4f9b917606e4194177134bad42bb7cd45a5a1fb47"} | ||
| {"ts":"2026-05-30T05:21:35.554Z","author":"maintainer-agent","op":"comment_add","patch":[{"op":"add","path":"/metadata/comments/21","value":{"created_at":"2026-05-30T05:21:35.554Z","author":"maintainer-agent","text":"PR #83 (perf/cache-split-and-agent-ux) opened: closes pm-407c/pm-8c2s/pm-jd3v/pm-wr74/pm-7by2/pm-4d1b/pm-bpaj. Light/collections cache split, per-field semantic defaults + partial-settings hardening, executable command aliases (show->get, 312 telemetry events), type synonyms, calendar positional-date routing + impossible-date rejection, explicit semantic refresh contract. Full suite 2003 green + all gates. Awaiting CI + bot review."}},{"op":"replace","path":"/metadata/updated_at","value":"2026-05-30T05:21:35.554Z"}],"before_hash":"b6e82d0490100e610fb9daa4f9b917606e4194177134bad42bb7cd45a5a1fb47","after_hash":"7ca0a7728ecc439913e3a62aecf02bf5bbb849c1268effaa75c2b05b6ae6cc5e"} | ||
| {"ts":"2026-05-30T05:59:25.473Z","author":"maintainer-agent","op":"comment_add","patch":[{"op":"add","path":"/metadata/comments/22","value":{"created_at":"2026-05-30T05:59:25.473Z","author":"maintainer-agent","text":"PR #83 MERGED to main 2026-05-30 (merge commit). All CI green across two review rounds (Build, 5 Gates, 4 runtime smokes, CodeRabbit, Sourcery, Docs). Round-1 bot feedback addressed: fixed real impossible-date guard gaps (compact-datetime/Z-suffixed dates), DRY centralization (HEAVY_METADATA_KEYS, findCommandTokenIndex), + test coverage. Closed: pm-407c/pm-8c2s/pm-jd3v/pm-wr74/pm-7by2/pm-4d1b/pm-bpaj. [Unreleased] populated -> daily auto-release will cut next version."}},{"op":"replace","path":"/metadata/updated_at","value":"2026-05-30T05:59:25.473Z"}],"before_hash":"7ca0a7728ecc439913e3a62aecf02bf5bbb849c1268effaa75c2b05b6ae6cc5e","after_hash":"dd1930dd30c09acbe794e02cc6a52df78c936eb80a3d1f59abf116d1ee3d397e"} |
There was a problem hiding this comment.
Do not directly edit .agents/pm history records.
This change directly modifies a .agents/pm/** file, which must be updated through the pm system workflow/tooling instead of manual edits in the PR.
As per coding guidelines, .agents/pm/**: "pm is the system of record. Do not edit .agents/pm files directly."
🤖 Prompt for 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.
In @.agents/pm/history/pm-th6y.jsonl at line 55, The change directly edits a pm
system-of-record file (.agents/pm/history/pm-th6y.jsonl) which is forbidden;
revert this manual edit and re-apply the update using the pm workflow/tooling
that publishes to the metadata/comments and metadata/updated_at fields (i.e.,
make the change by invoking the pm tool that appends the new comment entry under
metadata.comments and updates metadata.updated_at) rather than committing JSON
edits by hand so the pm system remains authoritative.
- fail closed on unexpected spawn-gate filesystem errors - avoid stale-lock removal and recursive parent mkdir unless needed on the telemetry hot path - reuse runtime _testOnly lock path helpers in tests and cover cleanup when the process flush lock is already held Verification: node scripts/run-tests.mjs test -- tests/unit/telemetry-runtime.spec.ts; pnpm typecheck; pnpm quality:static; node scripts/check-secrets.mjs; pnpm changelog:pm:check; git diff --check
Owner
Author
|
Addressed the Sourcery and Gemini telemetry spawn-gate feedback in 7920682: unexpected filesystem errors now fail closed, stale-lock removal and recursive parent mkdir are only used when needed, tests reuse runtime _testOnly lock-path helpers, and added coverage for spawn-gate cleanup when the process flush lock is already held. Re-ran: node scripts/run-tests.mjs test -- tests/unit/telemetry-runtime.spec.ts; pnpm typecheck; pnpm quality:static; node scripts/check-secrets.mjs; pnpm changelog:pm:check; git diff --check. |
Owner
Author
|
CodeRabbit follow-up note: the valid test helper feedback was addressed in 7920682. The comments are false positives for this repository workflow: these tracker artifacts were generated through the pm CLI (, , , , , ) and committed as the system-of-record output, not edited directly. |
Owner
Author
|
CodeRabbit follow-up note: the valid test helper feedback was addressed in 7920682. The .agents/pm path comments are false positives for this repository workflow: these tracker artifacts were generated through the pm CLI commands and committed as the system-of-record output, not edited directly. |
unbraind
added a commit
that referenced
this pull request
Jul 14, 2026
…e backfill, milestone note Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VDJ1m5HDPDBVp8BBAixtnP
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Audit coverage
Verification
Summary by Sourcery
Introduce a cross-process telemetry flush spawn gate to reduce redundant flush child processes and update project metadata for the latest SDK dogfood audit.
Enhancements:
Documentation:
Tests:
Chores: