Prevent global command settings mutation from auto-registered commands

[matdev83]

Summary

• add a scoped DefaultCommandStateService so auto-registered commands store overrides locally instead of mutating the shared command settings
• cover the new state service with unit tests that prove command prefix, API key redaction, and interactive command flags remain session-scoped

Testing

• python -m pytest --override-ini addopts='' tests/unit/core/app/test_default_command_state_service.py
• python -m pytest --override-ini addopts=''

---

https://chatgpt.com/codex/tasks/task_e_68ec25089a4c8333a7c641442c526119

Summary by CodeRabbit

• New Features
  • Centralized command settings with session-local overrides for command prefix, failover routes, API key redaction, and interactive command enablement.
  • Allows adjusting command behavior at runtime without altering global defaults, improving flexibility and predictability during command auto-registration.
• Tests
  • Added unit tests to confirm that overrides remain local to the session and do not mutate default settings.

[coderabbitai]

Walkthrough

Introduces DefaultCommandStateService to manage command auto-registration state with override capability, replacing a local inline state holder. Adds getters and setters for prefix, failover routes, API key redaction, and interactive commands. Wires the service into command population. Adds unit tests validating session-local overrides without mutating underlying settings.

Changes

Cohort / File(s)	Summary
Core command state service src/core/app/stages/command.py	Adds public DefaultCommandStateService implementing secure state access/modification, proxying to ICommandSettingsService by default and supporting runtime overrides for command prefix, failover routes, API key redaction, and interactive command toggles; integrates into auto-discovery population flow.
Unit tests tests/unit/core/app/test_default_command_state_service.py	Adds tests ensuring overrides in DefaultCommandStateService are session-local and do not alter underlying CommandSettingsService defaults (prefix, API key redaction, interactive commands).

Sequence Diagram(s)

sequenceDiagram autonumber actor Caller participant State as DefaultCommandStateService participant Settings as ICommandSettingsService Caller->>State: get_command_prefix() alt Override set State-->>Caller: return override value else No override State->>Settings: get_command_prefix() Settings-->>State: default prefix State-->>Caller: return default prefix end Caller->>State: update_command_prefix(prefix) State-->>Caller: store override Caller->>State: get_api_key_redaction_enabled() alt Override set State-->>Caller: return override else No override State->>Settings: get_api_key_redaction_enabled() Settings-->>State: default State-->>Caller: return default end Caller->>State: update_api_key_redaction(enabled) State-->>Caller: store override Caller->>State: get_failover_routes() alt Override set State-->>Caller: routes override else No override State->>Settings: get_failover_routes() Settings-->>State: default routes State-->>Caller: return default routes end Caller->>State: update_failover_routes(routes) State-->>Caller: store override Caller->>State: get_disable_interactive_commands() alt Override set State-->>Caller: return override else No override State->>Settings: get_disable_interactive_commands() Settings-->>State: default State-->>Caller: return default end Caller->>State: update_interactive_commands(enabled) State-->>Caller: store override 

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

I twitch my whiskers, state in paw,
New service hops in, crisp and raw.
Defaults in burrows, overrides on breeze,
Prefix and routes, toggled with ease.
Tests nibble proofs, carrots aligned—
Session-local secrets, neatly confined. 🥕🐇

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 13.33% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The title succinctly captures the core change of preventing auto-registered commands from altering global command settings by introducing a scoped state service. It directly references key aspects of the update—global mutation prevention and auto-registration scope—and aligns with the modifications described in the changeset. The phrasing is clear, specific, and concise, providing enough context for a reviewer to understand the primary intent at a glance. Therefore, it meets the criteria for a focused and descriptive pull request title.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch codex/fix-unexpected-state-changes-in-proxy-ekh2ev

---

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

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4b959ef and 33b3388.

📒 Files selected for processing (2)

• src/core/app/stages/command.py (2 hunks)
• tests/unit/core/app/test_default_command_state_service.py (1 hunks)

🧰 Additional context used

📓 Path-based instructions (2)

**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

**/*.py: Avoid using emojis in Python code comments or docstrings
Follow PEP 8 and use type hints for all functions
Import order must be: standard library, third-party, then local imports, separated by blank lines
Naming conventions: snake_case for variables and functions; PascalCase for classes
Error handling: use specific exceptions and include meaningful error messages
Prefer f-strings for string formatting

Files:

• src/core/app/stages/command.py
• tests/unit/core/app/test_default_command_state_service.py

src/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

src/**/*.py: Catch specific exceptions; avoid broad except Exception blocks
If a broad exception must be caught, log with exc_info=True and re-raise a specific custom exception
Use the most specific exception class from src.core.common.exceptions that accurately describes the error
Provide clear, helpful error messages and include relevant details in the details dictionary of custom exceptions

Files:

• src/core/app/stages/command.py

🧬 Code graph analysis (2)

src/core/app/stages/command.py (5)

src/core/interfaces/command_settings_interface.py (1)

• ICommandSettingsService (13-50)

src/core/interfaces/state_provider_interface.py (10)

• ISecureStateAccess (27-44)
• ISecureStateModification (47-64)
• get_command_prefix (31-32)
• get_failover_routes (43-44)
• update_failover_routes (63-64)
• get_api_key_redaction_enabled (35-36)
• get_disable_interactive_commands (39-40)
• update_command_prefix (51-52)
• update_api_key_redaction (55-56)
• update_interactive_commands (59-60)

src/core/services/application_state_service.py (4)

• get_command_prefix (38-44)
• get_failover_routes (196-209)
• get_api_key_redaction_enabled (52-58)
• get_disable_interactive_commands (74-80)

src/core/services/app_settings_service.py (3)

• get_command_prefix (193-203)
• get_failover_routes (171-183)
• get_disable_interactive_commands (233-243)

src/core/services/secure_state_service.py (8)

• get_command_prefix (36-39)
• get_failover_routes (51-54)
• update_failover_routes (96-121)
• get_api_key_redaction_enabled (41-44)
• get_disable_interactive_commands (46-49)
• update_command_prefix (57-72)
• update_api_key_redaction (74-83)
• update_interactive_commands (85-94)

tests/unit/core/app/test_default_command_state_service.py (2)

src/core/app/stages/command.py (7)

• DefaultCommandStateService (30-69)
• update_command_prefix (61-63)
• get_command_prefix (40-43)
• update_api_key_redaction (65-66)
• get_api_key_redaction_enabled (51-54)
• update_interactive_commands (68-69)
• get_disable_interactive_commands (56-59)

src/core/services/command_settings_service.py (1)

• CommandSettingsService (17-110)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)

• GitHub Check: test (3.10)
• GitHub Check: Analyze (python)

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Preserve command-prefix validation parity

Line 62 now accepts any truthy string, so a value like " " becomes the session prefix and all commands stop matching. The old SecureStateService.update_command_prefix rejected blank/whitespace prefixes; we need the same guard here.

 def update_command_prefix(self, prefix: str) -> None: - if isinstance(prefix, str) and prefix: - self._command_prefix_override = prefix + if not isinstance(prefix, str): + return + + trimmed = prefix.strip() + if not trimmed: + return + + self._command_prefix_override = trimmed

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	def update_command_prefix(self, prefix: str) -> None:
	if isinstance(prefix, str) and prefix:
	self._command_prefix_override = prefix
	def update_command_prefix(self, prefix: str) -> None:
	if not isinstance(prefix, str):
	return
	trimmed = prefix.strip()
	if not trimmed:
	return
	self._command_prefix_override = trimmed

🤖 Prompt for AI Agents

In src/core/app/stages/command.py around lines 61 to 64, the current setter accepts any truthy string so a whitespace-only value like " " becomes the prefix and breaks command matching; change the guard to require a non-empty string after trimming whitespace (e.g., keep the isinstance(prefix, str) check but also require prefix.strip() to be truthy) so blank/whitespace-only prefixes are rejected and the previous validation parity with SecureStateService.update_command_prefix is preserved.