Entire

I've reviewed PR #1263 thoroughly — read the full diff, traced the logic against main, and tested the merge. Here's my assessment.

Review: PR #1263 — Fix Cursor hook misattribution + token usage

Verdict: Sound design, well-tested, but has one merge blocker. The author's "rebased latest main" claim is now stale — main has moved and the branch no longer merges cleanly.

What it does (two independent fixes)

Cross-agent hook guard (#1262) — shouldSkipForwardedHook() short-circuits executeAgentHook when a hook fires for agent X but the payload's transcript_path belongs to agent Y's session dir (Cursor IDE firing Claude Code hooks when only .claude/settings.json is installed).
Cursor token usage (#1264) — parses token fields from Cursor's stop hook (the only authoritative source — Cursor's JSONL has no usage data), threads them through event.TokenUsage, and adds per-checkpoint scoping via SessionState.CheckpointTokenUsage.

Correctness — looks right

I traced the token-scoping flow against main:

SaveStep accumulates into both TokenUsage (session-wide, for entire status) and CheckpointTokenUsage (pending delta). ✓
CondenseSession backfills sessionData.TokenUsage from CheckpointTokenUsage only when transcript-derived usage is empty (hasTokenUsageData guard) — correct for Cursor, no-op for agents with real transcript usage. ✓
All three callers of CondenseSession (condenseAndUpdateState, CondenseSessionByID, CondenseAndMarkFullyCondensed) reset CheckpointTokenUsage = nil after condensing — so checkpoint N+1 gets only its own delta, not the cumulative total. The PerCheckpointScoping integration test asserts exactly this (500, not 700). ✓
The negative-clamp and nil-vs-zero ("no data" vs "all zeros") semantics in tokenUsageFromStop are thoughtful and tested.

The guard fails open in every ambiguous case (no SessionRef, unattributable path, unresolvable repo root) — the safe default. .cursor/hooks.json correctly matches the existing committed .codex/hooks.json / .gemini/settings.json dogfooding convention (go run main.go), so that's not a stray file.

🔴 Blocker: merge conflict

mergeable: CONFLICTING. A real textual conflict exists in cmd/entire/cli/lifecycle_test.go (3 hunks, ~lines 1144–1262) — both main and this branch added adjacent test helpers/functions. It's mechanical (keep-both), not logical. Everything else auto-merges. The author needs to rebase/merge main again and resolve it.

🟡 Minor notes (non-blocking)

Scope creep: two unrelated edits ride along — telemetry/detached_test.go (adds a redundant return after t.Fatal, presumably to silence a nil-deref linter) and internal/coreapi/UPSTREAM.md (drops a trailing newline). Harmless, but worth asking the author to drop them or call them out.
Subagent tokens for Cursor: handleLifecycleTurnEnd now prefers event.TokenUsage and skips CalculateTokenUsage, which is where SubagentAwareExtractor adds subagent tokens. Since .cursor/hooks.json registers subagentStart/subagentStop, if Cursor's stop payload doesn't already fold in subagent usage, those tokens are lost. Likely acceptable (the alternative was zero), but worth a one-line confirmation from the author.
CI: no checks ran — it's a fork PR, so CI is gated. Can't independently confirm the "full suite + lint pass" claim without running it on the merged tree.

Suggested reply to the contributor

Thanks for the follow-up. The token-scoping design is correct and the test coverage (condensation survival + per-checkpoint scoping) is exactly what I wanted. One blocker before I can merge: the branch conflicts with current main — there's a content conflict in cmd/entire/cli/lifecycle_test.go (both sides added test helpers in the same spot; keep-both resolves it). Please merge/rebase main again and push. Two small asks: (1) drop the unrelated telemetry/detached_test.go and internal/coreapi/UPSTREAM.md changes (or note why they're here), and (2) confirm whether Cursor's stop payload includes subagent tokens — since we now skip the transcript-based subagent extractor for Cursor turns.

Want me to post this as a PR comment, or draft inline review comments on the specific lines?

Done. The merge conflict is resolved and pushed.

Summary

mergeable flipped from CONFLICTING → MERGEABLE. (mergeStateStatus: BLOCKED just reflects the still-pending maintainer review/CI approval — not a conflict.)

What I did:

Fetched the fork branch (SnowingFox/cli@3f25b2c2) and merged current origin/main into it.
Resolved the sole conflict in cmd/entire/cli/lifecycle_test.go — an interleaved "both sides added adjacent code" conflict. Kept both independent additions intact:
- PR's TestHandleLifecycleTurnEnd_PrefersEventTokenUsage
- main's mockContextInjectorAgent type, addGitHubOriginForLifecycleTest helper, and the two ContextInjection tests
Verified: go build ✓, go vet ✓, the three affected tests pass ✓, mise run fmt (no changes) ✓, mise run lint → 0 issues ✓.
Pushed the merge commit (950f947b9) to the PR branch via maintainerCanModify.

Your working tree is back on a clean main; the temporary pr1263-merge branch is deleted.

The remaining items from my earlier review are unchanged (non-blocking): the unrelated telemetry/detached_test.go + UPSTREAM.md edits, and the open question about whether Cursor's stop payload folds in subagent tokens. Want me to post either of those as a PR comment?

You were right — the textual merge was clean, but there was a semantic conflict it couldn't catch.

Root cause

The PR's new integration test (cursor_token_condensation_test.go) referenced checkpoint.CommittedMetadata, but main has since renamed that type to checkpoint.Metadata (now an alias for apicheckpoint.Metadata — part of the api/checkpoint refactor). That file sits behind the //go:build integration tag, so the normal build, go vet, lint, and the canary all passed — only the integration shards compile it, and all three failed with undefined: checkpoint.CommittedMetadata.

Fix

Renamed the two references to checkpoint.Metadata. The field is identical (TokenUsage *types.TokenUsage, same JSON tag) and agent.TokenUsage is a type alias for types.TokenUsage, so the test logic is unchanged.

Verified before pushing:

Integration package compiles under the build tag ✓
Both new tests pass locally: TestCursorTokenUsage_SurvivesCondensation and TestCursorTokenUsage_PerCheckpointScoping ✓
mise run fmt (no changes) + mise run lint → 0 issues ✓

Pushed as fa112bcf4; PR is back to MERGEABLE. The integration shards should pass on the new run.

Want me to watch the CI run and confirm it goes green? (Integration takes a few minutes.)