Skip to content

fix(vscode): send schema-valid permission responses - #1401

Merged
kevincodex1 merged 1 commit into
Twigpine:mainfrom
chioarub:fix/1034-vscode-permission-response
Jun 3, 2026
Merged

fix(vscode): send schema-valid permission responses#1401
kevincodex1 merged 1 commit into
Twigpine:mainfrom
chioarub:fix/1034-vscode-permission-response

Conversation

@chioarub

@chioarub chioarub commented May 27, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Fix VS Code chat permission approvals so Allow and Allow for session resolve tool prompts instead of looping.
  • Send schema-valid can_use_tool control responses with behavior, updatedInput, and session updatedPermissions when applicable.
  • Run the extension package test script with Bun's recursive src discovery.

Issue

Fixes #1034

What Changed

  • Added permissionResponse.js helper to build SDK permission results.
  • Store pending control request input and suggestions by request_id in ChatController.
  • Updated sendPermissionResponse to emit correct allow/deny payloads.
  • Updated the VS Code extension test script to run the full extension test set under Bun.

Validation

  • cd vscode-extension/openclaude-vscode && bun test src/chat/permissionResponse.test.js — 3 pass, 0 fail
  • cd vscode-extension/openclaude-vscode && bun run test — 42 pass, 0 fail
  • cd vscode-extension/openclaude-vscode && bun run lint
  • git diff --check

Risk

  • Low — localized VS Code host protocol fix; deny path now uses explicit behavior: 'deny' instead of control error subtype.

jatmn
jatmn approved these changes Jun 2, 2026
View reviewed changes

@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 approved these changes Jun 3, 2026
View reviewed changes

@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 8065f8d into Twigpine:main Jun 3, 2026
2 checks passed
@github-actions github-actions Bot mentioned this pull request Jun 3, 2026
Merged
@chioarub
chioarub deleted the fix/1034-vscode-permission-response branch June 3, 2026 11:42
hotmanxp added a commit to hotmanxp/openclaude that referenced this pull request Jun 7, 2026
@hotmanxp
sync upstream tier 2: API retry + vscode permission fixes (2 commits)
939802d 
Upstream tier 2 KEEPs, applied 2 of 7 candidates (5 already applied
under prior syncs but missed by subject-match dedup):

  f1013df fix(api): honor OpenAI-compatible retry classification (Twigpine#1547)
  8065f8d fix(vscode): send schema-valid permission responses (Twigpine#1401)

Already-applied (real DIFFERS = local fork divergence only):
  1fc5116 fix(api): tighten reasoning_content heuristic (Twigpine#1201)  -- byte-equal
  2bed184 perf(attachments): skip skill listings for utility forks (Twigpine#1545)
                -- diff is feature('TRANSCRIPT_CLASSIFIER')→true + CLAUDE.md→AGENTS.md
  8416faa fix(BashTool): include captured output in non-zero-exit error (Twigpine#1249)
                -- diff is feature('MONITOR_TOOL')→drop + local @ts-ignore
  f7d42c2 fix(cron): enforce MAX_CRON_PROMPT_CHARS cap (Twigpine#1224)
                -- diff is 'Claude'→'Open CC' brand string in description

Notes:
- f1013df: openaiErrorClassification.ts adds RETRYABLE_OPENAI_COMPATIBILITY_
  FAILURE_CATEGORIES set + isRetryableOpenAICompatibilityFailureCategory();
  withRetry.ts integrates the new classifier. withRetry.test.ts had a
  merge conflict in the 'retry configuration' describe block (upstream
  added OPENCLAUDE_MAX_RETRIES/OPENCLAUDE_RETRY_DELAY_MS env var tests
  that fork doesn't support -- AGENTS.md keeps CLAUDE_CODE_* env vars).
  Resolved by KEEPING THEIRS for the new 'OpenAI-compatible retry
  classification' block, then DELETED the 'retry configuration' block
  (9 unsupported tests) per fork policy.
- 8065f8d: 2 new files (permissionResponse.js + .test.js) pulled; 2
  existing files patched cleanly. Fork has 2 separate vscode extensions
  (opencc-vscode v0.1.1 simplified + openclaude-vscode v0.2.0 full); the
  upstream sync only touches the openclaude-vscode chat/ subdir.
  permissionResponse.test.js uses jest-style globals; bun test passes
  3/3 (bun:test is jest-compatible for describe/it/expect).

Verification:
  typecheck:  0 errors
  bun test:   2532 pass / 0 fail / 34 skip (full suite)
  build:      Built v0.16.1 → dist/cli.mjs
  vscode:     permissionResponse.test.js 3/3 pass
  naming:     no new openclaude/gitlawb leaks (pre-existing
              openclaude-vscode dir name is intentional)
hotmanxp added a commit to hotmanxp/openclaude that referenced this pull request Jun 11, 2026
@hotmanxp
sync upstream tier 2: API retry + vscode permission fixes (2 commits)
29bb266 
Upstream tier 2 KEEPs, applied 2 of 7 candidates (5 already applied
under prior syncs but missed by subject-match dedup):

  f1013df fix(api): honor OpenAI-compatible retry classification (Twigpine#1547)
  8065f8d fix(vscode): send schema-valid permission responses (Twigpine#1401)

Already-applied (real DIFFERS = local fork divergence only):
  1fc5116 fix(api): tighten reasoning_content heuristic (Twigpine#1201)  -- byte-equal
  2bed184 perf(attachments): skip skill listings for utility forks (Twigpine#1545)
                -- diff is feature('TRANSCRIPT_CLASSIFIER')→true + CLAUDE.md→AGENTS.md
  8416faa fix(BashTool): include captured output in non-zero-exit error (Twigpine#1249)
                -- diff is feature('MONITOR_TOOL')→drop + local @ts-ignore
  f7d42c2 fix(cron): enforce MAX_CRON_PROMPT_CHARS cap (Twigpine#1224)
                -- diff is 'Claude'→'Open CC' brand string in description

Notes:
- f1013df: openaiErrorClassification.ts adds RETRYABLE_OPENAI_COMPATIBILITY_
  FAILURE_CATEGORIES set + isRetryableOpenAICompatibilityFailureCategory();
  withRetry.ts integrates the new classifier. withRetry.test.ts had a
  merge conflict in the 'retry configuration' describe block (upstream
  added OPENCLAUDE_MAX_RETRIES/OPENCLAUDE_RETRY_DELAY_MS env var tests
  that fork doesn't support -- AGENTS.md keeps CLAUDE_CODE_* env vars).
  Resolved by KEEPING THEIRS for the new 'OpenAI-compatible retry
  classification' block, then DELETED the 'retry configuration' block
  (9 unsupported tests) per fork policy.
- 8065f8d: 2 new files (permissionResponse.js + .test.js) pulled; 2
  existing files patched cleanly. Fork has 2 separate vscode extensions
  (opencc-vscode v0.1.1 simplified + openclaude-vscode v0.2.0 full); the
  upstream sync only touches the openclaude-vscode chat/ subdir.
  permissionResponse.test.js uses jest-style globals; bun test passes
  3/3 (bun:test is jest-compatible for describe/it/expect).

Verification:
  typecheck:  0 errors
  bun test:   2532 pass / 0 fail / 34 skip (full suite)
  build:      Built v0.16.1 → dist/cli.mjs
  vscode:     permissionResponse.test.js 3/3 pass
  naming:     no new openclaude/gitlawb leaks (pre-existing
              openclaude-vscode dir name is intentional)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jatmn jatmn jatmn approved these changes
@kevincodex1 kevincodex1 kevincodex1 approved these changes
@Vasanthdev2004 Vasanthdev2004 Awaiting requested review from Vasanthdev2004

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

The VS Code extension plugin cannot access the disk, and always prompts for permission approval

3 participants

@chioarub @jatmn @kevincodex1