Skip to content

Commit 5000d02

Browse files
examonCopilot
andcommitted
Close the two remaining reviewer findings on isTerminal
The automated reviewer raised two suppressed findings against the new Tool.isTerminal field. Both were accurate. Rust: Tool has a hand-written Debug impl that enumerates every other serializable field, so is_terminal was silently missing from its output and a terminal tool debug-printed identically to a plain one. Add the field in declaration order, plus a test that fails if the hand-written impl drifts again. Python: dotnet, go, java, nodejs and rust all assert that isTerminal reaches the wire on both the session.create and session.resume paths and is omitted at its default. Python was the only SDK without that coverage. Add the test, mirroring the adjacent tool-metadata test. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 012f981 commit 5000d02

2 files changed

Lines changed: 58 additions & 0 deletions

File tree

python/test_client.py

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -793,6 +793,45 @@ async def mock_request(method, params, **kwargs):
793793
finally:
794794
await client.force_stop()
795795

796+
@pytest.mark.asyncio
797+
async def test_create_and_resume_session_forward_tool_is_terminal(self):
798+
client = CopilotClient(connection=RuntimeConnection.for_stdio(path=CLI_PATH))
799+
await client.start()
800+
try:
801+
captured = {}
802+
803+
async def mock_request(method, params, **kwargs):
804+
captured[method] = params
805+
if method in ("session.create", "session.resume"):
806+
result = {"sessionId": params.get("sessionId") or "session-1"}
807+
callback = kwargs.get("on_response_inline")
808+
if callback is not None:
809+
callback(result)
810+
return result
811+
return {}
812+
813+
client._client.request = mock_request
814+
tool = Tool(name="my_tool", description="a tool", is_terminal=True)
815+
plain_tool = Tool(name="plain_tool", description="a tool")
816+
817+
session = await client.create_session(
818+
on_permission_request=PermissionHandler.approve_all,
819+
tools=[tool, plain_tool],
820+
)
821+
await client.resume_session(
822+
session.session_id,
823+
on_permission_request=PermissionHandler.approve_all,
824+
tools=[tool],
825+
)
826+
827+
create_tools = captured["session.create"]["tools"]
828+
assert create_tools[0]["isTerminal"] is True
829+
# Omitted when left at its default.
830+
assert "isTerminal" not in create_tools[1]
831+
assert captured["session.resume"]["tools"][0]["isTerminal"] is True
832+
finally:
833+
await client.force_stop()
834+
796835
@pytest.mark.asyncio
797836
async def test_create_and_resume_session_forward_canvas_provider(self):
798837
client = CopilotClient(connection=RuntimeConnection.for_stdio(path=CLI_PATH))

rust/src/types.rs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -530,6 +530,7 @@ impl std::fmt::Debug for Tool {
530530
.field("parameters", &self.parameters)
531531
.field("overrides_built_in_tool", &self.overrides_built_in_tool)
532532
.field("skip_permission", &self.skip_permission)
533+
.field("is_terminal", &self.is_terminal)
533534
.field("defer", &self.defer)
534535
.field("metadata", &self.metadata)
535536
.field(
@@ -7562,4 +7563,22 @@ mod is_terminal_tests {
75627563
let value = serde_json::to_value(&tool).expect("tool serializes");
75637564
assert!(value.get("isTerminal").is_none());
75647565
}
7566+
7567+
/// `Tool` has a hand-written `Debug` impl, so a new field is only reported
7568+
/// if it is added there by hand. Guard against that drift.
7569+
#[test]
7570+
fn is_terminal_appears_in_debug_output() {
7571+
let terminal = Tool {
7572+
name: "clear_context".to_owned(),
7573+
is_terminal: true,
7574+
..Default::default()
7575+
};
7576+
assert!(format!("{terminal:?}").contains("is_terminal: true"));
7577+
7578+
let plain = Tool {
7579+
name: "plain".to_owned(),
7580+
..Default::default()
7581+
};
7582+
assert!(format!("{plain:?}").contains("is_terminal: false"));
7583+
}
75657584
}

0 commit comments

Comments
 (0)