Skip to content

Commit b4b55a7

Browse files
authored
fix(e2e): fix kernel auto-launch and trust flow for fixture notebooks (#1046)
* fix(e2e): fix kernel auto-launch and trust flow for fixture notebooks Three root causes addressed: 1. Fixture notebooks missing kernelspec metadata — added python3 kernelspec to 1-vanilla.ipynb, 2-uv-inline.ipynb, and 5-pyproject.ipynb. All passing fixtures already had kernelspec; without it, detect_runtime() returned None and the auto-launch took a less-tested code path. 2. detect_runtime() didn't infer Python from runt.uv/runt.conda — added fallback so notebooks with inline Python deps are correctly identified as Python even without a kernelspec. Three new tests cover this. 3. Trust-dependent E2E specs waited for kernel ready before handling trust — untrusted notebooks block auto-launch, so the kernel never starts. Restructured uv-inline and conda-inline specs to trigger execution first (surfacing the trust dialog), approve it, then wait for kernel ready. Also: - Improved auto-launch decision logging (always logs skip reason) - getKernelStatus() helper now prefers data-kernel-status attribute - Removed unused imports in prewarmed-uv spec * fix(notebook-doc): use struct init to satisfy clippy field_reassign_with_default
1 parent ef2efce commit b4b55a7

File tree

9 files changed

+152
-98
lines changed

9 files changed

+152
-98
lines changed

crates/notebook-doc/src/metadata.rs

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -328,6 +328,11 @@ impl NotebookMetadataSnapshot {
328328
return Some("deno".to_string());
329329
}
330330

331+
// Fall back to runt.uv or runt.conda presence — these are implicitly Python
332+
if self.runt.uv.is_some() || self.runt.conda.is_some() {
333+
return Some("python".to_string());
334+
}
335+
331336
None
332337
}
333338

@@ -898,6 +903,60 @@ mod tests {
898903
assert_eq!(s.detect_runtime(), Some("deno".to_string()));
899904
}
900905

906+
#[test]
907+
fn test_detect_runtime_runt_uv_fallback() {
908+
let s = NotebookMetadataSnapshot {
909+
runt: RuntMetadata {
910+
uv: Some(UvInlineMetadata {
911+
dependencies: vec!["requests".to_string()],
912+
requires_python: None,
913+
prerelease: None,
914+
}),
915+
..RuntMetadata::default()
916+
},
917+
..Default::default()
918+
};
919+
assert_eq!(s.detect_runtime(), Some("python".to_string()));
920+
}
921+
922+
#[test]
923+
fn test_detect_runtime_runt_conda_fallback() {
924+
let s = NotebookMetadataSnapshot {
925+
runt: RuntMetadata {
926+
conda: Some(CondaInlineMetadata {
927+
dependencies: vec!["numpy".to_string()],
928+
channels: vec!["conda-forge".to_string()],
929+
python: None,
930+
}),
931+
..RuntMetadata::default()
932+
},
933+
..Default::default()
934+
};
935+
assert_eq!(s.detect_runtime(), Some("python".to_string()));
936+
}
937+
938+
#[test]
939+
fn test_detect_runtime_kernelspec_takes_priority_over_runt_uv() {
940+
// kernelspec should still win even if runt.uv is present
941+
let s = NotebookMetadataSnapshot {
942+
kernelspec: Some(KernelspecSnapshot {
943+
name: "deno".to_string(),
944+
display_name: "Deno".to_string(),
945+
language: Some("typescript".to_string()),
946+
}),
947+
runt: RuntMetadata {
948+
uv: Some(UvInlineMetadata {
949+
dependencies: vec!["requests".to_string()],
950+
requires_python: None,
951+
prerelease: None,
952+
}),
953+
..RuntMetadata::default()
954+
},
955+
..Default::default()
956+
};
957+
assert_eq!(s.detect_runtime(), Some("deno".to_string()));
958+
}
959+
901960
#[test]
902961
fn test_detect_runtime_none_for_empty_metadata() {
903962
let s = NotebookMetadataSnapshot::default();

crates/notebook/fixtures/audit-test/1-vanilla.ipynb

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,11 @@
22
"nbformat": 4,
33
"nbformat_minor": 5,
44
"metadata": {
5+
"kernelspec": {
6+
"name": "python3",
7+
"display_name": "Python 3",
8+
"language": "python"
9+
},
510
"runt": {
611
"schema_version": "1"
712
}

crates/notebook/fixtures/audit-test/2-uv-inline.ipynb

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,11 @@
1313
}
1414
],
1515
"metadata": {
16+
"kernelspec": {
17+
"display_name": "Python 3",
18+
"language": "python",
19+
"name": "python3"
20+
},
1621
"runt": {
1722
"env_id": "fixture-uv-inline",
1823
"uv": {

crates/notebook/fixtures/audit-test/pyproject-project/5-pyproject.ipynb

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,11 @@
1313
}
1414
],
1515
"metadata": {
16+
"kernelspec": {
17+
"display_name": "Python 3",
18+
"language": "python",
19+
"name": "python3"
20+
},
1621
"runt": {
1722
"env_id": "fixture-pyproject"
1823
}

crates/runtimed/src/notebook_sync_server.rs

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -954,7 +954,7 @@ where
954954
let is_new_notebook =
955955
!notebook_path_snapshot.exists() && uuid::Uuid::parse_str(&notebook_id).is_ok();
956956

957-
let (should_auto_launch, trust_status) = {
957+
let (should_auto_launch, trust_status, has_kernel) = {
958958
let trust_state = room.trust_state.read().await;
959959
let has_kernel = room.has_kernel().await;
960960
let status = trust_state.status.clone();
@@ -967,7 +967,7 @@ where
967967
// For new notebooks (UUID, no file): NoDependencies is safe to auto-launch
968968
// For newly-created notebooks at a path: also safe to auto-launch
969969
&& (notebook_path_snapshot.exists() || is_new_notebook || created_new_at_path);
970-
(should_launch, status)
970+
(should_launch, status, has_kernel)
971971
};
972972

973973
if should_auto_launch {
@@ -994,13 +994,11 @@ where
994994
)
995995
.await;
996996
});
997-
} else if !matches!(
998-
trust_status,
999-
runt_trust::TrustStatus::Trusted | runt_trust::TrustStatus::NoDependencies
1000-
) {
997+
} else {
1001998
info!(
1002-
"[notebook-sync] Notebook {} not trusted, skipping auto-launch (status: {:?})",
1003-
notebook_id, trust_status
999+
"[notebook-sync] Auto-launch skipped for {} (trust: {:?}, has_kernel: {}, path_exists: {}, is_new: {}, created_at_path: {})",
1000+
notebook_id, trust_status, has_kernel,
1001+
notebook_path_snapshot.exists(), is_new_notebook, created_new_at_path
10041002
);
10051003
}
10061004
}

e2e/helpers.js

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -284,6 +284,14 @@ export async function approveTrustDialog(timeout = 15000) {
284284
*/
285285
export async function getKernelStatus() {
286286
return await browser.execute(() => {
287+
// Prefer the data-kernel-status attribute (added in #1041) — it reflects
288+
// the raw status enum value and doesn't depend on DOM text rendering.
289+
const statusEl = document.querySelector('[data-testid="kernel-status"]');
290+
if (statusEl) {
291+
const attr = statusEl.getAttribute("data-kernel-status");
292+
if (attr) return attr.trim().toLowerCase();
293+
}
294+
// Fallback: read the visible status text from the toolbar
287295
const el = document.querySelector(
288296
'[data-testid="notebook-toolbar"] .capitalize',
289297
);

e2e/specs/conda-inline.spec.js

Lines changed: 32 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,10 @@
66
*
77
* Fixture: 3-conda-inline.ipynb (has markupsafe dependency via conda)
88
*
9-
* Updated to use setCellSource + explicit button clicks for compatibility
10-
* with tauri-plugin-webdriver (synthetic keyboard events don't work).
9+
* Flow: Notebooks with inline deps are untrusted by default. The kernel
10+
* won't auto-launch until the user approves the trust dialog. This spec
11+
* triggers execution to surface the dialog, approves it, then verifies
12+
* the kernel starts with the correct conda inline environment.
1113
*/
1214

1315
import { browser } from "@wdio/globals";
@@ -20,9 +22,34 @@ import {
2022
} from "../helpers.js";
2123

2224
describe("Conda Inline Dependencies", () => {
23-
it("should auto-launch kernel (may need trust approval)", async () => {
25+
it("should start kernel after trust approval", async () => {
26+
console.log("[conda-inline] Waiting for notebook to sync...");
27+
await waitForNotebookSynced();
28+
29+
// For untrusted notebooks, the kernel won't auto-launch.
30+
// Trigger execution to surface the trust dialog.
31+
const codeCell = await $('[data-cell-type="code"]');
32+
await codeCell.waitForExist({ timeout: 10000 });
33+
34+
await setCellSource(codeCell, "import sys; print(sys.executable)");
35+
36+
const executeButton = await codeCell.$('[data-testid="execute-button"]');
37+
await executeButton.waitForClickable({ timeout: 5000 });
38+
await executeButton.click();
39+
console.log("[conda-inline] Clicked execute to trigger trust dialog");
40+
41+
// Approve the trust dialog (inline deps require approval)
42+
const approved = await approveTrustDialog(30000);
43+
if (approved) {
44+
console.log("[conda-inline] Trust dialog approved");
45+
} else {
46+
console.log(
47+
"[conda-inline] No trust dialog appeared (may already be trusted)",
48+
);
49+
}
50+
51+
// Now wait for kernel to be ready (300s for conda env creation on cold CI)
2452
console.log("[conda-inline] Waiting for kernel ready (up to 300s)...");
25-
// Wait for kernel or trust dialog (300s for first startup + conda env creation)
2653
await waitForKernelReady(300000);
2754
console.log("[conda-inline] Kernel is ready");
2855
});
@@ -46,48 +73,18 @@ describe("Conda Inline Dependencies", () => {
4673

4774
expect(await depsToggle.getAttribute("data-env-manager")).toBe("conda");
4875
expect(await depsToggle.getAttribute("data-runtime")).toBe("python");
49-
console.log("[conda-inline] Conda badge verified in toolbar");
5076
});
5177

52-
it("should have inline deps available after trust", async () => {
53-
console.log("[conda-inline] Waiting for notebook to sync...");
54-
await waitForNotebookSynced();
55-
56-
// Find the first code cell
78+
it("should execute code in conda inline environment", async () => {
5779
const codeCell = await $('[data-cell-type="code"]');
5880
await codeCell.waitForExist({ timeout: 10000 });
59-
console.log("[conda-inline] Found first code cell");
6081

61-
// Set the cell source via CodeMirror dispatch (bypasses keyboard events)
6282
await setCellSource(codeCell, "import sys; print(sys.executable)");
63-
console.log("[conda-inline] Set cell source via setCellSource");
6483

65-
// Click the execute button
6684
const executeButton = await codeCell.$('[data-testid="execute-button"]');
6785
await executeButton.waitForClickable({ timeout: 5000 });
6886
await executeButton.click();
69-
console.log("[conda-inline] Clicked execute button");
70-
71-
// May need to approve trust dialog for inline deps
72-
const approved = await approveTrustDialog(15000);
73-
if (approved) {
74-
console.log(
75-
"[conda-inline] Trust dialog approved, waiting for kernel restart...",
76-
);
77-
// If trust dialog appeared, wait for kernel to restart with deps
78-
await waitForKernelReady(300000);
79-
console.log("[conda-inline] Kernel restarted after trust approval");
8087

81-
// Re-execute after kernel restart by clicking the button again
82-
const reExecuteButton = await codeCell.$(
83-
'[data-testid="execute-button"]',
84-
);
85-
await reExecuteButton.waitForClickable({ timeout: 5000 });
86-
await reExecuteButton.click();
87-
console.log("[conda-inline] Re-executed cell after kernel restart");
88-
}
89-
90-
// Wait for output
9188
const output = await waitForCellOutput(codeCell, 120000);
9289
console.log(`[conda-inline] Cell output: ${output}`);
9390

@@ -96,31 +93,21 @@ describe("Conda Inline Dependencies", () => {
9693
});
9794

9895
it("should be able to import inline dependency", async () => {
99-
// Find the cells — use a second cell if available, otherwise the first
10096
const cells = await $$('[data-cell-type="code"]');
10197
const cell = cells.length > 1 ? cells[1] : cells[0];
102-
console.log(
103-
`[conda-inline] Using cell index ${cells.length > 1 ? 1 : 0} for import test`,
104-
);
10598

106-
// Set the cell source directly via CodeMirror dispatch
10799
await setCellSource(
108100
cell,
109101
"import markupsafe; print(markupsafe.__version__)",
110102
);
111-
console.log("[conda-inline] Set import test source via setCellSource");
112103

113-
// Click the execute button
114104
const executeButton = await cell.$('[data-testid="execute-button"]');
115105
await executeButton.waitForClickable({ timeout: 5000 });
116106
await executeButton.click();
117-
console.log("[conda-inline] Clicked execute button for import test");
118107

119-
// Wait for version output
120108
const output = await waitForCellOutput(cell, 30000);
121109
console.log(`[conda-inline] Import test output: ${output}`);
122110

123-
// Should show a version number (e.g., "1.26.4")
124111
expect(output).toMatch(/^\d+\.\d+/);
125112
});
126113
});

e2e/specs/prewarmed-uv.spec.js

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,7 @@
1212

1313
import { browser } from "@wdio/globals";
1414
import {
15-
isManagedEnv,
1615
setCellSource,
17-
waitForAppReady,
1816
waitForCellOutput,
1917
waitForKernelReady,
2018
waitForNotebookSynced,

0 commit comments

Comments
 (0)