-
Notifications
You must be signed in to change notification settings - Fork 37
feat: agent background edits on notebooks #586
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
emrberk
wants to merge
22
commits into
feat/notebook-cell-toolbar-redesign
Choose a base branch
from
feat/agent-passive-notebook-edits
base: feat/notebook-cell-toolbar-redesign
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
22 commits
Select commit
Hold shift + click to select a range
bc9ef5b
feat: agent background edits on notebooks
emrberk e1704aa
submodule
emrberk 128a681
reviews pt1
emrberk 64e720b
simplifications on controller and freshness/ownership
emrberk 651cd8c
refactor(notebooks): make getCellBasics required, drop getCellSql/get…
emrberk c8ecf35
chore(notebooks): phase 6 sweep — dead exports, dead activate knob, r…
emrberk 4373d4c
fix(notebooks): report superseded live agent runs as unverified
emrberk 76ec5b7
refactor(notebooks): collapse NotebookController and unify tool dispa…
emrberk 6ea8c49
propagate notebook tool error on seed phase to the user
emrberk 330777c
remove patchCell
emrberk b227253
cleanup unused functions, make properties required
emrberk e1e2690
harden type definition, simplifications
emrberk af23403
reviews
emrberk 641cecf
smoke test fixes
emrberk 9c43ce8
fix(notebooks): include cell name in list_cells payload
emrberk 0569fd4
fix(security): fail closed on validate-ERROR SQL so writes can't bypa…
emrberk 83d4b6a
Revert "fix(security): fail closed on validate-ERROR SQL so writes ca…
emrberk 4df047d
reviews
emrberk 28a7631
reviews for notifications and freshness
emrberk 6e55cf6
submodule
emrberk df41f42
Merge branch 'feat/notebook-cell-toolbar-redesign' into feat/agent-pa…
emrberk e503f80
update gitleaksignore
emrberk File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,130 @@ | ||
| /// <reference types="cypress" /> | ||
|
|
||
| // E2E coverage for background (passive) agent notebook edits over the MCP | ||
| // bridge: creating and editing a notebook never steals the active tab, the | ||
| // footer popper announces the change, and View lands on the edited notebook | ||
| // with the agent's cells persisted. | ||
|
|
||
| const contextPath = process.env.QDB_HTTP_CONTEXT_WEB_CONSOLE || "" | ||
| const baseUrl = `http://localhost:9999${contextPath}` | ||
|
|
||
| const { | ||
| installFakeWebSocket, | ||
| TEST_BRIDGE_TOKEN, | ||
| TEST_BRIDGE_URL, | ||
| } = require("../../utils/mcpFakeWebSocket") | ||
|
|
||
| const deepLinkSuffix = () => | ||
| `?mcp-pair=1&mcp-ws=${encodeURIComponent(TEST_BRIDGE_URL)}` + | ||
| `&mcp-token=${encodeURIComponent(TEST_BRIDGE_TOKEN)}` | ||
|
|
||
| const loginAndVisitDeepLink = () => { | ||
| cy.visit(`${baseUrl}/${deepLinkSuffix()}`, { | ||
| onBeforeLoad: (win) => { | ||
| win.localStorage.clear() | ||
| win.sessionStorage.clear() | ||
| win.indexedDB.deleteDatabase("web-console") | ||
| win.localStorage.setItem( | ||
| "mcp:permissions", | ||
| JSON.stringify({ grantSchemaAccess: true, read: true, write: true }), | ||
| ) | ||
| installFakeWebSocket(win) | ||
| }, | ||
| }) | ||
| cy.loginWithUserAndPassword() | ||
| } | ||
|
|
||
| const waitForPaired = () => { | ||
| cy.window({ timeout: 10000 }).its("__mcpFakeWS").should("exist") | ||
| cy.window({ timeout: 10000 }).should((win) => { | ||
| expect(win.__mcpFakeWS.framesOfType("hello").length).to.be.greaterThan(0) | ||
| }) | ||
| cy.window().then((win) => win.__mcpFakeWS.helloAck()) | ||
| cy.getByDataHook("mcp-bridge-status-pill", { timeout: 10000 }).should( | ||
| "contain", | ||
| "MCP connected", | ||
| ) | ||
| } | ||
|
|
||
| const toolResult = (win, requestId) => | ||
| win.__mcpFakeWS | ||
| .framesOfType("tool_result") | ||
| .find((r) => r.requestId === requestId) | ||
|
|
||
| const activeTabTitle = () => | ||
| cy.get(".chrome-tab[active] .chrome-tab-title").first().invoke("text") | ||
|
|
||
| const awaitToolResult = (id) => { | ||
| cy.window({ timeout: 10000 }).should((win) => { | ||
| expect(toolResult(win, id), `result for ${id}`).to.exist | ||
| }) | ||
| return cy.window().then((win) => { | ||
| const result = toolResult(win, id) | ||
| expect(result.isError, `tool ${id} errored`).to.not.equal(true) | ||
| // The single-line JSON payload may be wrapped by a permissions notice | ||
| // above and a since-last-check block below. | ||
| const text = result.content[0].text | ||
| const payloadLine = text | ||
| .split("\n") | ||
| .find((line) => line.trimStart().startsWith("{")) | ||
| expect(payloadLine, `JSON payload in result for ${id}`).to.exist | ||
| return JSON.parse(payloadLine) | ||
| }) | ||
| } | ||
|
|
||
| describe("agent background notebook edits (e2e)", () => { | ||
| it("creates and fills a notebook in the background; View opens it with the cells persisted", () => { | ||
| loginAndVisitDeepLink() | ||
| cy.getByDataHook("mcp-pair-consent-connect").click() | ||
| waitForPaired() | ||
|
|
||
| activeTabTitle().then((initialTab) => { | ||
| // Agent creates a notebook — the active tab must not change. | ||
| cy.window() | ||
| .then((win) => | ||
| awaitToolResult( | ||
| win.__mcpFakeWS.toolCall("create_notebook", { | ||
| label: "Agent NB", | ||
| }), | ||
| ), | ||
| ) | ||
| .then((created) => { | ||
| expect(created.bufferId).to.be.a("number") | ||
| expect(created.hint).to.match(/background/i) | ||
| cy.get(`.chrome-tab[data-tab-title="Agent NB"]`).should("exist") | ||
| activeTabTitle().should("equal", initialTab) | ||
|
|
||
| // Read → edit, per the freshness gate; still fully in the background. | ||
| cy.window().then((win) => | ||
| awaitToolResult( | ||
| win.__mcpFakeWS.toolCall("get_notebook_state", { | ||
| buffer_id: created.bufferId, | ||
| }), | ||
| ), | ||
| ) | ||
| cy.window().then((win) => | ||
| awaitToolResult( | ||
| win.__mcpFakeWS.toolCall("add_cell", { | ||
| buffer_id: created.bufferId, | ||
| sql: "SELECT 123 as agent_made_this", | ||
| after_cell_id: null, | ||
| run: false, | ||
| type: "sql", | ||
| }), | ||
| ), | ||
| ) | ||
| activeTabTitle().should("equal", initialTab) | ||
|
|
||
| // The footer popper announces the background change. | ||
| cy.getByDataHook("agent-changes-popper", { timeout: 10000 }) | ||
| .should("be.visible") | ||
| .and("contain", "Agent NB") | ||
|
|
||
| // View lands on the notebook with the agent's cell persisted. | ||
| cy.getByDataHook("agent-changes-view").click() | ||
| activeTabTitle().should("equal", "Agent NB") | ||
| cy.contains("agent_made_this", { timeout: 10000 }).should("exist") | ||
| }) | ||
| }) | ||
| }) | ||
| }) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,92 @@ | ||
| // Fake MCP-bridge WebSocket shared by the bridge e2e specs. Installed before | ||
| // the app boots; the instance is exposed on `win.__mcpFakeWS`. | ||
|
|
||
| const TEST_BRIDGE_URL = "ws://127.0.0.1:57123" | ||
| const TEST_BRIDGE_TOKEN = "abcdef0123456789abcdef0123456789" | ||
| // Must match src/utils/mcp/protocolVersion.ts; mismatches are silently dropped. | ||
| const PROTOCOL_VERSION = "1" | ||
|
|
||
| const installFakeWebSocket = (win) => { | ||
| class FakeWS { | ||
| constructor(url) { | ||
| this.url = url | ||
| this.readyState = 0 | ||
| this.sent = [] | ||
| this.listeners = {} | ||
| win.__mcpFakeWS = this | ||
| // Async "open" mirrors real WebSocket — lets listener registration finish. | ||
| win.setTimeout(() => { | ||
| this.readyState = 1 | ||
| this._dispatch("open", {}) | ||
| }, 0) | ||
| } | ||
| addEventListener(type, fn) { | ||
| if (!this.listeners[type]) this.listeners[type] = new Set() | ||
| this.listeners[type].add(fn) | ||
| } | ||
| removeEventListener(type, fn) { | ||
| this.listeners[type]?.delete(fn) | ||
| } | ||
| send(data) { | ||
| if (this.readyState !== 1) { | ||
| throw new Error("FakeWS.send while not open") | ||
| } | ||
| this.sent.push(data) | ||
| const parsed = JSON.parse(data) | ||
| if (parsed.type === "ping") { | ||
| this.receive({ | ||
| v: PROTOCOL_VERSION, | ||
| type: "pong", | ||
| nonce: parsed.nonce, | ||
| }) | ||
| } | ||
| } | ||
| close() { | ||
| this.readyState = 3 | ||
| this._dispatch("close", { code: 1000, reason: "test-close" }) | ||
| } | ||
| receive(payload) { | ||
| this._dispatch("message", { data: JSON.stringify(payload) }) | ||
| } | ||
| helloAck() { | ||
| this.receive({ | ||
| v: PROTOCOL_VERSION, | ||
| type: "hello_ack", | ||
| sessionId: "test-session", | ||
| heartbeatIntervalMs: 60000, | ||
| seenToolCount: 1, | ||
| }) | ||
| } | ||
| toolCall(name, args, requestId) { | ||
| const id = requestId || "req-" + Math.random().toString(36).slice(2) | ||
| this.receive({ | ||
| v: PROTOCOL_VERSION, | ||
| type: "tool_call", | ||
| requestId: id, | ||
| name, | ||
| arguments: args, | ||
| deadlineMs: 15000, | ||
| }) | ||
| return id | ||
| } | ||
| _dispatch(type, ev) { | ||
| this.listeners[type]?.forEach((fn) => fn(ev)) | ||
| } | ||
| framesOfType(type) { | ||
| return this.sent.map((s) => JSON.parse(s)).filter((m) => m.type === type) | ||
| } | ||
| } | ||
| // Real-WebSocket statics — MCPBridgeClient reads WebSocket.OPEN. | ||
| FakeWS.CONNECTING = 0 | ||
| FakeWS.OPEN = 1 | ||
| FakeWS.CLOSING = 2 | ||
| FakeWS.CLOSED = 3 | ||
| win.WebSocket = FakeWS | ||
| } | ||
|
|
||
| module.exports = { | ||
| installFakeWebSocket, | ||
| PROTOCOL_VERSION, | ||
| TEST_BRIDGE_TOKEN, | ||
| TEST_BRIDGE_URL, | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.