Repository navigation
fix(agent): keep native citation tokens out of chat replies - #1994
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 19a8327c69
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
The citation fallback works on the webhook paths, but the PR adds failing owner-package tests and leaves supported structured chat messages unsanitized.
Reviewed changes in 19a8327c6, covering the citation formatter, Teams instructions, every changed Chat SDK delivery boundary, documentation, and regression tests.
- Citation formatting: Replaces complete and truncated Codex native citation markers with
[source link unavailable]while preserving ordinary Markdown links. - Delivery integration: Applies formatting to primary streams, collected final replies, finish-hook messages, manual placeholder edits, and message-channel reply effects.
- Teams guidance: Requests descriptive Markdown links to verified URLs and prohibits native citation IDs.
- Regression coverage: Adds formatter split tests and webhook final, loading, manual, and streaming cases.
gpt-5.6-sol ｜ 𝕏
There was a problem hiding this comment.
Important
The new commits address both earlier behavior findings, but the added AST regression still fails and leaves the owner-package checks red.
Reviewed changes since the prior pullfrog review at 19a8327c6, covering commits e938ceaca..a5c89a9ec.
- Structured messages: Recursively formats mdast text and visible card fields while preserving URLs and action values.
- Truncated markers: Replaces every incomplete native marker prefix at finalization instead of leaking private-use characters.
- Direct Channel test: Returns the finish-hook reply effect so static and streaming manual delivery cases execute.
- Provider runtime: Updates the embedded T3 provider-runtime preview and its lockfile graph.
gpt-5.6-sol ｜ 𝕏
@vite-hub/agent
@vite-hub/auth
@vite-hub/blob
@vite-hub/box
@vite-hub/browser
@vite-hub/channels
@vite-hub/cli
@vite-hub/connections
@vite-hub/content
@vite-hub/database
@vite-hub/email
@vite-hub/env
@vite-hub/kv
@vite-hub/markdown-template
@vite-hub/queue
@vite-hub/rate-limit
@vite-hub/realtime
@vite-hub/runtime
@vite-hub/sandbox
@vite-hub/schedule
@vite-hub/shell
@vite-hub/source
@vite-hub/ui
vite-hub
@vite-hub/workflow
@vite-hub/workspace
commit: |
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes since the prior Pullfrog review at a5c89a9ec, covering the corrected structured-message regression and refreshed dependency snapshots.
- Corrected AST and card assertions: Compared semantic structured-message content without requiring incompatible mdast source positions or exact object equality.
- Stabilized dependency snapshots: Preserved peer-dependent lockfile variants and added integrity metadata for the refreshed T3 runtime tarballs.
gpt-5.6-sol ｜ 𝕏
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4f8be6ef66
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Important
The new commits add two regressions that must be addressed before merge: one new owner-package scenario is red, and the cancellation change does not work for the async-generator streams used by the runtime.
Reviewed changes since the prior Pullfrog review at 4f8be6ef6, covering commits 528402007 and 97ac8a7be.
- Formatted error fallbacks: Applied citation cleanup to resolved error fallback text and added webhook coverage for automatic and manual delivery.
- Extended restart coverage: Added a host-restart scenario intended to verify citation cleanup in a custom restart notice.
- Changed stream cancellation: Replaced the async-generator formatter wrapper with a custom iterator that forwards
return()andthrow()to its source. - Added cancellation regression coverage: Exercised cancellation while a custom source iterator has a pending
next()call.
gpt-5.6-sol ｜ 𝕏
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4f01831561
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Important
The cancellation fix is complete, but the revised restart regression still leaves the Agent package checks red because the fixture constructs an invalid Agent definition.
Reviewed changes since the prior Pullfrog review at 97ac8a7be, covering commit 4f0183156.
- Unblocked stream cancellation: Raced pending source reads against wrapper cancellation, requested queued async-generator cleanup without blocking delivery shutdown, and added a real async-generator regression.
- Moved restart fallback configuration: Placed the citation-bearing fallback on the Channel options consumed by interrupted webhook recovery, but retained the incompatible chat capability in the same fixture.
gpt-5.6-sol ｜ 𝕏
There was a problem hiding this comment.
Important
The new loading-placeholder and restart regressions still leave focused Agent checks red.
Reviewed changes since the prior Pullfrog review at 4f0183156, covering commits 10759bb64 and ff0f51ba0.
- Formatted resolved loading placeholders: Applied citation cleanup to static, array-selected, and resolver-produced loading text across native and manual delivery setup.
- Expanded loading regression coverage: Added a matrix for both loading option names, three configuration forms, and native or manual delivery.
- Corrected restart fixture shape: Removed the incompatible chat capability from the Channel-based citation notice scenario.
gpt-5.6-sol ｜ 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes since the prior Pullfrog review at ff0f51ba0, covering the two test corrections that close the remaining findings.
- Selected restart fallback by error code: Used the supported
errorFallbackTextresolver contract soHOST_RESTARTEDdelivers and formats the citation-bearing notice. - Limited loading coverage to valid delivery modes: Kept native streaming on
fallbackStreamingPlaceholderTextand exercisedmessages.loadingthrough manual non-streaming delivery across static, array, and resolver forms. - Verified focused regressions: Passed all 22 selected loading-placeholder and host-restart cases in
providers.test.ts.
gpt-5.6-sol ｜ 𝕏
Teams receives Codex native citation markers such as
citeturn228505view0turn395856view0as plain text. Updating the T3 provider runtime alone cannot turn those opaque IDs into links. The documented app-server protocol provides assistant text and web-search actions without a citation-ID-to-URL map.Request verified Markdown source links in the Teams Channel instructions. At Chat SDK delivery, replace unresolved native citations with
[source link unavailable]while preserving explicit links and attachments. Cover split or truncated stream markers, final and finish-hook replies, loading placeholders, error/restart notices, and visible AST/card text. Preserve card URLs, action payloads, and caller-owned objects. Compare formatted text when suppressing duplicate replies and record the delivered text in delivery observations. Stream cancellation completes without waiting for a stalled source read and requests source cleanup.For example,
Answer. citeturn0view0 See [the PR](https://github.com/acme/portal/pull/1188).becomesAnswer. [source link unavailable] See [the PR](https://github.com/acme/portal/pull/1188).This prevents unreadable output; it does not invent or recover citation URLs.Local baseline verification reproduced the raw screenshot markers on main in both static and streaming delivery. Final revision validation passed in CI. All verification jobs passed: all three Agent shards, every other package shard, repository lint/types, TypeScript doctor, contracts/examples, and docs checks. The Agent shards passed 7,442 tests with one skipped across 222 files. These include all 541 webhook tests, 307 provider tests, and the citation, Channel delivery, and Channel instruction suites. Nine loading-placeholder cases cover supported native/manual delivery and static/array/resolver options. Restart notices use the supported
HOST_RESTARTEDfallback resolver. Cancellation regressions include both a custom stalled iterator and a real async generator. The latest review found no new issues and independently passed 22 loading/restart cases.An earlier local full-suite run was stopped after source changes and does not validate the final code. The duplicate focused local job was cancelled while queued after CI completed its checks. No live Teams deployment was performed. The complete final CI run and latest review are green.
The T3 fork resync is now on
mainatee5848bfdff74af1d14ad9464403d275e47f8822, pushed directly as requested by the maintainer. GitHub automatically marked the earlier resync PR as merged. This PR already pins the same published preview, so no dependency change or repeat validation is needed. Its runtime build, typecheck, and 125 focused provider tests pass, and its preview packages passed publint. Inherited upstream CI remains queued on its configured runners. Upstreammainis already included in the fork.Model: GPT-6.1-Sol. Harness: Codex through T3 Code.