Skip to content

fix(claude_code): prevent the fullscreen-renderer onboarding upsell via settings #600

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
klabulan wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(claude_code): prevent the fullscreen-renderer onboarding upsell via settings #600

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 43 additions & 22 deletions src/cli_agent_orchestrator/providers/claude_code.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@

logger = logging.getLogger(__name__)

# Serializes concurrent _ensure_skip_bypass_prompt_setting() read-modify-writes to
# Serializes concurrent _ensure_startup_settings() read-modify-writes to
# ~/.claude/settings.json -- after the async conversion, N concurrent inits can run this
# in N threads (via asyncio.to_thread), and an unlocked read-modify-write can race: one
# thread reads while another is mid-write, decodes a truncated file, falls back to {}, and
Expand Down Expand Up @@ -459,21 +459,33 @@ def _build_claude_command(self, profile: Optional["AgentProfile"] = _UNSET) -> s
return f"{unset_cmd}; {claude_cmd}"

@staticmethod
def _ensure_skip_bypass_prompt_setting() -> None:
"""Ensure ``skipDangerousModePermissionPrompt`` is set in settings.

Claude Code (v2.1.41+) shows a bypass permissions confirmation dialog
on every launch with ``--dangerously-skip-permissions`` unless
``skipDangerousModePermissionPrompt: true`` is persisted in
``~/.claude/settings.json``. CAO already uses the flag intentionally,
so the confirmation is redundant and blocks initialization.

After the async conversion, N concurrent inits may run this
read-modify-write in N threads. ``_SETTINGS_WRITE_LOCK`` serializes
our own threads (in-process only: a second cao-server process, or
Claude Code itself, writing between our read and ``os.replace`` is
still a last-writer-wins lost update); ``os.replace`` only guarantees
no torn reads for anything outside CAO.
def _ensure_startup_settings() -> None:
"""Seed ``~/.claude/settings.json`` with the settings that prevent CLI startup
prompts CAO never wants to see, so PREVENTION is what suppresses them rather than
runtime detect-and-dismiss.

Two keys are seeded in a single atomic read-modify-write:

- ``skipDangerousModePermissionPrompt: true``: Claude Code (v2.1.41+) shows a bypass
permissions confirmation dialog on every launch with ``--dangerously-skip-permissions``
unless this is persisted. CAO already uses the flag intentionally, so the confirmation
is redundant and blocks initialization.
- ``tui: "default"`` (workain/harness-control#225): Claude Code shows a first-run
"Try the new fullscreen renderer?" onboarding upsell on a HOME dir whose stored
onboarding-version state lags the installed CLI, unless the CLI's own ``/tui`` setting
is already explicitly set. ``"default"`` keeps the classic renderer that this file's
own screen-scraping status detection (``get_status``/``wait_until_status`` parse raw
pane content) already expects -- the real fullscreen mode uses the terminal's alternate
screen, untested against that scraping, so it is not something to switch on as a side
effect of dialog suppression. Prevention beats reacting to a prompt shape that only
exists at all because this setting was left unset (this replaces an earlier runtime
detect-and-dismiss approach).

After the async conversion, N concurrent inits may run this read-modify-write in N
threads (via ``asyncio.to_thread``). ``_SETTINGS_WRITE_LOCK`` serializes our own threads
(in-process only: a second cao-server process, or Claude Code itself, writing between our
read and ``os.replace`` is still a last-writer-wins lost update); ``os.replace`` only
guarantees no torn reads for anything outside CAO.
"""
settings_path = Path.home() / ".claude" / "settings.json"
with _SETTINGS_WRITE_LOCK:
Expand All @@ -487,10 +499,19 @@ def _ensure_skip_bypass_prompt_setting() -> None:
except (json.JSONDecodeError, OSError):
pass

if settings.get("skipDangerousModePermissionPrompt") is True:
# Seed both keys in one write. Only overwrite ``tui`` when it is ABSENT --
# an operator who deliberately chose ``"fullscreen"`` should not be reset on
# every launch (unlike skipDangerousModePermissionPrompt, which CAO always owns).
changed = False
if settings.get("skipDangerousModePermissionPrompt") is not True:
settings["skipDangerousModePermissionPrompt"] = True
changed = True
if "tui" not in settings:
settings["tui"] = "default"
changed = True
if not changed:
return

settings["skipDangerousModePermissionPrompt"] = True
settings_path.parent.mkdir(parents=True, exist_ok=True)
# PID-suffixed so a stale tmp file from a prior crashed process
# can never collide with -- or be clobbered by -- this write.
Expand All @@ -501,7 +522,7 @@ def _ensure_skip_bypass_prompt_setting() -> None:
# `apiKeyHelper` secrets) -- the tmp file would otherwise pick
# up the process umask (typically 0644) and os.replace would
# make the target adopt that on every launch that toggles
# this flag.
# these settings.
with open(tmp_path, "w") as f:
json.dump(settings, f, indent=2)
os.chmod(tmp_path, existing_mode if existing_mode is not None else 0o600)
Expand All @@ -512,7 +533,7 @@ def _ensure_skip_bypass_prompt_setting() -> None:
# the tmp file indefinitely.
tmp_path.unlink(missing_ok=True)
raise
logger.info("Set skipDangerousModePermissionPrompt in ~/.claude/settings.json")
logger.info("Seeded startup-prompt-suppressing settings in ~/.claude/settings.json")

async def _handle_startup_prompts(
self, idle_gap: Optional[float] = None, outer_timeout: Optional[float] = None
Expand All @@ -523,7 +544,7 @@ async def _handle_startup_prompts(

1. **Bypass permissions confirmation** (``--dangerously-skip-permissions``)
– shows "Yes, I accept" as option 2; requires ``Down`` + ``Enter``.
The settings-based fix (``_ensure_skip_bypass_prompt_setting``) prevents
The settings-based fix (``_ensure_startup_settings``) prevents
this in most cases; this handler is a defensive fallback.
2. **Workspace trust dialog** – shows "Yes, I trust this folder";
requires ``Enter``.
Expand Down Expand Up @@ -663,7 +684,7 @@ async def initialize(self) -> bool:
# Not exhaustive: wait_for_shell's own backend polling, _load_profile(),
# and _build_claude_command's temp-file I/O above/below are still
# loop-side -- tens of ms each, not the multi-second pileup #451 fixes.
await asyncio.to_thread(self._ensure_skip_bypass_prompt_setting)
await asyncio.to_thread(self._ensure_startup_settings)

# Build properly escaped command string
command = self._build_claude_command(profile)
Expand Down
76 changes: 63 additions & 13 deletions test/providers/test_claude_code_unit.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,9 @@ def cleanup_tmp_files():
f.unlink(missing_ok=True)


# All initialization tests need to patch _ensure_skip_bypass_prompt_setting
# All initialization tests need to patch _ensure_startup_settings
# to avoid writing to the real ~/.claude/settings.json.
_PATCH_SETTINGS = patch.object(ClaudeCodeProvider, "_ensure_skip_bypass_prompt_setting")
_PATCH_SETTINGS = patch.object(ClaudeCodeProvider, "_ensure_startup_settings")


def _extract_mcp_config(command: str) -> dict:
Expand Down Expand Up @@ -1955,8 +1955,8 @@ class TestClaudeCodeProviderSettings:
"""Tests for Claude Code settings management."""

@patch("cli_agent_orchestrator.providers.claude_code.Path")
def test_ensure_skip_bypass_prompt_already_set(self, mock_path_cls):
"""Test no-op when setting is already present."""
def test_ensure_startup_settings_noop_when_both_keys_present(self, mock_path_cls):
"""Test no-op when BOTH seeded keys are already present."""
mock_settings_path = MagicMock()
mock_settings_path.exists.return_value = True
mock_path_cls.home.return_value.__truediv__ = MagicMock(
Expand All @@ -1969,13 +1969,63 @@ def test_ensure_skip_bypass_prompt_already_set(self, mock_path_cls):
mock_home.__truediv__ = MagicMock(return_value=mock_claude_dir)
mock_claude_dir.__truediv__ = MagicMock(return_value=mock_settings_path)

existing = json.dumps({"skipDangerousModePermissionPrompt": True})
# Both keys already set (and tui at some explicit value) -> genuine no-op.
existing = json.dumps(
{"skipDangerousModePermissionPrompt": True, "tui": "default"}
)
with patch("builtins.open", mock_open(read_data=existing)):
ClaudeCodeProvider._ensure_skip_bypass_prompt_setting()
ClaudeCodeProvider._ensure_startup_settings()

# Should not write (file handle's write not called)
# Should not write (no mkdir, no tmp-file replace)
mock_settings_path.parent.mkdir.assert_not_called()

def test_ensure_startup_settings_seeds_tui_default_with_bypass(self, tmp_path):
"""harness-control#225: both keys are seeded together in ONE atomic write --
tui:"default" (upsell prevention) alongside skipDangerousModePermissionPrompt."""
settings_file = tmp_path / ".claude" / "settings.json"

with (
patch("cli_agent_orchestrator.providers.claude_code.Path") as mock_path_cls,
patch(
"cli_agent_orchestrator.providers.claude_code.os.replace", wraps=os.replace
) as mock_replace,
):
mock_home = MagicMock()
mock_path_cls.home.return_value = mock_home
mock_home.__truediv__ = MagicMock(
return_value=MagicMock(__truediv__=MagicMock(return_value=settings_file))
)

ClaudeCodeProvider._ensure_startup_settings()

result = json.loads(settings_file.read_text())
assert result["skipDangerousModePermissionPrompt"] is True
assert result["tui"] == "default"
# Both keys landed in a SINGLE atomic os.replace (not two separate writes).
mock_replace.assert_called_once()
# A freshly-created settings.json may carry secrets -> 0600.
assert stat.S_IMODE(settings_file.stat().st_mode) == 0o600

def test_ensure_startup_settings_preserves_explicit_tui(self, tmp_path):
"""An operator who deliberately chose tui:"fullscreen" must NOT be reset on
every launch -- tui is only seeded when ABSENT (unlike the bypass flag CAO owns)."""
settings_file = tmp_path / ".claude" / "settings.json"
settings_file.parent.mkdir(parents=True)
settings_file.write_text(json.dumps({"tui": "fullscreen"}))

with patch("cli_agent_orchestrator.providers.claude_code.Path") as mock_path_cls:
mock_home = MagicMock()
mock_path_cls.home.return_value = mock_home
mock_home.__truediv__ = MagicMock(
return_value=MagicMock(__truediv__=MagicMock(return_value=settings_file))
)

ClaudeCodeProvider._ensure_startup_settings()

result = json.loads(settings_file.read_text())
assert result["tui"] == "fullscreen" # preserved, not clobbered
assert result["skipDangerousModePermissionPrompt"] is True # still seeded

def test_ensure_skip_bypass_prompt_writes_setting(self, tmp_path):
"""Test that setting is written when missing."""
settings_file = tmp_path / ".claude" / "settings.json"
Expand All @@ -1989,7 +2039,7 @@ def test_ensure_skip_bypass_prompt_writes_setting(self, tmp_path):
return_value=MagicMock(__truediv__=MagicMock(return_value=settings_file))
)

ClaudeCodeProvider._ensure_skip_bypass_prompt_setting()
ClaudeCodeProvider._ensure_startup_settings()

result = json.loads(settings_file.read_text())
assert result["skipDangerousModePermissionPrompt"] is True
Expand All @@ -2007,7 +2057,7 @@ def test_ensure_skip_bypass_prompt_creates_file(self, tmp_path):
return_value=MagicMock(__truediv__=MagicMock(return_value=settings_file))
)

ClaudeCodeProvider._ensure_skip_bypass_prompt_setting()
ClaudeCodeProvider._ensure_startup_settings()

result = json.loads(settings_file.read_text())
assert result["skipDangerousModePermissionPrompt"] is True
Expand All @@ -2028,7 +2078,7 @@ def test_ensure_skip_bypass_prompt_preserves_file_mode(self, tmp_path):
return_value=MagicMock(__truediv__=MagicMock(return_value=settings_file))
)

ClaudeCodeProvider._ensure_skip_bypass_prompt_setting()
ClaudeCodeProvider._ensure_startup_settings()

assert stat.S_IMODE(settings_file.stat().st_mode) == 0o600

Expand All @@ -2045,7 +2095,7 @@ def test_ensure_skip_bypass_prompt_new_file_defaults_to_0600(self, tmp_path):
return_value=MagicMock(__truediv__=MagicMock(return_value=settings_file))
)

ClaudeCodeProvider._ensure_skip_bypass_prompt_setting()
ClaudeCodeProvider._ensure_startup_settings()

assert stat.S_IMODE(settings_file.stat().st_mode) == 0o600

Expand All @@ -2067,7 +2117,7 @@ def test_ensure_skip_bypass_prompt_concurrent_writes_preserve_keys(self, tmp_pat
)

threads = [
threading.Thread(target=ClaudeCodeProvider._ensure_skip_bypass_prompt_setting)
threading.Thread(target=ClaudeCodeProvider._ensure_startup_settings)
for _ in range(32)
]
for t in threads:
Expand Down Expand Up @@ -2100,7 +2150,7 @@ def test_ensure_skip_bypass_prompt_uses_atomic_replace(self, tmp_path):
return_value=MagicMock(__truediv__=MagicMock(return_value=settings_file))
)

ClaudeCodeProvider._ensure_skip_bypass_prompt_setting()
ClaudeCodeProvider._ensure_startup_settings()

mock_replace.assert_called_once()
tmp_arg = mock_replace.call_args[0][0]
Expand Down
2 changes: 1 addition & 1 deletion test/providers/test_container_wrapped.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ async def test_idle_timeout_prompt_handler(mock_backend, mock_time, mock_sleep):


@pytest.mark.asyncio
@patch.object(ClaudeCodeProvider, "_ensure_skip_bypass_prompt_setting")
@patch.object(ClaudeCodeProvider, "_ensure_startup_settings")
@patch.object(ClaudeCodeProvider, "_build_claude_command", return_value="claude")
@patch("cli_agent_orchestrator.providers.claude_code.load_agent_profile")
@patch("cli_agent_orchestrator.providers.claude_code.wait_for_shell")
Expand Down
10 changes: 5 additions & 5 deletions test/providers/test_provider_init_timeout.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ class TestInitializePassesResolvedInitTimeout:
load_agent_profile (profile source), wait_for_shell / wait_until_status /
wait_until_input_ready (the async waits), _build_claude_command (avoids
temp-file I/O), _handle_startup_prompts (asserted separately),
_ensure_skip_bypass_prompt_setting (avoids writing
_ensure_startup_settings (avoids writing
~/.claude/settings.json), and the terminal backend.
"""

Expand All @@ -58,7 +58,7 @@ def _mock_input_ready(self):
yield

@pytest.mark.asyncio
@patch.object(ClaudeCodeProvider, "_ensure_skip_bypass_prompt_setting")
@patch.object(ClaudeCodeProvider, "_ensure_startup_settings")
@patch.object(ClaudeCodeProvider, "_build_claude_command", return_value="claude")
@patch.object(ClaudeCodeProvider, "_handle_startup_prompts")
@patch(f"{_CC}.load_agent_profile")
Expand Down Expand Up @@ -91,7 +91,7 @@ async def test_profile_override_flows_to_every_wait(

@pytest.mark.asyncio
@patch(_SETTINGS, return_value={"provider_init_timeout": 60})
@patch.object(ClaudeCodeProvider, "_ensure_skip_bypass_prompt_setting")
@patch.object(ClaudeCodeProvider, "_ensure_startup_settings")
@patch.object(ClaudeCodeProvider, "_build_claude_command", return_value="claude")
@patch.object(ClaudeCodeProvider, "_handle_startup_prompts")
@patch(f"{_CC}.load_agent_profile")
Expand Down Expand Up @@ -125,7 +125,7 @@ async def test_profile_without_override_uses_server_default(

@pytest.mark.asyncio
@patch(_SETTINGS, return_value={"provider_init_timeout": 60})
@patch.object(ClaudeCodeProvider, "_ensure_skip_bypass_prompt_setting")
@patch.object(ClaudeCodeProvider, "_ensure_startup_settings")
@patch.object(ClaudeCodeProvider, "_build_claude_command", return_value="claude")
@patch.object(ClaudeCodeProvider, "_handle_startup_prompts")
@patch(f"{_CC}.wait_for_shell")
Expand Down Expand Up @@ -155,7 +155,7 @@ async def test_no_profile_uses_server_default(
assert mock_handle.call_args.kwargs["outer_timeout"] == 60

@pytest.mark.asyncio
@patch.object(ClaudeCodeProvider, "_ensure_skip_bypass_prompt_setting")
@patch.object(ClaudeCodeProvider, "_ensure_startup_settings")
@patch.object(ClaudeCodeProvider, "_build_claude_command", return_value="claude")
@patch.object(ClaudeCodeProvider, "_handle_startup_prompts")
@patch(f"{_CC}.load_agent_profile")
Expand Down
Loading
Loading