Skip to content

fix(trace): separate Pi cache token accounting and update generated hook configs - #3731

Open
migmartri wants to merge 4 commits into
chainloop-dev:mainfrom
migmartri:dw/32050-1793051504
Open

migmartri wants to merge 4 commits into
chainloop-dev:mainfrom
migmartri:dw/32050-1793051504

Conversation

@migmartri

Copy link
Copy Markdown
Member

Summary

  • Separate Pi cache accounting: cache reads and writes are preserved as their own evidence categories, and total_tokens is recomputed as base input + output, excluding both cache categories. Cost stays authoritative from the session. Invalid or overflowing counters are ignored with deterministic warnings.
  • Keep the Pi evidence schema_version at v1 and stop gating the cache-accounting checks on the schema version.
  • Regenerate the Claude Code and Cursor hook configurations after the missing-CLI guard, so hooks no-op when the chainloop CLI is not on PATH.
  • Add the managed Pi extension (.pi/extensions/chainloop-trace.ts) and teach the OpenCode plugin the OpenCode 2.x tool-result shape.

Refs #3704 (Pi provider support).

Assisted-by: Claude Code

waveywaves and others added 4 commits October 25, 2026 02:50
Recompute Pi v2 totals from native input and output counters, preserve cache categories, and verify them against cache-active raw session records.

Assisted-by: Vibhav's Pi
Signed-off-by: Vibhav Bobade <vibhav.bobde@gmail.com>
Retain the existing Chainloop evidence version while normalizing Pi cache counters. Clarify that agent releases and evidence versions are distinct.

Assisted-by: Vibhav's Pi
Signed-off-by: Vibhav Bobade <vibhav.bobde@gmail.com>
Compare Pi usage with the native session records regardless of the Chainloop evidence version; only Codex requires its v2 accounting guard.

Assisted-by: Vibhav's Pi
Signed-off-by: Vibhav Bobade <vibhav.bobde@gmail.com>
Run chainloop trace init after the Pi provider support: guard the
generated Claude Code and Cursor hook commands so they no-op when the
CLI is missing, add the managed Pi extension reference copy, and teach
the OpenCode plugin the OpenCode 2.x tool-result shape.

Assisted-by: Claude Code
Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev>
@chainloop-platform

chainloop-platform Bot commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

PR validation — ⚠️ 1 failing

Status Policy Material Messages
⚠️ Failed pr-min-approvals pr-info PR/MR #3731 has 0 approving reviews, 1 required.
✅ Passed pr-description-required pr-info -
✅ Passed pr-user-story-linked pr-info -

View attestation ↗

AI Session Checks — ⚠️ no AI session found

Missing AI Coding Sessions

This organization requires every PR to be backed by a Chainloop Trace AI coding session, and none was found for this one.

Please make sure the AI coding session evidence has been sent by the Chainloop CLI, or add the skip-ai-session label to this PR to bypass this check.

Learn more about Chainloop Trace.

Security Checks — ✅ 6 passing

✅ secret-scan

Status Policy Messages
✅ Passed secrets-detection -

✅ sast-scan

Status Policy Messages
✅ Passed owasp-top10-2025 -
✅ Passed sast -
✅ Passed cwe-top25 -
✅ Passed cwe-top26-40-cusp -

✅ iac-scan

Status Policy Messages
✅ Passed iac-misconfiguration -
Scans not applied (2)
Scan Reason
vulnerability-scan no manifest/lockfile changed
github-actions-scan no workflow files changed

View attestation ↗

Security context

✅ Nothing this change touches has a recorded security-fix history.

View in Chainloop ↗ · How this works ↗


Powered by Chainloop and Chainloop Trace

@cubic-dev-ai cubic-dev-ai Bot 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.

2 issues found across 11 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name=".pi/extensions/chainloop-trace.ts">

<violation number="1" location=".pi/extensions/chainloop-trace.ts:101">
P3: The stderr buffering in `runHook` is never read, and its overflow kills the hook. Discard stderr instead of buffering it, so a noisy stderr cannot SIGTERM the process and drop its stdout response.</violation>

<violation number="2" location=".pi/extensions/chainloop-trace.ts:146">
P2: Banner-only session-start responses are not cached, so every `before_agent_start` invokes session-start again and repeats the banner and its auth/network work on every turn. Cache any successful session-start response, not only responses containing `instruction`.</violation>
</file>

Reply with feedback, questions, or to request a fix.

View guided diff | Re-trigger cubic

const start = async (ctx, hookEventName) => {
const id = sessionID(ctx)
const response = await runHook(ctx, "session-start", hookPayload(ctx, hookEventName))
if (response?.instruction) starts.set(id, response)

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.

P2: Banner-only session-start responses are not cached, so every before_agent_start invokes session-start again and repeats the banner and its auth/network work on every turn. Cache any successful session-start response, not only responses containing instruction.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At .pi/extensions/chainloop-trace.ts, line 146:

<comment>Banner-only session-start responses are not cached, so every `before_agent_start` invokes session-start again and repeats the banner and its auth/network work on every turn. Cache any successful session-start response, not only responses containing `instruction`.</comment>

<file context>
@@ -0,0 +1,246 @@
+  const start = async (ctx, hookEventName) => {
+    const id = sessionID(ctx)
+    const response = await runHook(ctx, "session-start", hookPayload(ctx, hookEventName))
+    if (response?.instruction) starts.set(id, response)
+    showBanner(ctx, response)
+    return response
</file context>
Suggested change
if (response?.instruction) starts.set(id, response)
if (response?.instruction || response?.banner) starts.set(id, response)

}

child.stdout.on("data", (chunk) => { stdoutBytes = capture(stdout, chunk, stdoutBytes) })
child.stderr.on("data", (chunk) => { stderrBytes = capture(stderr, chunk, stderrBytes) })

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.

P3: The stderr buffering in runHook is never read, and its overflow kills the hook. Discard stderr instead of buffering it, so a noisy stderr cannot SIGTERM the process and drop its stdout response.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At .pi/extensions/chainloop-trace.ts, line 101:

<comment>The stderr buffering in `runHook` is never read, and its overflow kills the hook. Discard stderr instead of buffering it, so a noisy stderr cannot SIGTERM the process and drop its stdout response.</comment>

<file context>
@@ -0,0 +1,246 @@
+      }
+
+      child.stdout.on("data", (chunk) => { stdoutBytes = capture(stdout, chunk, stdoutBytes) })
+      child.stderr.on("data", (chunk) => { stderrBytes = capture(stderr, chunk, stderrBytes) })
+      child.on("error", () => {
+        exited = true
</file context>
Suggested change
child.stderr.on("data", (chunk) => { stderrBytes = capture(stderr, chunk, stderrBytes) })
child.stderr.resume()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants