feat: core primitives for significant extension capability improvements (clean)#200
feat: core primitives for significant extension capability improvements (clean)#200dgarson merged 2 commits intodgarson/forkfrom
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 01157a747e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| config, | ||
| "session", | ||
| "session.end", | ||
| randomUUID(), |
There was a problem hiding this comment.
Preserve actual runId when emitting session.end records
session.end events are currently written with randomUUID() as runId, but the summary pipeline keys off session.end run IDs (RunSummarizer.scanDayForAgent filters by e.type === "session.end" then aggregates by endEvt.runId), so event_ledger.run_summary cannot return data for real agent run IDs and instead stores summaries under opaque random IDs. This makes run-summary queries effectively unusable for callers that only know the true run ID.
Useful? React with 👍 / 👎.
| if (ok) { | ||
| resolve((payload ?? {}) as T); | ||
| } else { | ||
| const msg = error?.message ?? `gateway method "${method}" failed`; |
There was a problem hiding this comment.
Surface payload error messages from gateway handlers
The in-process dispatcher only reads error?.message when respond(false, ...) is called, so handlers that report failures via payload (for example respond(false, { error: "..." }) patterns used in this commit’s extension gateway methods) are collapsed into the generic gateway method "..." failed error. This loses actionable failure details for runtime.gateway.call(...) callers and makes debugging/branching on specific failures much harder.
Useful? React with 👍 / 👎.
| // Track run starts for trace context and spans | ||
| api.on("before_agent_start", (event, ctx) => { | ||
| const agentId = ctx.agentId ?? "unknown"; | ||
| const runId = ctx.sessionKey ?? agentId; |
There was a problem hiding this comment.
Use ctx.runId for observability run/span correlation
The observability plugin keys run tracing by ctx.sessionKey instead of ctx.runId, even though the hook context includes a per-run identifier; this causes multiple runs in the same session to share the same run key, so trace context and span bookkeeping are merged at session scope rather than run scope. As a result, telemetry can no longer reliably distinguish individual runs for analysis or debugging.
Useful? React with 👍 / 👎.
Summary
Recreated from PR #186 with clean ancestry, similar to the #199 process.
Features
Changes
This is a minimal re-application of the core feature commits from PR #186, excluding the cherry-picked main fixes that caused merge conflicts.
Original PR #186 had many cherry-picked commits causing unrelated-history conflicts. This version contains only the essential feature changes.
Commits included:
Residual Risk
src/agents/pi-embedded-runner/run.ts,src/agents/system-prompt.ts,src/config/types.agents.ts,src/config/zod-schema.agent-runtime.ts,src/infra/diagnostic-events.ts, andsrc/plugins/types.tswere not applied as those changes already exist in dgarson/forkRelated