Skip to content

test: cover claude code skip permissions args#1364

Merged
houko merged 5 commits intolibrefang:mainfrom
TechWizard9999:codex/test-claude-code-skip-permissions
Mar 21, 2026
Merged

test: cover claude code skip permissions args#1364
houko merged 5 commits intolibrefang:mainfrom
TechWizard9999:codex/test-claude-code-skip-permissions

Conversation

@TechWizard9999
Copy link
Copy Markdown
Contributor

Summary

  • extract Claude Code CLI argument construction into a small helper
  • add regression tests for --dangerously-skip-permissions in complete mode
  • add regression tests for --dangerously-skip-permissions in streaming mode

Testing

  • CARGO_TARGET_DIR=/tmp/librefang-target cargo test -p librefang-runtime claude_code

Copy link
Copy Markdown
Contributor

@houko houko left a comment

Choose a reason for hiding this comment

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

Nice refactor — extracting build_command_args eliminates the duplicated arg construction between complete and complete_stream. The three test cases cover the key combinations (skip_permissions on/off, verbose on/off). LGTM.

@houko
Copy link
Copy Markdown
Contributor

houko commented Mar 21, 2026

@TechWizard9999 sorry, conflict, please resolve then I will merge .

@TechWizard9999 TechWizard9999 force-pushed the codex/test-claude-code-skip-permissions branch from 1354bc9 to f630413 Compare March 21, 2026 05:25
@TechWizard9999
Copy link
Copy Markdown
Contributor Author

@houko Could you please check it now ?

@houko
Copy link
Copy Markdown
Contributor

houko commented Mar 21, 2026

@TechWizard9999 sorry need format

Copy link
Copy Markdown
Contributor

@houko houko left a comment

Choose a reason for hiding this comment

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

Code review: LGTM

  • Refactoring is logic-equivalent — build_command_args() correctly extracts all CLI arg construction from both complete() and stream()
  • Argument order change (--model before --add-dir) is irrelevant for CLI flag parsing
  • Test coverage for --dangerously-skip-permissions on/off is solid
  • Minor nit: .to_string() on &str args is redundant but harmless

Clean PR, no issues found.

@houko houko merged commit 19c0bb6 into librefang:main Mar 21, 2026
This was referenced Mar 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/runtime Agent loop, LLM drivers, WASM sandbox

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants