Skip to content

test(acp): add terminal delegation fixtures and fix shell singleton#7923

Merged
codefromthecrypt merged 5 commits intomainfrom
adrian/shell-terminal-tests
Mar 16, 2026
Merged

test(acp): add terminal delegation fixtures and fix shell singleton#7923
codefromthecrypt merged 5 commits intomainfrom
adrian/shell-terminal-tests

Conversation

@codefromthecrypt
Copy link
Copy Markdown
Collaborator

Summary

ACP terminal delegation had no test coverage. TerminalFixture and capability wiring follow the same pattern as the existing FS fixtures.

This also fixes a test flake in the shell codebase: user_login_path() cached the resolved PATH in a function-scoped OnceLock, leaking state across concurrent tests. Now a LazyLock with an instance field on ShellTool, matching PermissionManager.

Type of Change

  • Tests
  • Refactor / Code quality

AI Assistance

  • This PR was created or reviewed with AI assistance

Testing

  • shell_terminal_true: client advertises terminal, fixture validates create/wait/output/release lifecycle
  • shell_terminal_false: no capability, agent falls back to local subprocess

Related Issues

Builds on #7843

@codefromthecrypt codefromthecrypt mentioned this pull request Mar 16, 2026
3 tasks
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5ebeae1f40

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

@codefromthecrypt codefromthecrypt added this pull request to the merge queue Mar 16, 2026
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to a conflict with the base branch Mar 16, 2026
@codefromthecrypt codefromthecrypt force-pushed the adrian/shell-terminal-tests branch from 5ebeae1 to 401df47 Compare March 16, 2026 06:30
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 401df478ae

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 19b90afd73

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

@codefromthecrypt codefromthecrypt force-pushed the adrian/shell-terminal-tests branch from 19b90af to bafb231 Compare March 16, 2026 10:50
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: bafb2310ca

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

ACP terminal delegation had no test coverage. TerminalFixture and
capability wiring follow the same pattern as the existing FS fixtures.

Also fixes a test flake: user_login_path() cached the resolved PATH
in a function-scoped OnceLock, leaking state across concurrent tests.
Now a LazyLock with an instance field on ShellTool, matching
PermissionManager.

Signed-off-by: Adrian Cole <[email protected]>
Signed-off-by: Adrian Cole <[email protected]>
Signed-off-by: Adrian Cole <[email protected]>
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: dc01bd6917

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Signed-off-by: Adrian Cole <[email protected]>
@codefromthecrypt codefromthecrypt force-pushed the adrian/shell-terminal-tests branch from dc01bd6 to 869176c Compare March 16, 2026 12:28
Signed-off-by: Adrian Cole <[email protected]>
@codefromthecrypt codefromthecrypt added this pull request to the merge queue Mar 16, 2026
Merged via the queue into main with commit 94eaacc Mar 16, 2026
23 checks passed
@codefromthecrypt codefromthecrypt deleted the adrian/shell-terminal-tests branch March 16, 2026 14:14
wpfleger96 added a commit that referenced this pull request Mar 16, 2026
…oken-retry

* origin/main: (21 commits)
  Remove java/.ai-usage-marker directory (#7925)
  test(acp): add terminal delegation fixtures and fix shell singleton (#7923)
  fix: bump pctx_code_mode to 0.3.0 for iterator type checking fix (#7892)
  feat: persist GooseMode per-session via session DB (#7854)
  feat(otel): propagate session.id to spans and log records (#7490)
  fix(test): add env_lock to is_openai_reasoning_model tests (#7917)
  fix(acp): pass session_id when loading extensions so skills are discovered (#7868)
  updated canonical models (#7920)
  feat(autovisualiser): Migrate the autovisualiser extension to MCP Apps  (#7852)
  fix: add tool_choice and parallel_tool_calls to chatgpt_codex provider (#7867)
  fix: tool confirmation handling for multiple requests (#7856)
  Remove dead OllamaSetup onboarding flow (#7861)
  fix: resolve tokio::sync::Mutex deadlock in recipe retry path (#7832)
  Upgrade Electron 40.6.0 → 41.0.0 (#7851)
  Only show up to 50 lines of source code (#7578)
  fix: stop writing without error when hitting broken pipe for goose session list (#7858)
  feat(acp): add session/set_mode handler (#7801)
  Keep messages in sync (#7850)
  More acp tools (#7843)
  fix: skip upgrade-insecure-requests CSP for external HTTP backends (#7714)
  ...
wpfleger96 added a commit that referenced this pull request Mar 16, 2026
* origin/main: (72 commits)
  No Check do Check (#7942)
  Log 500 errors and also show error for direct download (#7936)
  fix: retry on authentication failure with credential refresh (#7812)
  Remove java/.ai-usage-marker directory (#7925)
  test(acp): add terminal delegation fixtures and fix shell singleton (#7923)
  fix: bump pctx_code_mode to 0.3.0 for iterator type checking fix (#7892)
  feat: persist GooseMode per-session via session DB (#7854)
  feat(otel): propagate session.id to spans and log records (#7490)
  fix(test): add env_lock to is_openai_reasoning_model tests (#7917)
  fix(acp): pass session_id when loading extensions so skills are discovered (#7868)
  updated canonical models (#7920)
  feat(autovisualiser): Migrate the autovisualiser extension to MCP Apps  (#7852)
  fix: add tool_choice and parallel_tool_calls to chatgpt_codex provider (#7867)
  fix: tool confirmation handling for multiple requests (#7856)
  Remove dead OllamaSetup onboarding flow (#7861)
  fix: resolve tokio::sync::Mutex deadlock in recipe retry path (#7832)
  Upgrade Electron 40.6.0 → 41.0.0 (#7851)
  Only show up to 50 lines of source code (#7578)
  fix: stop writing without error when hitting broken pipe for goose session list (#7858)
  feat(acp): add session/set_mode handler (#7801)
  ...
lifeizhou-ap added a commit that referenced this pull request Mar 17, 2026
* main:
  Add DCO git commit command to AGENTS.md (#7945)
  fix(claude-code): remove incorrect agent_visible filter on user message (#7931)
  No Check do Check (#7942)
  Log 500 errors and also show error for direct download (#7936)
  fix: retry on authentication failure with credential refresh (#7812)
  Remove java/.ai-usage-marker directory (#7925)
  test(acp): add terminal delegation fixtures and fix shell singleton (#7923)
  fix: bump pctx_code_mode to 0.3.0 for iterator type checking fix (#7892)
  feat: persist GooseMode per-session via session DB (#7854)
  feat(otel): propagate session.id to spans and log records (#7490)
  fix(test): add env_lock to is_openai_reasoning_model tests (#7917)
  fix(acp): pass session_id when loading extensions so skills are discovered (#7868)
jh-block added a commit that referenced this pull request Mar 17, 2026
* origin/main:
  feat: adversarial agent for preventing leaking of info and more  (#7948)
  Update contributing.md (#7927)
  docs: add credit balance monitoring section (#7952)
  docs: add Cerebras provider to supported providers list (#7953)
  docs: add TUI client documentation to ACP clients guide (#7950)
  fix: removed double dash in pnpm command (#7951)
  docs: polish ACP docs (#7946)
  claude adaptive thinking (#7944)
  feat: new onboarding flow (#7266)
  Add DCO git commit command to AGENTS.md (#7945)
  fix(claude-code): remove incorrect agent_visible filter on user message (#7931)
  No Check do Check (#7942)
  Log 500 errors and also show error for direct download (#7936)
  fix: retry on authentication failure with credential refresh (#7812)
  Remove java/.ai-usage-marker directory (#7925)
  test(acp): add terminal delegation fixtures and fix shell singleton (#7923)
  fix: bump pctx_code_mode to 0.3.0 for iterator type checking fix (#7892)
  feat: persist GooseMode per-session via session DB (#7854)
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.

2 participants