[E2E] session.todos_changed event + readSqlTodosWithDependencies (6 languages)#1622
[E2E] session.todos_changed event + readSqlTodosWithDependencies (6 languages)#1622SteveSandersonMS wants to merge 5 commits into
Conversation
…encies Validates the new runtime APIs added in copilot-agent-runtime#10061: - session.todos_changed event fires when the sql tool mutates the prompted todos / todo_deps tables - session.plan.readSqlTodosWithDependencies returns structured rows and dependency edges suitable for progress UIs The generated rpc.ts and session-events.ts diffs were produced by re-running the Node codegen against the runtime PR's generated/ schemas; they will reproduce automatically once the runtime PR ships in the @github/copilot npm package. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
- Re-runs Node codegen against runtime PR's regenerated schemas so the rows/dependencies arrays use the newly-named PlanTodo / PlanTodoDependency types. - Simplifies the E2E prompt: the runtime already prompts the agent to create the todos / todo_deps tables, so the CREATE TABLE steps were noisy in the captured snapshot (SQLite returned 'table todos already exists'). The agent only needs to INSERT. - Re-records the snapshot. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
Mirrors the existing nodejs test in all 4 other SDK languages: agent inserts 2 todos + 1 dependency edge via the sql tool, then we assert the session.todos_changed event fires and readSqlTodosWithDependencies returns the expected rows + dependencies. All 5 language tests share a single recorded CAPI snapshot. Renamed the existing snapshot from 'rows___dependencies' to 'rows_and_dependencies' and the nodejs test title to match, so the slug is identical across all languages without per-language slugification quirks. Also regenerates SDK bindings against the latest runtime schema so the new readSqlTodosWithDependencies RPC, PlanSqlTodosRow, and PlanSqlTodoDependency types are available via typed APIs in every language. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
Adds the generated Java bindings for SessionTodosChangedEvent, PlanSqlTodoDependency, and SessionPlan.readSqlTodosWithDependencies, plus a Java E2E test mirroring the other 5 SDK languages. Status: the test currently fails locally - the runtime's session.todos_changed event is not reaching the Java listener even though the same scenario passes in Node/.NET/Go/Python/Rust against the identical CAPI snapshot. Needs follow-up investigation; pushed as-is so reviewers can see the full 6-language surface. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Auto-committed by java-codegen-check workflow.
Cross-SDK consistency reviewThe six language test files are structurally well-aligned — same prompt, same snapshot key, same two assertions (event observed + typed RPC returns the expected rows and dependency edge). Five of the six tests are consistent.
|
| Node / Python / Go / .NET / Java / other Rust E2E | This Rust test | |
|---|---|---|
| Event check | Typed SDK enum variant (SessionTodosChanged, session.todos_changed type, etc.) |
SessionEventType::Unknown && event.event_type == "session.todos_changed" |
| RPC call | Generated typed helper (session.rpc().plan().read_sql_todos_with_dependencies()) |
Raw session.client().call("session.plan.readSqlTodosWithDependencies", json!({...})) |
| Result types | Generated PlanReadSqlTodosWithDependenciesResult, PlanSqlTodosRow, PlanSqlTodoDependency |
Three local private structs |
The Unknown variant check is also a logic bug: because session.todos_changed is already in the generated SessionEventType enum as SessionTodosChanged, parsed_type() will return SessionTodosChanged — not Unknown — so the predicate would never match. See the inline comments for concrete suggested fixes.
✅ All other SDKs look consistent
Generated type names, field names, event names, and the overall test structure are parallel across Node.js, Python, Go, .NET, and Java (noting the Java WIP status already called out in the PR description).
Generated by SDK Consistency Review Agent for issue #1622 · sonnet46 1.7M · ◷
There was a problem hiding this comment.
Generated by SDK Consistency Review Agent for issue #1622 · sonnet46 1.7M
| let todos_changed = wait_for_event(session.subscribe(), "todos changed", |event| { | ||
| event.parsed_type() == SessionEventType::Unknown | ||
| && event.event_type == "session.todos_changed" |
There was a problem hiding this comment.
Inconsistency with generated SDK types: The predicate combines SessionEventType::Unknown with a string check, but session.todos_changed already has its own generated variant SessionEventType::SessionTodosChanged in the Rust enum. The Unknown branch will never fire for a properly-recognised event, so this condition is always false.
Every other Rust E2E test uses the typed variant directly — e.g. rpc_event_log.rs uses event.parsed_type() == SessionEventType::SessionPlanChanged. Suggest:
let todos_changed = wait_for_event(session.subscribe(), "todos changed", |event| {
event.parsed_type() == SessionEventType::SessionTodosChanged
});| let value = session | ||
| .client() | ||
| .call( | ||
| "session.plan.readSqlTodosWithDependencies", | ||
| Some(json!({ "sessionId": session.id() })), | ||
| ) | ||
| .await | ||
| .expect("read SQL todos with dependencies"); | ||
| let result: PlanReadSqlTodosWithDependenciesResult = | ||
| serde_json::from_value(value).expect("deserialize todos with dependencies"); |
There was a problem hiding this comment.
Inconsistency: raw client().call() instead of typed RPC: All other five SDK tests use the generated typed plan API (session.rpc.plan.readSqlTodosWithDependencies(), session.Rpc.Plan.ReadSqlTodosWithDependenciesAsync(), etc.). The Rust SDK has the same generated typed helper — session.rpc().plan().read_sql_todos_with_dependencies() — which returns the generated PlanReadSqlTodosWithDependenciesResult (rows as Vec<PlanSqlTodosRow>, deps as Vec<PlanSqlTodoDependency>).
The three local structs PlanReadSqlTodosWithDependenciesResult, PlanTodo, and PlanTodoDependency (lines 15–33) also duplicate what's already in github_copilot_sdk::rpc. Suggest replacing the raw call + local types with:
use github_copilot_sdk::rpc::PlanReadSqlTodosWithDependenciesResult;
// ...
let result: PlanReadSqlTodosWithDependenciesResult = session
.rpc()
.plan()
.read_sql_todos_with_dependencies()
.await
.expect("read SQL todos with dependencies");This also lets you remove the serde::Deserialize, serde_json::json, and three local struct definitions from the top of the file.
Adds E2E coverage for the new runtime APIs in copilot-agent-runtime#10061:
session.todos_changedevent fires when the agent'ssqltool issues a write touching the promptedtodos/todo_depstables.session.plan.readSqlTodosWithDependenciesreturns the structured rows + dependency edges for progress UIs.Coverage
One scenario, one shared CAPI snapshot (
test/snapshots/session_todos_changed/fires_session_todos_changed_and_exposes_rows_and_dependencies.yaml), replayed across all 6 SDK languages. Each test instructs the agent to insert two known rows (alpha,beta) plus one dependency edge (beta -> alpha), then asserts:session.todos_changedevent was observed, andsession.plan.readSqlTodosWithDependencies()(typed RPC) returns ids[alpha, beta]and thebeta -> alphaedge.nodejs/test/e2e/session_todos_changed.e2e.test.tsdotnet/tests/GitHub.Copilot.Sdk.E2ETests/SessionTodosChangedTests.csgo/test/e2e/session_todos_changed_test.gopython/tests/e2e/test_session_todos_changed.pyrust/tests/session_todos_changed.rsjava/src/test/java/com/github/copilot/SessionTodosChangedTest.javaDraft notes
generated/tree were produced by re-running the per-language codegen against the runtime PR'sgenerated/*.schema.jsonfiles. They will reproduce automatically once the runtime PR ships in the@github/copilotnpm package.@github/copilotbump.