Skip to content

fix(typecheck): narrow remote agent SDK logs - #1573

Merged
kevincodex1 merged 1 commit into
Twigpine:mainfrom
chioarub:codex/typecheck-remote-agent-sdk-logs
Jun 10, 2026
Merged

kevincodex1 merged 1 commit into
Twigpine:mainfrom
chioarub:codex/typecheck-remote-agent-sdk-logs

Conversation

@chioarub

@chioarub chioarub commented Jun 9, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Addresses part of #1486 by narrowing SDK-generated remote-agent log content before scanning it.

  • adds local guards for assistant text blocks and TodoWrite tool-use blocks
  • uses the guards when extracting ultraplan, remote-review, and live delta text
  • keeps TodoWrite parsing behavior unchanged while avoiding direct property reads on SDK unknown[] content

Duplicate PR check

Checked all open PRs in Gitlawb/openclaude before opening this branch. No open PR touches src/tasks/RemoteAgentTask/RemoteAgentTask.tsx, and no open PR title appears to target this remote-agent SDK log typecheck cluster.

Validation

Ran bun run typecheck on this branch.

  • src/tasks/RemoteAgentTask/RemoteAgentTask.tsx diagnostics: 10 -> 0
  • total src/ diagnostics: 1043 -> 1033

The full command still exits nonzero because #1486 has unrelated remaining diagnostics outside this PR's scope.

Summary by CodeRabbit

  • Refactor
    • Enhanced internal content extraction and parsing mechanisms for improved reliability and maintainability.

@coderabbitai

coderabbitai Bot commented Jun 9, 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: e5605dad-dda6-4380-9e6b-edb3ef4a2c61

📥 Commits

Reviewing files that changed from the base of the PR and between 9e942da and 53b24cc.

📒 Files selected for processing (1)
  • src/tasks/RemoteAgentTask/RemoteAgentTask.tsx
📜 Recent review details
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: smoke-and-tests
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx,py}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

**/*.{ts,tsx,js,jsx,py}: Follow the existing code style in the touched files
Keep comments useful and concise

Files:

  • src/tasks/RemoteAgentTask/RemoteAgentTask.tsx
**/*

⚙️ CodeRabbit configuration file

**/*: Apply the OpenClaude maintainer review rubric from AGENTS.md. Review the current diff, not stale discussion context. Separate real blockers from suggestions. Do not request changes for vague style churn. Treat approval as merge-ready from CodeRabbit's side, pending required human review and GitHub Checks. If checks are failing or unavailable, say so clearly instead of implying the PR is fully ready.

Files:

  • src/tasks/RemoteAgentTask/RemoteAgentTask.tsx
**

⚙️ CodeRabbit configuration file

**: # Contributing to OpenClaude

Thanks for contributing.

OpenClaude is a fast-moving open-source coding-agent CLI with support for multiple providers, local backends, MCP, and a terminal-first workflow. The best contributions here are focused, well-tested, and easy to review.

Before You Start

  • Search existing issues and discussions before opening a new thread.
  • Check open pull requests for work that overlaps with your contribution. If a PR already exists that addresses the same change, open an issue or discussion first to align on direction — duplicate PRs may be closed without review.
  • Use issues for confirmed bugs and actionable feature work.
  • Use discussions for setup help, ideas, and general community conversation.
  • For larger changes, open an issue first so the scope is clear before implementation.
  • For security reports, follow SECURITY.md.

Pull Requests

Every PR needs a reason. Your PR description must include:

  • what changed and why
  • the user or developer impact
  • the exact checks you ran
  • a linked issue when one exists, using Fixes fix: skip assertMinVersion for third-party providers #123, `Closes `#123, or another clear link
  • screenshots when the PR touches UI, terminal presentation, or the VS Code extension
  • which provider path was tested when the PR changes provider behavior

The PR author is responsible for ensuring their PR is merge-ready. PRs with merge conflicts will not be reviewed or approved until the conflicts are resolved.

Issues are the recommended starting point for anything non-trivial — opening one first helps avoid wasted effort if the change is out of scope or already being worked on. Small fixes, doc corrections, and obvious improvements can stand on their own without a linked issue, as long as the PR description explains the intent.

What Gets Closed Without Review

PRs may be closed without review...

Files:

  • src/tasks/RemoteAgentTask/RemoteAgentTask.tsx
🔇 Additional comments (1)
src/tasks/RemoteAgentTask/RemoteAgentTask.tsx (1)

70-78: LGTM!

Also applies to: 96-110, 237-237, 292-292, 305-305, 333-333, 390-394, 595-595


📝 Walkthrough

Walkthrough

RemoteAgentTask.tsx consolidates SDK message-content text extraction by replacing a generic extractTextContent helper with localized type guards and an internal extractSdkTextContent() function. The utility is applied uniformly across plan, review, review-tag, todo, and polling-delta parsing workflows.

Changes

Internal SDK Content Extraction

Layer / File(s) Summary
Type definitions and extraction utilities
src/tasks/RemoteAgentTask/RemoteAgentTask.tsx
Removes extractTextContent import, introduces local TextBlock and TodoWriteToolUseBlock type shapes, and adds isRecord, isTextContentBlock, and isTodoWriteToolUseBlock type guards. Adds extractSdkTextContent() to filter and join text-only blocks from message content arrays.
Parsing functions migrated to extractSdkTextContent()
src/tasks/RemoteAgentTask/RemoteAgentTask.tsx
Plan extraction, review assembly, review-tag per-message scan, todo list detection (using new isTodoWriteToolUseBlock guard), and remote polling delta text generation all switch to the new utility for consistent SDK block text handling.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested reviewers

  • jatmn
  • kevincodex1
🚥 Pre-merge checks | ✅ 6 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (6 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main change: fixing TypeScript typecheck issues in the RemoteAgentTask by narrowing SDK-generated log content types.
Description check ✅ Passed The description covers the summary, validation results, and PR scope, but is missing the Impact and Testing sections from the repository template.
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.
Risk Surface Disclosed ✅ Passed PR modifies text extraction logic only. No changes to auth, permissions, routing, network, background execution, startup, MCP, CI, or release—all risk surfaces excluded.
No Hidden Policy Change ✅ Passed PR adds type guards for SDK content extraction without changing policy. Purely type-safe refactoring—no product, trust-model, routing, telemetry, or permission policy changes.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@jatmn jatmn left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution. I do not see any actionable issues from my review.
@kevincodex1 LGTM

@kevincodex1
kevincodex1 merged commit 7727a9f into Twigpine:main Jun 10, 2026
3 checks passed
@chioarub
chioarub deleted the codex/typecheck-remote-agent-sdk-logs branch June 10, 2026 05:56
deagwon97 pushed a commit to deagwon97/openclaude that referenced this pull request Jun 11, 2026
hotmanxp added a commit to hotmanxp/openclaude that referenced this pull request Jun 12, 2026
3way apply failed on all 3 (fork has heavily diverged from upstream
context). Per skill's 'direct overwrite via git show' escape hatch,
took upstream verbatim + re-evaluated byte-level.

src/grpc/server.ts (upstream Twigpine#1572 gRPC stream messages):
- 78 lines added (proper typing for PermissionDenyDecision, msg.event,
  block structures)
- 15 lines removed (fork's looser typing escape hatches)
- @ts-nocheck dropped

src/tasks/RemoteAgentTask/RemoteAgentTask.tsx (upstream Twigpine#1573):
- 33 lines added (typed message block structure, narrowed content types)
- 10 lines removed (fork's any[] escape hatches)
- @ts-nocheck dropped

src/utils/sessionStorage.test.ts (upstream Twigpine#1526):
- 322 lines added (proper UUID template literal types, typed test
  fixtures: SessionMessage[] instead of string)
- 42 lines removed (fork's nocheck helper functions + persisted-output
  test cases; can be re-added in a follow-up session if needed)
- @ts-nocheck dropped

VERIFICATION:
  typecheck: 47 errors → 0 from these 3 files
  tests: pre-existing acorn-module-missing failure (NOT from these
         changes — confirmed via git stash + bun test on clean HEAD
         which also fails with the same error)

REGRESSION RISK:
  sessionStorage.test.ts lost 42 lines of fork test coverage
  (helper functions id/base/user/assistant + 1 persisted-output test
  case). All TypeScript-clean, but runtime test count drops. Re-add
  in dedicated session if needed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants