Skip to content

fix(cli): preserve JSON while escaping human output - #6459

Merged
lidge-jun merged 1 commit into
devfrom
carry/fork-697-terminal-output
Oct 3, 2026
Merged

lidge-jun merged 1 commit into
devfrom
carry/fork-697-terminal-output

Conversation

@luvs01

@luvs01 luvs01 commented Oct 2, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • Prevent role and delegation-model suggestion text from emitting raw terminal controls in human-readable CLI output. Based on dev at e0af52c8a2701dccd81fa5e672c92744e59d2e29.
  • Escape terminal controls at the shared human-output renderer while preserving exact JSON values. Keep exported native documents multiline by passing individual lines.
  • Cover both role/injection JSON comparisons, separate human-output assertions, and export framing regression. Retain the existing CLI diagnostics documentation.

Verification

Linux / Bun 1.4.0, exact tree d2164af2e7131154328e600b0d947cde97b5fcbb:

  • bun test tests/cli/cli-headless-parity.test.ts tests/cli/cli-export-command.test.ts: 126 pass, 0 fail, 513 assertions.
  • Negative control: removing only shared human-output escaping makes the two suggestion cases fail. Source restored afterward.
  • bun node_modules/typescript/bin/tsc --noEmit, bun scripts/structure-ssot.ts, bun scripts/privacy-scan.ts, bun scripts/file-size-ratchet.ts, and git diff --cached --check: pass.
  • bun scripts/test.ts --changed=e0af52c8a2701dccd81fa5e672c92744e59d2e29: 1520 pass, 1 skip, 35 fail across 87 files. Coordinator/client failures report the sandbox's untrusted /tmp ownership; Claude integration assertions also failed. This is not a green broad run, and the complete failure set has not been independently cleared.
  • Two earlier concurrent changed-test attempts were interrupted during shared-runner contention; they are incomplete, not passing evidence. No full-suite or native Windows/macOS pass is claimed. Hosted checks associated with PR head db774c7da4e45c26555c141b7260ef225aca08c0 have passed: Cross-platform CI, including its ci aggregate, and Enforce PR target branch, attempt 2. The latest check inventory is 21 successful and 9 scope-skipped jobs; skipped platform lanes are not claimed as tested.

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.

Co-authored-by: Epinephrine luvs01@hanmail.net
Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

Summary by CodeRabbit

  • Bug Fixes
    • Human-readable CLI output now safely escapes terminal-control characters while preserving multiline document formatting.
    • JSON output continues to preserve original values, including control characters.
  • Tests
    • Added coverage for safe human-readable suggestions and consistency between exported documents and structured JSON output.
  • Documentation
    • Clarified how human-readable and JSON CLI output handle escaping and multiline content.

Maintainer integration evidence

Owner lidge-jun elects maintainer integration into dev under the recorded repository policy and current user authorization. The five changed file blobs match the independently reviewed recovery carry. No unresolved review threads or maintainer change requests were found. Exact-head pull_request CI run 37018199273, attempt 1, completed successfully at db774c7da4e45c26555c141b7260ef225aca08c0; all requested jobs passed, platform-unrequested jobs remain skipped rather than claimed executed. CLI human-output escaping preserves JSON behavior, with focused recovery coverage. This decision is maintainer integration, not self-approval.

Carry luvs01#697 onto current dev, preserving its implementation and regression coverage.

Co-authored-by: Epinephrine <luvs01@hanmail.net>
Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@coderabbitai

coderabbitai Bot commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: lidge-jun/opencodex/.coderabbit.yaml
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: a34bdacb-02b8-4780-9c7e-c0b1cc51d9e4
📥 Commits

Reviewing files that changed from the base of the PR and between e0af52c and db774c7.

📒 Files selected for processing (5)
  • src/cli/export-command.ts
  • src/cli/runtime-api.ts
  • structure/runtime.md
  • tests/cli/cli-export-command.test.ts
  • tests/cli/cli-headless-parity.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.


📝 Walkthrough

Walkthrough

Human-readable CLI output now escapes terminal control characters while JSON output retains original values. Exported documents are passed as individual lines to preserve multiline formatting.

Changes

CLI output behavior

Layer / File(s) Summary
Escape control characters in human output
src/cli/runtime-api.ts, tests/cli/cli-headless-parity.test.ts, structure/runtime.md
printData escapes human-readable lines with terminalSafeText. Tests cover control characters in role and injection suggestion output. The documentation describes escaping in human output and preservation of original JSON values.
Preserve exported document line layout
src/cli/export-command.ts, tests/cli/cli-export-command.test.ts
The export command passes the trimmed serialized document as individual lines. The test checks that the JSON document at the start of human output matches the structured JSON export.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to db774

The CLI changes preserve JSON values while escaping human-readable output and retaining exported document layout. No identified issue remains that should prevent merging after normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to db774

Human-readable output now escapes terminal controls while JSON values remain unchanged. The remaining terminal-output finding predates this change, and the inspected paths do not show increased exposure. Its practical impact depends on how a terminal displays structured output.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The residual attack path concerns an operator terminal displaying attacker-influenced structured output. Effective impact depends on terminal interpretation of surviving controls. The inspected comparison does not show new suggestion callers, greater authority, or expanded exposure.

Security Findings and Attack Paths

  • observed — The retained low-severity finding concerns JSON output bypassing terminalSafeText. JSON serialization escapes C0 controls but can preserve C1 characters such as U+009B. This serialization behavior and the inspected suggestion caller are unchanged from the base, so the finding is a pre-existing security condition rather than an active PR architecture concern. Reachability and likelihood remain unknown in the supplied assessment.

Trust Boundaries and Controls

  • observed — The changed control sits between supplied human text and terminal output. Structured output deliberately retains the data contract rather than inheriting human-rendering escapes; therefore, the human-output protection must not be treated as protection for every output mode.

Resilience and Maintainability Implications

  • observed — The inspected renderer and escaping helper perform synchronous presentation work without mutating configuration or introducing a persistence, ownership, rollback, or recovery transition.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 4 files. (1 skipped: 1 … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
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.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: escaping terminal controls in human-readable CLI output while preserving JSON values.
Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 4 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


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.

@github-actions

github-actions Bot commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

✅ Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the bug Something isn't working label Oct 2, 2026
@luvs01
luvs01 marked this pull request as ready for review October 2, 2026 23:44
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 2, 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 ⚠️ Failed 2026-10-02T23:48:25.695288Z db774c7 Draft marked ready
ℹ️ 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.

@lidge-jun
lidge-jun merged commit ee2e15f into dev Oct 3, 2026
46 of 47 checks passed
@lidge-jun
lidge-jun deleted the carry/fork-697-terminal-output branch October 3, 2026 06:24
robin-bially pushed a commit to robin-bially/opencodex that referenced this pull request Oct 3, 2026
)

Suggestion rationale could emit terminal controls in human-readable output.
Escape shared human lines while preserving JSON values and multiline exports.
Add public guidance alongside the contributor regression tests.

Carries lidge-jun#6459 by @luvs01.
Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants