Skip to content

docs(devlog): enumerate every invocation-line site to bound the cursor fix - #2913

Merged
lidge-jun merged 1 commit into
devfrom
codex/cursor-invocation-site-audit
Aug 29, 2026
Merged

docs(devlog): enumerate every invocation-line site to bound the cursor fix#2913
lidge-jun merged 1 commit into
devfrom
codex/cursor-invocation-site-audit

Conversation

@lidge-jun

@lidge-jun lidge-jun commented Aug 29, 2026

Copy link
Copy Markdown
Owner

Summary

Devlog only. Follow-up to #2910, which fixed the checkpoint replay path.

That was the third partial fix for one defect: #2900 named the invocation inside a replayed tool
result, #2903 bounded and identity-checked it, and #2910 covered the checkpoint continuation path
that the first two missed. The obvious residual risk was a fourth site with the same shape, so this
enumerates them against the source instead of assuming the set is closed.

Only two functions attach an invocation line, and both take knownCalls after #2910:

Site Line Emits Indexed from
rootPromptMessages 240 root [Tool Result] blob knownCalls ?? toolCallsByCallId(messages)
conversationTurns 953 turn step [Tool Result] knownCalls ?? toolCallsByCallId(messages)

toolResultToText has a third caller, contentText (line 498), which passes no call and so can never
name an invocation. It is not a gap: no tool result reaches it, because all three of its callers
select on role first — line 285 guards on user/developer, line 1034 is the turn userMessage
reached only after the assistant and toolResult branches both continue, and line 1052
(activePromptText) scans backwards for a user/developer message.

request-builder.ts has its own toolResultToText for the text messages channel; different
channel, no invocation line by design, out of scope.

Verification

  • No source change, so behaviour is unchanged by construction.
  • bun run privacy:scan — passed.
  • bun test tests/cursor-tool-result-invocation.test.ts tests/cursor-tool-continuation.test.ts tests/cursor-blob.test.ts — 124 pass, 0 fail.
  • Full suite and bun x tsc --noEmit on ssh lidge at the head SHA.

No GUI change.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.

Summary by CodeRabbit

  • Documentation
    • Added analysis documenting how invocation lines and tool results are handled during replay.
    • Clarified the supported replay paths and distinguished them from unrelated text-processing channels.
    • Confirmed that no public interfaces or externally visible behavior were changed.

Two partial fixes had already shipped for this defect, so the residual risk was
a third replay site with the same suffix-indexing gap. Enumerated instead of
assumed: only rootPromptMessages and conversationTurns attach an invocation
line and both now take knownCalls. The third toolResultToText caller,
contentText, passes no call and is unreachable for toolResult because all three
of its callers select on user/developer role first.
@lidge-jun
lidge-jun requested a review from Ingwannu as a code owner August 29, 2026 14:33
@github-actions

Copy link
Copy Markdown
Contributor

Deterministic PR hygiene checks passed.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Aug 29, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-08-29T14:37:04.816997Z 1241a8d PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions github-actions Bot added the documentation Improvements or additions to documentation label Aug 29, 2026
@coderabbitai

coderabbitai Bot commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: dec349fb-ced4-46f1-8607-6a5f909d8bbf

📥 Commits

Reviewing files that changed from the base of the PR and between a0a8e8e and 1241a8d.

📒 Files selected for processing (1)
  • devlog/_plan/260829_cursor_tool_continuation_pairing/040_phase5_checkpoint_suffix_gap.md

Included review availability: Your plan provides up to 10 included reviews per hour; 3 remain after this review.


📝 Walkthrough

Walkthrough

The plan adds a completeness audit for invocation-line generation. It identifies rootPromptMessages and conversationTurns as the replay sites that use knownCalls, and documents why other text paths and request-builder.ts are out of scope.

Changes

Invocation Site Audit

Layer / File(s) Summary
Document invocation-line replay coverage
devlog/_plan/260829_cursor_tool_continuation_pairing/040_phase5_checkpoint_suffix_gap.md:108-131
The plan documents the two invocation-line replay sites, their full-history lookup behavior, and the exclusion of unrelated toolResultToText callers and request-builder.ts.

Estimated code review effort: 1 (Trivial) | ~3 minutes

Merge Risk: ⚪ Minimal · up to 1241a

This documentation-only PR records the invocation-line site audit without changing product behavior, so no actionable merge-blocking risk remains after normal checks and review.

Suggested reviewers: ingwannu

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the documentation change. It identifies the devlog scope, the enumeration of invocation-line sites, and the purpose of bounding the cursor fix.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (1 skipped: 1 unsupported.)

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/cursor-invocation-site-audit

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@lidge-jun
lidge-jun merged commit cfb70c9 into dev Aug 29, 2026
20 checks passed
@lidge-jun
lidge-jun deleted the codex/cursor-invocation-site-audit branch August 29, 2026 14:38
@lidge-jun

Copy link
Copy Markdown
Owner Author

리뷰 · 우선순위 43 / 80

이 PR은 코드를 안 고친다. 지금 dev HEAD는 a0a8e8e2b (#2912, 대시보드 slop 유닛 Outcome 마감) 이다. 그 바로 앞에 #2910 이 있다. 체크포인트가 호출은 앞에 두고 결과만 다시 보낼 때, 호출 줄을 전체 히스토리에서 찾게 한 고침이다. #2900 이 재생 결과에 호출을 붙였고, #2903 이 그 줄을 이름과 크기 한도로 묶었고, #2910 이 체크포인트 경로를 덮었다. 같은 구멍이 네 번째 자리에 남아 있으면 또 부분 고침이 된다. 이 PR은 그 자리가 더 있는지 소스를 세어 기록한다.

지금 HEAD의 src/adapters/cursor/protobuf-request.ts 를 보면 호출 줄을 붙이는 함수는 둘이다. rootPromptMessages 240행은 외부 모델일 때 knownCalls ?? toolCallsByCallId(messages) 로 맵을 만든다. conversationTurns 953행은 같은 맵을 turnCalls 에 담는다. 실제로 [Tool Result] 텍스트를 쓰는 곳은 1003-1014행이고, externalModel 일 때만이다. 체크포인트 경로는 1172-1174행에서 전체 히스토리 맵을 두 함수에 넘긴다. full-replay 1201-1204행은 맵을 안 넘기므로 예전처럼 자기 messages 에서 인덱스를 만든다.

toolResultToText 의 다른 호출은 contentText 498행이다. 여기는 호출 인자를 안 넘기므로 호출 줄이 안 붙는다. 이 길로 도구 결과가 오면 안 된다. historyContentText 를 부르는 285행과 1034행은 그 앞에서 역할이 user 또는 developer 인지 먼저 본다. activePromptText 1052행도 contentText 를 부르기 전에 같은 역할만 고른다. 그래서 이 세 곳으로는 도구 결과가 안 들어온다. src/adapters/cursor/request-builder.ts 241행의 toolResultToText 는 텍스트 messages 채널용이고, 호출 줄이 원래 없다. 다른 채널이다.

네이티브(비외부) 모델은 호출 줄을 텍스트로 안 붙인다. conversationTurnsexternalModel 이 아니면 turnCalls 를 안 만든다. 같은 턴 안의 짝은 pendingToolCallstoolCallStep 으로 붙인다. 짝이 없으면 1025행이 toolResultToText(message) 만 호출해서 호출 줄 없이 텍스트를 낸다. 이번 유닛이 고친 결함은 외부 모델의 텍스트 에코다. 네이티브에서 짝이 없는 결과는 다른 이야기다.

types.ts/config.ts 분할과 무관하다. close-don't-rebase 대상이 아니다. 런타임, 라우팅, 인증은 안 건드린다. merge-base 가 지금 HEAD 와 같아서 파일도 040 노트 하나만 갈라진다.

라인 106 다음 Completeness 제목 - Verification 목록 한가운데에 들어간다. 123 pass 다음에 제목이 나오고, 그 아래 - bun x tsc 와 full suite 불릿이 Completeness 소속이 된다. 검증 기록이 끊긴다.

라인 표 conversationTurns 953 - 953은 맵을 담는 줄이다. [Tool Result] 를 쓰는 곳은 1003-1014행이고 외부 모델일 때만이다. 표의 Emits 칸이 그 줄을 가리키는 것처럼 읽힌다.

경로 contentText 호출부 - 본문은 contentText 호출이 셋이라고 한다. 실제 contentText 직접 호출은 historyContentText 525행과 activePromptText 1052행 둘이다. 285행과 1034행은 historyContentText 호출이다. 도구 결과가 안 온다는 결론은 맞다. 세는 단위만 섞였다.

경로 conversationTurns 1025 - 네이티브에서 짝이 없으면 호출 줄 없이 toolResultToText 를 부른다. 이번 세기 대상은 아니지만, "두 자리가 전부"라고만 적으면 이 줄이 안 보인다.

PR 본문 124 pass - 파일 Verification 은 여전히 123 pass 다. 이 PR이 테스트를 안 바꾸므로 파일 숫자가 맞다.

메인테이너의 판단이 필요한 지점

  • Completeness 제목을 Verification 목록 밖으로 옮긴 뒤 머지할지, 지금 구조 그대로 갈지
  • 네이티브 1025행을 범위 밖이라고 한 줄 명시할지, 다음 유닛으로 남길지
  • 표 행번호를 실제 emit 줄(1003-1014)로 고칠지

너의 추천
Completeness 제목을 Verification 불릿이 끝난 뒤(tsc/full suite 아래, live-runs 앞)로 옮기고 머지하세요. 소스 세기는 지금 HEAD 와 맞다. 코드를 바꿀 필요는 없다. 표는 953 대신 실제 [Tool Result] emit 줄을 적는 편이 다음 사람 손이 덜 간다.

이 댓글은 grok-bot이 작성했습니다

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

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant