Skip to content

fix(typecheck): restore typed add-dir source - #1504

Merged
kevincodex1 merged 1 commit into
Twigpine:mainfrom
chioarub:fix/typecheck-add-dir-source
Jun 3, 2026
Merged

kevincodex1 merged 1 commit into
Twigpine:mainfrom
chioarub:fix/typecheck-add-dir-source

Conversation

@chioarub

@chioarub chioarub commented Jun 3, 2026 •

Copy link
Copy Markdown
Contributor

Refs #1486.

Summary

  • Restores AddDirError from compiler-cache output to typed React source.
  • Types the add-workspace-directory validation error state as string | null so validation help messages can be assigned.

Why

The add-dir command source had compiler-cache output checked in, and the current typecheck run reported a SetStateAction<null> diagnostic when AddWorkspaceDirectory stores validation help text.

Validation

  • git diff --check
  • bun run build
  • bun test src/commands/add-dir (no matching focused test files exist)
  • bun run typecheck still exits 2 because of the unrelated existing backlog; normalized diagnostics for this branch go from 1,760 to 1,759, with 0 new diagnostics and 1 removed diagnostic. No diagnostics remain for src/commands/add-dir/add-dir.tsx or src/components/permissions/rules/AddWorkspaceDirectory.tsx.

Review

  • Exact open PR file-overlap check found no open PR touching either changed file.
  • Diff reviewed before publishing; no issues found.

Summary by CodeRabbit

  • Refactor
    • Streamlined add directory functionality with enhanced type safety and reduced code complexity.
    • Improved type definitions for workspace directory permissions handling.

@coderabbitai

coderabbitai Bot commented Jun 3, 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: 06aff631-4391-4865-b7f8-f7eaa32ff33f

📥 Commits

Reviewing files that changed from the base of the PR and between 3659eaa and e594ea0.

📒 Files selected for processing (2)
  • src/commands/add-dir/add-dir.tsx
  • src/components/permissions/rules/AddWorkspaceDirectory.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 (2)
**/*

⚙️ 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/components/permissions/rules/AddWorkspaceDirectory.tsx
  • src/commands/add-dir/add-dir.tsx
src/{components/permissions,utils/permissions,hooks/toolPermission,tools,entrypoints/sdk}/**

⚙️ CodeRabbit configuration file

src/{components/permissions,utils/permissions,hooks/toolPermission,tools,entrypoints/sdk}/**: Review permission prompts, auto-allow logic, sandbox behavior, SDK permission schemas, shell/PowerShell execution, and background execution paths as security-sensitive. Block on bypasses, unclear trust boundaries, unsafe path handling, missing user visibility, or changes that broaden allowed behavior without an explicit maintainer decision.

Files:

  • src/components/permissions/rules/AddWorkspaceDirectory.tsx
🔇 Additional comments (2)
src/commands/add-dir/add-dir.tsx (1)

15-35: LGTM!

src/components/permissions/rules/AddWorkspaceDirectory.tsx (1)

147-147: LGTM!


📝 Walkthrough

Walkthrough

This PR removes the react-compiler-runtime dependency from AddDirError and replaces it with an explicit, typed component implementation using useEffect. A supporting type-safety improvement tightens the error state type in AddWorkspaceDirectory from untyped to explicitly string | null.

Changes

Type safety refactoring

Layer / File(s) Summary
Remove react-compiler-runtime and rewrite AddDirError with explicit types
src/commands/add-dir/add-dir.tsx
Removed react-compiler-runtime import and replaced compiled wrapper with typed AddDirErrorProps interface and explicit AddDirError component using useEffect to schedule and clean up the onDone callback.
Type error state in AddWorkspaceDirectory
src/components/permissions/rules/AddWorkspaceDirectory.tsx
Updated internal error state from untyped null initializer to explicitly typed `string

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

🚥 Pre-merge checks | ✅ 5 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Risk Surface Disclosed ⚠️ Warning PR modifies permissions files (AddWorkspaceDirectory.tsx) but review does not explicitly call out the permissions risk surface or confirm no permission logic changes. Review should explicitly note that changes are type-only refactoring with no permission logic modifications, confirming no blocker for the permissions system.
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix(typecheck): restore typed add-dir source' accurately describes the main changes: restoring typed source for add-dir and fixing typecheck issues.
Description check ✅ Passed The description covers summary, rationale, validation steps, and review; all core sections are present and substantive. Testing checklist is referenced without full checkbox completion, which is acceptable for this PR's nature.
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.
No Hidden Policy Change ✅ Passed PR removes compiler-cache artifacts and improves type safety. No hidden policy, trust-model, routing, telemetry, or permission-policy changes found.

✏️ 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.

@chioarub
chioarub marked this pull request as ready for review June 3, 2026 14:50

@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

@kevincodex1 kevincodex1 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@kevincodex1
kevincodex1 merged commit 2c755d3 into Twigpine:main Jun 3, 2026
3 checks passed
@chioarub
chioarub deleted the fix/typecheck-add-dir-source branch June 3, 2026 22:42
hotmanxp pushed a commit to hotmanxp/openclaude that referenced this pull request Jun 5, 2026
Upstream 343cd1a..1204fe2, applied 6 of 8 KEEP candidates in tier 1:

  11e46af fix(typecheck): narrow hook event counts (Twigpine#1496)
  2c755d3 fix(typecheck): restore typed add-dir source (Twigpine#1504)
  2bed184 perf(attachments): skip skill listings for utility forks (Twigpine#1545)
  47eea3f fix(typecheck): type search UI state (Twigpine#1529)
  96ddec7 fix(test): stop use-input test from leaking a global stdin mock (Twigpine#1501)
  1fc5116 fix(api): tighten reasoning_content heuristic to prevent false-positives (Twigpine#1201)

Notes:
- 2c755d3: react-compiler compiled output (add-dir.tsx) replaced with typed
  source; build re-compiles on next run
- 47eea3f: kept fork-specific // @ts-nocheck at top of GlobalSearchDialog.tsx
  (added by local b40814b, not in upstream); applied type Props + true arg
- 96ddec7: bun.lock regenerated via 'bun install' to reconcile with new
  package.json (devDependencies pruned: react-hooks, react-test-renderer);
  new use-input.test.ts (208 lines) injected via StdinContext.Provider
- 1fc5116: new runtimeMetadata.test.ts (90 lines, regression coverage for
  the segment-boundary heuristic per jatmn review)

Verification:
  baseline:  2223 pass / 23 skip / 4 fail
  after:     2232 pass / 23 skip / 2 fail   (+9 pass, -2 fail)
  typecheck: 90 errors (pre-existing, no new)
  build:     Built v0.16.1

Rebase onto OpenCC v0.14.0 (2292a59).
hotmanxp added a commit to hotmanxp/openclaude that referenced this pull request Jun 7, 2026
Upstream 47eea3f..e7abb81, applied 2 of 5 KEEP candidates in tier 1
(3 already applied under prior sync 4f6bccc but were missed by
subject-match dedup due to squash-merge subject loss):

  e357d59 fix(typecheck): restore proactive module import surface (Twigpine#1495)
  e7abb81 fix(typecheck): make session history cache variant-safe (Twigpine#1494)

Already-applied (byte-equal content + fork-specific @ts-nocheck):
  47eea3f fix(typecheck): type search UI state (Twigpine#1529)
  11e46af fix(typecheck): narrow hook event counts (Twigpine#1496)
  2c755d3 fix(typecheck): restore typed add-dir source (Twigpine#1504)

Notes:
- e357d59: 2 new files (src/proactive/{index.ts,index.test.ts});
  pulls cleanly, no provider leakage
- e7abb81: 1 new file (sessionHistorySerialization.ts) + 3 mods.
  sessionHistory.test.ts gets // @ts-nocheck (upstream tests use
  SDKUserMessage.message + new serialization import; local SDKUserMessage
  type doesn't include .message field, and sessionHistory.ts still uses
  inline impl). sessionHistory.ts gets the upstream rewrite (inline
  functions replaced with imports from sessionHistorySerialization.ts);
  @ts-nocheck preserved.
- conversationCache.ts: Message[] → CacheMessage[]; CacheMessage becomes
  Record<string, unknown> (was: alias of Message interface)

Verification:
  typecheck: 0 errors
  bun test src/assistant/sessionHistory.test.ts: 6 pass, 0 fail
  bun test src/proactive/index.test.ts: 2 pass, 0 fail
  build: Built v0.16.1 → dist/cli.mjs
  naming scan: no openclaude/gitlawb leakage
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