feat(agents): plan checklist renderer — 4 formats, all statuses, activeForm [Phase 3.B]#67534
feat(agents): plan checklist renderer — 4 formats, all statuses, activeForm [Phase 3.B]#67534100yenadmin wants to merge 10 commits into
Conversation
…bility Phase 3.B of the GPT 5.4 parity sprint. Adds a renderPlanChecklist() utility that produces formatted checklists from AgentPlanEventData for every delivery surface: - html: Telegram HTML parse mode (emoji + <b>/<s> tags) - markdown: GitHub-flavored checkboxes (Control UI, Matrix) - plaintext: ASCII markers (iMessage, BlueBubbles, SMS) - slack-mrkdwn: Slack's native formatting (*bold*, ~strike~) Supports all four plan statuses from #67514: - pending (⬚), in_progress (⏳), completed (✅), cancelled (❌) - Uses activeForm for in_progress steps when available - HTML-escapes user content to prevent injection Includes renderPlanWithHeader() for titled checklists. 23 unit tests covering all formats, all statuses, edge cases. This is the core renderer. Channel adapter wiring follows once #67514 (task-system parity) merges and the extended AgentPlanEventData type is available. Tracking: #66345, #67519.
There was a problem hiding this comment.
Pull request overview
Adds a core, format-agnostic plan checklist renderer under src/agents/ to support cross-channel visibility of structured agent plans (Phase 3.B), along with unit tests to validate output across formats/statuses.
Changes:
- Introduces
renderPlanChecklist()to render plan steps in HTML, Markdown, plaintext, and Slack mrkdwn. - Adds
renderPlanWithHeader()to prepend a format-appropriate header to rendered checklists. - Adds Vitest coverage for all formats, all statuses,
activeFormbehavior, and HTML escaping.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/agents/plan-render.ts | New plan rendering utilities for multiple channel-oriented text formats. |
| src/agents/plan-render.test.ts | Unit tests validating renderer output across formats, statuses, and edge cases. |
![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: 6e7675df0b
ℹ️ 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".
Greptile SummaryAdds Confidence Score: 5/5Safe to merge — only P2 findings remain; no logic errors, security issues, or broken paths. All findings are P2: a missing src/agents/plan-render.ts — Prompt To Fix All With AIThis is a comment left during a code review.
Path: src/agents/plan-render.ts
Line: 175
Comment:
**`~` missing from `escapeMarkdown` character class**
GFM strikethrough is triggered by `~~text~~`. A step label containing `~~` passes through unescaped, so `"Deploy ~~now~~"` renders as `- [ ] Deploy ~~now~~` (with "now" struck through), and inside a cancelled row it corrupts the renderer-added `~~…~~` wrapper.
```suggestion
return text.replace(/[\\`*_~{}[\]()#+\-.!<>|]/g, "\\$&");
```
The test suite covers backticks, brackets, and emphasis but has no `~~` case — worth adding one alongside this fix.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: src/agents/plan-render.ts
Line: 187-196
Comment:
**Module-level `warnedStatuses` Set leaks between test runs**
`warnedStatuses` is never cleared, so any test that exercises the unknown-status path (e.g. a future JS caller bypassing types) will permanently suppress the warning for every subsequent test in the same process under `--isolate=false`. CLAUDE.md flags module state as something tests must be able to clean up.
The Set is purely a deduplication guard for a defensive console path that TypeScript callers can never reach anyway. Exporting a `resetWarningCacheForTest()` helper (or moving it to a closure inside a factory if the module grows) keeps the guard without the bleed risk.
How can I resolve this? If you propose a fix, please make it concise.Reviews (2): Last reviewed commit: "fix(agents): #67534 — neutralize @mentio..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
…title Step text containing *, ~, `, or <...> tokens would break Slack formatting or trigger unintended mentions/links. Add escapeSlackMrkdwn() and apply it to both step labels and the header title in slack-mrkdwn format.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ecd540190d
ℹ️ 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".
…sing Tool names like update_user_profile render as italic in Slack without escaping. Replace _ with fullwidth low line (U+FF3F) which is visually identical but not parsed as mrkdwn emphasis.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6496cd3b8d
ℹ️ 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".
Model output for plan steps can contain embedded newlines which break the single-line checklist rendering across all formats. Strip \n and \r from labels before rendering. Added test covering multi-line step text and activeForm.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6b0bf6e328
ℹ️ 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".
Replace @ with U+FE6B (small form variant) in escapeSlackMrkdwn to prevent model-generated plan text from triggering @here/@channel/@everyone.
|
Merge order: 4/8. See #66345 for the full merge sequence. |
…095517064) Codex P2 (PR #67534 r3095517064): > The plaintext renderer intentionally neutralizes @channel/@here/@everyone > in step labels, but the header path emits safeTitle verbatim. When callers > pass model- or user-derived titles (for example '@everyone release plan'), > plaintext output can still trigger mentions even though checklist lines > are protected. Fix: pipe safeTitle through neutralizeMentions() in the plaintext branch of renderPlanWithHeader. (HTML/markdown/slack-mrkdwn branches escape via escapeHtml/escapeSlackMrkdwn which already neutralize the mention forms relevant to those formats.) Also: dropped a stale 'eslint-disable-next-line no-console' directive on warnUnknownStatus that lint flagged as unused. Tests: 2 new in plan-render.test.ts: - '@everyone release plan' title in plaintext does not match /@everyone\\b/ - '@channel deploy now' / '@here urgent' both neutralized in title 40 tests pass.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6069a036fe
ℹ️ 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".
Plan-mode rollout series — status updateThis PR is part of a 10-PR plan-mode rollout. The cumulative state ships locally as Series overview + landing order: see #68441 (latest comment). This PR's role in the series: see the original PR description above. Live test status: covered by the cumulative install — all behaviors from this PR are verified in production-like usage on Telegram + webchat. No regressions detected since the live install (2026-04-18). |
📋 Architecture & Status — single source of truth (commit
|
|
To use Codex here, create an environment for this repo. |
| /** Escapes Slack mrkdwn control characters: *, ~, `, _, and angle brackets. */ | ||
| function escapeSlackMrkdwn(text: string): string { | ||
| // Replace angle-bracket tokens first, then mrkdwn formatting chars. | ||
| return text | ||
| .replace(/&/g, "&") | ||
| .replace(/</g, "<") | ||
| .replace(/>/g, ">") | ||
| .replace(/\*/g, "\u2217") // ∗ (asterisk operator, visually similar) | ||
| .replace(/~/g, "\u223C") // ∼ (tilde operator) | ||
| .replace(/`/g, "\u2018") // ' (left single quote) | ||
| .replace(/_/g, "\uFF3F") // ＿ (fullwidth low line, prevents italic parse) | ||
| .replace(/@/g, "\uFE6B"); // ﹫ (small form variant, prevents mention parsing) | ||
| } |
There was a problem hiding this comment.
This file defines its own escapeSlackMrkdwn() that replaces characters with Unicode lookalikes and also rewrites @. The repo already has an established Slack mrkdwn escaping helper (extensions/slack/src/monitor/mrkdwn.ts) that backslash-escapes control chars and escapes &<>. To avoid drift and confusion (same name, different semantics), consider either reusing a shared helper (move to a common module) or aligning this implementation + tests with the established semantics.
| const markers: Record<PlanStepForRender["status"], string> = { | ||
| completed: "[x]", | ||
| in_progress: "[>]", | ||
| cancelled: "[~]", | ||
| pending: "[ ]", | ||
| }; | ||
| if (!Object.hasOwn(markers, s.status)) { | ||
| warnUnknownStatus(s.status); | ||
| } | ||
| return `${markers[s.status] ?? "[ ]"} ${safe}`; |
There was a problem hiding this comment.
Unknown-status warning/handling is currently only implemented in the plaintext branch via Object.hasOwn(markers, s.status), but other formats silently treat unknown statuses as pending. If runtime data can be untrusted, consider validating/status-normalizing once before the switch (and warning consistently across formats), or removing the warning path if the status union is trusted.
| const markers: Record<PlanStepForRender["status"], string> = { | ||
| completed: "[x]", | ||
| in_progress: "[>]", |
There was a problem hiding this comment.
PR description examples say plaintext uses [✓] for completed, but the renderer outputs [x]. If downstream channel adapters/user-facing docs expect the checkmark variant, consider switching the completed marker or updating the documentation/examples to match this output.
|
Closing in favor of consolidated PR #68939. Rationale: this branch was 734 commits behind upstream/main and the The full iteration history + decision log lives in |
TL;DR
Core plan rendering utility that formats plan checklists for HTML (Telegram), Markdown (GitHub), plaintext (SMS), and Slack mrkdwn. Supports all 4 statuses (pending, in_progress, completed, cancelled) with `activeForm` live progress text.
Tracking
What this PR does
`renderPlanChecklist()` + `renderPlanWithHeader()` utilities that take a plan's step array and produce formatted output for any channel:
Deploy to staging` (cancelled)`activeForm` renders live progress: shows "Building artifacts" instead of "Build artifacts" during `in_progress`.
Files changed
23 unit tests
Dependencies
What follows