Skip to content

Commit 2d725a5

Browse files
tclemCopilot
andcommitted
Add ClientOptions::new() and Tool::new() builder methods
Mirrors the existing MessageOptions::new() / with_* builder pattern on the two other public #[non_exhaustive] config types that consumers construct most often. Closes the cross-crate ergonomics gap that #[non_exhaustive] introduces: external callers can't use struct- literal syntax (with or without ..Default::default()) on these types, so every field assignment becomes its own mut-let statement. The builder shape collapses that into a single chained expression. Caught by github-app PR #4140 review on src-tauri/src/session/cli.rs: the mut-let-and-assign pattern read as awkward to reviewers even after trimming to the four fields that diverge from default. Same ergonomic friction that prompted the make_tool() helper consumers were writing for Tool. Builder ships zero-cost convenience without giving up #[non_exhaustive]'s SemVer protection on growable config struct. ClientOptions builder (rust/src/lib.rs): - new() -> Self (documented entry point) - with_program(impl Into<CliProgram>) - with_prefix_args<I: IntoIterator<...>>(args) - with_cwd(impl Into<PathBuf>) - with_env<I: IntoIterator<(K, V)>> - with_env_remove<I: IntoIterator<S>> - with_extra_args<I: IntoIterator<S>> - with_transport(Transport) - with_github_token(impl Into<String>) - with_use_logged_in_user(bool) - with_log_level(LogLevel) - with_session_idle_timeout_seconds(u64) - with_list_models_handler<H: ListModelsHandler + 'static> (Arc-wrapped internally) - with_session_fs(SessionFsConfig) - with_trace_context_provider<P: TraceContextProvider + 'static> (Arc-wrapped internally) - with_telemetry(TelemetryConfig) Tool builder (rust/src/types.rs): - new(impl Into<String>) -> Self (name + defaults) - with_namespaced_name(impl Into<String>) - with_description(impl Into<String>) - with_instructions(impl Into<String>) - with_parameters(serde_json::Value) - with_overrides_built_in_tool(bool) - with_skip_permission(bool) Pure additive: no existing ClientOptions::default() / mut-let patterns break, no public field visibility changes. Both builders compose by wrapping ::default() and mutating-and-returning self, matching MessageOptions's idiom. Tests: client_options_builder_composes verifies all 10 chained builder methods produce the expected internal state. tool_builder_composes does the same for Tool's seven methods, plus tool_with_parameters_handles_non_object_value covers the HashMap-from-Value extraction edge case (json!(null) -> empty map rather than panic). CHANGELOG: new "Builder ergonomics" subsection under Documentation, explaining the rationale and listing both builders. Validation: 220 tests pass (was 215 + 5 new builder tests across unit + doctest), cargo doc -D warnings clean, cargo fmt --check clean, cargo clippy --all-features --all-targets -- -D warnings clean. Companion ask from github-app PR #4140 review (Sync session) follows up: github-app's `cli.rs` and ~25 Tool::new() call sites will switch to the builder form once this lands and the next sync round-trip pulls it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent e04357c commit 2d725a5

3 files changed

Lines changed: 300 additions & 1 deletion

File tree

rust/CHANGELOG.md

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -349,6 +349,28 @@ public surface.
349349
`lifecycle_observer`.
350350
- `RELEASING.md` operational runbook for maintainers.
351351

352+
#### Builder ergonomics
353+
- `ClientOptions::new()` plus a chainable `with_*` builder per public
354+
field (`with_program`, `with_prefix_args`, `with_cwd`, `with_env`,
355+
`with_env_remove`, `with_extra_args`, `with_transport`,
356+
`with_github_token`, `with_use_logged_in_user`, `with_log_level`,
357+
`with_session_idle_timeout_seconds`, `with_list_models_handler`,
358+
`with_session_fs`, `with_trace_context_provider`, `with_telemetry`).
359+
Mirrors the existing [`MessageOptions::new`] / `with_*` shape and
360+
closes the cross-crate ergonomics gap on `#[non_exhaustive]`
361+
external callers no longer need to write
362+
`let mut opts = ClientOptions::default(); opts.field = ...;` for
363+
every field they touch. Existing `ClientOptions::default()` and
364+
mut-let-and-assign continue to work unchanged.
365+
- `Tool::new(name)` plus `with_namespaced_name`, `with_description`,
366+
`with_instructions`, `with_parameters`, `with_overrides_built_in_tool`,
367+
`with_skip_permission` for tool definitions. Same rationale —
368+
`Tool` is the most-instantiated `#[non_exhaustive]` type at consumer
369+
call sites (~25 sites in github-app's tool catalog), where the
370+
builder shape replaces the per-consumer `make_tool(name, desc,
371+
params)` helper that consumers were writing to smooth over the
372+
mut-let pattern.
373+
352374
### Fixed
353375
- `SessionUi::elicitation` (and the `confirm` / `select` / `input`
354376
convenience helpers that delegate through it) now sends the user-supplied

rust/src/lib.rs

Lines changed: 171 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -571,6 +571,146 @@ impl Default for ClientOptions {
571571
}
572572
}
573573

574+
impl ClientOptions {
575+
/// Construct a new [`ClientOptions`] with default values.
576+
///
577+
/// Equivalent to [`ClientOptions::default`]; provided as a documented
578+
/// construction entry point for the builder chain. The struct is
579+
/// `#[non_exhaustive]`, so external callers cannot use struct-literal
580+
/// syntax — use this builder or [`Default::default`] plus mut-let.
581+
///
582+
/// # Example
583+
///
584+
/// ```
585+
/// # use github_copilot_sdk::{ClientOptions, LogLevel};
586+
/// let opts = ClientOptions::new()
587+
/// .with_log_level(LogLevel::Debug)
588+
/// .with_github_token("ghp_…");
589+
/// ```
590+
pub fn new() -> Self {
591+
Self::default()
592+
}
593+
594+
/// How to locate the CLI binary. See [`CliProgram`].
595+
pub fn with_program(mut self, program: impl Into<CliProgram>) -> Self {
596+
self.program = program.into();
597+
self
598+
}
599+
600+
/// Arguments prepended before `--server` (e.g. the script path for node).
601+
pub fn with_prefix_args<I, S>(mut self, args: I) -> Self
602+
where
603+
I: IntoIterator<Item = S>,
604+
S: Into<OsString>,
605+
{
606+
self.prefix_args = args.into_iter().map(Into::into).collect();
607+
self
608+
}
609+
610+
/// Working directory for the CLI process.
611+
pub fn with_cwd(mut self, cwd: impl Into<PathBuf>) -> Self {
612+
self.cwd = cwd.into();
613+
self
614+
}
615+
616+
/// Environment variables to set on the child process.
617+
pub fn with_env<I, K, V>(mut self, env: I) -> Self
618+
where
619+
I: IntoIterator<Item = (K, V)>,
620+
K: Into<OsString>,
621+
V: Into<OsString>,
622+
{
623+
self.env = env.into_iter().map(|(k, v)| (k.into(), v.into())).collect();
624+
self
625+
}
626+
627+
/// Environment variable names to remove from the child process.
628+
pub fn with_env_remove<I, S>(mut self, names: I) -> Self
629+
where
630+
I: IntoIterator<Item = S>,
631+
S: Into<OsString>,
632+
{
633+
self.env_remove = names.into_iter().map(Into::into).collect();
634+
self
635+
}
636+
637+
/// Extra CLI flags appended after the transport-specific arguments.
638+
pub fn with_extra_args<I, S>(mut self, args: I) -> Self
639+
where
640+
I: IntoIterator<Item = S>,
641+
S: Into<String>,
642+
{
643+
self.extra_args = args.into_iter().map(Into::into).collect();
644+
self
645+
}
646+
647+
/// Transport mode used to communicate with the CLI server. See [`Transport`].
648+
pub fn with_transport(mut self, transport: Transport) -> Self {
649+
self.transport = transport;
650+
self
651+
}
652+
653+
/// GitHub token for authentication. The SDK passes the token to the
654+
/// CLI via `--auth-token-env COPILOT_SDK_AUTH_TOKEN`.
655+
pub fn with_github_token(mut self, token: impl Into<String>) -> Self {
656+
self.github_token = Some(token.into());
657+
self
658+
}
659+
660+
/// Whether the CLI should fall back to the logged-in `gh` user when
661+
/// no token is provided. See the field docs for default semantics.
662+
pub fn with_use_logged_in_user(mut self, use_logged_in: bool) -> Self {
663+
self.use_logged_in_user = Some(use_logged_in);
664+
self
665+
}
666+
667+
/// Log level passed to the CLI server via `--log-level`.
668+
pub fn with_log_level(mut self, level: LogLevel) -> Self {
669+
self.log_level = Some(level);
670+
self
671+
}
672+
673+
/// Server-wide idle timeout for sessions (seconds). Pass `0` to leave
674+
/// sessions running indefinitely (the CLI default).
675+
pub fn with_session_idle_timeout_seconds(mut self, seconds: u64) -> Self {
676+
self.session_idle_timeout_seconds = Some(seconds);
677+
self
678+
}
679+
680+
/// Override [`Client::list_models`] with a caller-supplied handler.
681+
/// The handler is wrapped in `Arc` internally.
682+
pub fn with_list_models_handler<H>(mut self, handler: H) -> Self
683+
where
684+
H: ListModelsHandler + 'static,
685+
{
686+
self.on_list_models = Some(Arc::new(handler));
687+
self
688+
}
689+
690+
/// Custom session filesystem provider configuration.
691+
pub fn with_session_fs(mut self, config: SessionFsConfig) -> Self {
692+
self.session_fs = Some(config);
693+
self
694+
}
695+
696+
/// Set the [`TraceContextProvider`] used to inject W3C Trace Context
697+
/// headers on outbound `session.create` / `session.resume` /
698+
/// `session.send` requests. The provider is wrapped in `Arc` internally.
699+
pub fn with_trace_context_provider<P>(mut self, provider: P) -> Self
700+
where
701+
P: TraceContextProvider + 'static,
702+
{
703+
self.on_get_trace_context = Some(Arc::new(provider));
704+
self
705+
}
706+
707+
/// OpenTelemetry config forwarded to the spawned CLI process.
708+
pub fn with_telemetry(mut self, config: TelemetryConfig) -> Self {
709+
self.telemetry = Some(config);
710+
self
711+
}
712+
}
713+
574714
/// Validate a [`SessionFsConfig`] before sending `sessionFs.setProvider`.
575715
fn validate_session_fs_config(cfg: &SessionFsConfig) -> Result<(), Error> {
576716
if cfg.initial_cwd.trim().is_empty() {
@@ -1619,6 +1759,37 @@ mod tests {
16191759
assert!(!err.is_transport_failure());
16201760
}
16211761

1762+
#[test]
1763+
fn client_options_builder_composes() {
1764+
let opts = ClientOptions::new()
1765+
.with_program(CliProgram::Path(PathBuf::from("/usr/local/bin/copilot")))
1766+
.with_prefix_args(["node"])
1767+
.with_cwd(PathBuf::from("/tmp"))
1768+
.with_env([("KEY", "value")])
1769+
.with_env_remove(["UNWANTED"])
1770+
.with_extra_args(["--quiet"])
1771+
.with_github_token("ghp_test")
1772+
.with_use_logged_in_user(false)
1773+
.with_log_level(LogLevel::Debug)
1774+
.with_session_idle_timeout_seconds(120);
1775+
assert!(matches!(opts.program, CliProgram::Path(_)));
1776+
assert_eq!(opts.prefix_args, vec![std::ffi::OsString::from("node")]);
1777+
assert_eq!(opts.cwd, PathBuf::from("/tmp"));
1778+
assert_eq!(
1779+
opts.env,
1780+
vec![(
1781+
std::ffi::OsString::from("KEY"),
1782+
std::ffi::OsString::from("value")
1783+
)]
1784+
);
1785+
assert_eq!(opts.env_remove, vec![std::ffi::OsString::from("UNWANTED")]);
1786+
assert_eq!(opts.extra_args, vec!["--quiet".to_string()]);
1787+
assert_eq!(opts.github_token.as_deref(), Some("ghp_test"));
1788+
assert_eq!(opts.use_logged_in_user, Some(false));
1789+
assert!(matches!(opts.log_level, Some(LogLevel::Debug)));
1790+
assert_eq!(opts.session_idle_timeout_seconds, Some(120));
1791+
}
1792+
16221793
#[test]
16231794
fn is_transport_failure_rejects_other_protocol_errors() {
16241795
let err = Error::Protocol(ProtocolError::CliStartupTimeout);

rust/src/types.rs

Lines changed: 107 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -333,6 +333,84 @@ fn is_false(b: &bool) -> bool {
333333
!*b
334334
}
335335

336+
impl Tool {
337+
/// Construct a new [`Tool`] with the given name and otherwise default
338+
/// values. The struct is `#[non_exhaustive]`, so external callers
339+
/// cannot use struct-literal syntax — use this builder or
340+
/// [`Default::default`] plus mut-let.
341+
///
342+
/// # Example
343+
///
344+
/// ```
345+
/// # use github_copilot_sdk::types::Tool;
346+
/// # use serde_json::json;
347+
/// let tool = Tool::new("greet")
348+
/// .with_description("Say hello to a user")
349+
/// .with_parameters(json!({
350+
/// "type": "object",
351+
/// "properties": { "name": { "type": "string" } },
352+
/// "required": ["name"]
353+
/// }));
354+
/// # let _ = tool;
355+
/// ```
356+
pub fn new(name: impl Into<String>) -> Self {
357+
Self {
358+
name: name.into(),
359+
..Default::default()
360+
}
361+
}
362+
363+
/// Set the namespaced name for declarative filtering (e.g.
364+
/// `"playwright/navigate"` for MCP tools).
365+
pub fn with_namespaced_name(mut self, namespaced_name: impl Into<String>) -> Self {
366+
self.namespaced_name = Some(namespaced_name.into());
367+
self
368+
}
369+
370+
/// Set the human-readable description of what the tool does.
371+
pub fn with_description(mut self, description: impl Into<String>) -> Self {
372+
self.description = description.into();
373+
self
374+
}
375+
376+
/// Set optional instructions for how to use this tool effectively.
377+
pub fn with_instructions(mut self, instructions: impl Into<String>) -> Self {
378+
self.instructions = Some(instructions.into());
379+
self
380+
}
381+
382+
/// Set the JSON Schema for the tool's input parameters.
383+
///
384+
/// Accepts anything that converts into a JSON object, including a
385+
/// `serde_json::Value` produced by `json!({...})`. Non-object values
386+
/// are stored as an empty parameter map; callers that need direct
387+
/// control over the field can construct a `HashMap<String, Value>`
388+
/// and assign it to [`Tool::parameters`] via [`Default::default`].
389+
pub fn with_parameters(mut self, parameters: Value) -> Self {
390+
self.parameters = parameters
391+
.as_object()
392+
.map(|m| m.iter().map(|(k, v)| (k.clone(), v.clone())).collect())
393+
.unwrap_or_default();
394+
self
395+
}
396+
397+
/// Mark this tool as overriding a built-in tool of the same name.
398+
/// E.g. supplying a custom `grep` that the agent uses in place of the
399+
/// CLI's built-in implementation.
400+
pub fn with_overrides_built_in_tool(mut self, overrides: bool) -> Self {
401+
self.overrides_built_in_tool = overrides;
402+
self
403+
}
404+
405+
/// When `true`, the CLI will not request permission before invoking
406+
/// this tool. Use with caution — the tool is responsible for any
407+
/// access control.
408+
pub fn with_skip_permission(mut self, skip: bool) -> Self {
409+
self.skip_permission = skip;
410+
self
411+
}
412+
}
413+
336414
/// Context passed to a [`CommandHandler`] when a registered slash command
337415
/// is executed by the user.
338416
#[non_exhaustive]
@@ -2323,9 +2401,37 @@ mod tests {
23232401
use super::{
23242402
Attachment, AttachmentLineRange, AttachmentSelectionPosition, AttachmentSelectionRange,
23252403
ConnectionState, DeliveryMode, GitHubReferenceType, ResumeSessionConfig, SessionConfig,
2326-
SessionId, ensure_attachment_display_names,
2404+
SessionId, Tool, ensure_attachment_display_names,
23272405
};
23282406

2407+
#[test]
2408+
fn tool_builder_composes() {
2409+
let tool = Tool::new("greet")
2410+
.with_description("Say hello")
2411+
.with_namespaced_name("hello/greet")
2412+
.with_instructions("Pass the user's name")
2413+
.with_parameters(json!({
2414+
"type": "object",
2415+
"properties": { "name": { "type": "string" } },
2416+
"required": ["name"]
2417+
}))
2418+
.with_overrides_built_in_tool(true)
2419+
.with_skip_permission(true);
2420+
assert_eq!(tool.name, "greet");
2421+
assert_eq!(tool.description, "Say hello");
2422+
assert_eq!(tool.namespaced_name.as_deref(), Some("hello/greet"));
2423+
assert_eq!(tool.instructions.as_deref(), Some("Pass the user's name"));
2424+
assert_eq!(tool.parameters.get("type").unwrap(), &json!("object"));
2425+
assert!(tool.overrides_built_in_tool);
2426+
assert!(tool.skip_permission);
2427+
}
2428+
2429+
#[test]
2430+
fn tool_with_parameters_handles_non_object_value() {
2431+
let tool = Tool::new("noop").with_parameters(json!(null));
2432+
assert!(tool.parameters.is_empty());
2433+
}
2434+
23292435
#[test]
23302436
fn session_config_default_enables_permission_flow_flags() {
23312437
let cfg = SessionConfig::default();

0 commit comments

Comments
 (0)