-
Notifications
You must be signed in to change notification settings - Fork 13
feat(mcp): drive ghost serve UI for query visualization & charting #36
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
murrayju
wants to merge
77
commits into
main
Choose a base branch
from
murrayju/mcp-serve-visualization
base: main
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
77 commits
Select commit
Hold shift + click to select a range
71db980
feat(mcp): drive ghost serve UI for query visualization & charting
murrayju 6d9a8d7
feat(mcp): always return a chart image from visualization tools
murrayju 559877d
fix(mcp): report chart render failures as chart_error, never fail the…
murrayju 8fed8a4
feat(web): show a disconnected banner when the backend goes away
murrayju 46f615b
feat(serve): add -p/--port and -n/--no-open shorthands; use them in r…
murrayju f477466
fix(web): disable animation for agent chart screenshots
murrayju fe793bb
feat(web): add chart config history
murrayju d874218
feat(mcp): surface chart config editor diagnostics to the agent
murrayju bca6696
fix(serve): enforce read-only mode, report true row count, harden age…
murrayju 3238413
fix(mcp): correct RowsAffected semantics, join close errors, fix scre…
murrayju 99e83c1
fix(mcp): check API client before opening browser in chart/ui_state t…
murrayju a1fdf81
fix(web): record the chart config that actually rendered, not the liv…
murrayju c7a7d2c
fix(serve): propagate MCP request cancellation to browser-run queries
murrayju 6b16739
fix(web): harden agent last-run scoping and panel teardown
murrayju 5c4cdad
fix(serve): return config from loadClient for consistent snapshot
murrayju 6859cf8
fix(mcp): render visualized SQL NULL cells as NULL, not empty string
murrayju 16905d9
fix(mcp): report accurate rows_affected for visualized SQL
murrayju a338daf
fix(mcp): include structured output in text content for visualized SQL
murrayju 0a7e086
fix(mcp): don't duplicate chart error in visualize prose summary
murrayju fe034d0
fix(mcp): normalize database name refs to id for visualization UI
murrayju 43c29e7
fix(serve): clear stale agent state on SSE reconnect
murrayju 37c9101
fix(mcp): carry command_tag through visualized query results
murrayju 3ceb25d
fix(mcp): surface chart render errors from ghost_chart gracefully
murrayju 28626cf
fix(web): honor config backgroundColor in chart screenshots
murrayju 7140750
fix(web): wait for ECharts render to finish before capturing screenshot
murrayju b616bd9
fix(web): chart the full result set, not the agent's row cap
murrayju 1881b4c
fix(mcp): render JSON cell values as JSON, not Go debug format
murrayju aaa4ea2
fix(mcp): keep rows_affected/command_tag for no-row UI-state runs
murrayju 25e71fa
fix(web): validate chart command before mutating the UI
murrayju 93e0c8e
fix(web): scope agent command cancellation to its own query
murrayju d0a3b3d
fix(mcp): preserve exact number text in visualized cell values
murrayju 93678b3
fix(mcp): fail fast with friendly error on visualizing a non-ready db
murrayju 5ec1701
fix(web): bound pre-empted cancel tracking to a single slot
murrayju c79d53e
fix(web): clear diagnostics timeout and clarify torn-down run error
murrayju 9c6375b
fix(serve): clear stale slot when removing a bridge client
murrayju 3a394ab
docs: track review cycle progress (rounds 1-3)
murrayju 5e7306e
fix(mcp): render visualized booleans as Postgres t/f text
murrayju 95a48c0
docs: track review cycle progress (round 4 + convergence)
murrayju 8ce8b00
fix(web): tear down chart screenshot timer/listener on every settle path
murrayju 63b2795
fix(serve): deliver agent command cancels reliably, not best-effort
murrayju 588b126
docs: track review cycle progress (round 5)
murrayju 0c5d31d
fix(web): harden ghost_chart against failed/uncached last runs
murrayju 330b66e
docs: track review cycle progress (round 6)
murrayju e285c84
fix(web): abort in-flight command on SSE drop; seed chart recorder ba…
murrayju 71b22ae
docs: track review cycle progress (round 7)
murrayju 6ee148a
fix(web): abort ghost_chart before mutating UI if the command is aban…
murrayju e056ba6
docs: track review cycle progress (round 8)
murrayju d6927a9
docs: track review cycle progress (round 9 — converged)
murrayju 4a361d0
fix(web): cap chart screenshot long edge under 2000px
murrayju 9207859
fix(serve): persist chartConfigHistory in /api/state
murrayju 0debbcc
fix(web): abort awaitExecutor when the visualize command is canceled
murrayju fd4cd82
docs(web): correct stale preemptedCommandId race comment
murrayju 584db73
docs: track review cycle progress (round 10 — opus)
murrayju bb97feb
improved tool descriptions, test coverage
murrayju 9dee074
don't return unrequested chart image
murrayju 52d4554
remove review md file
murrayju f37f6e8
docs(web): correct stale 'now' memoization comment in ChartHistoryModal
murrayju 4a4489a
fix(mcp): reject invalid visualize value with explicit error
murrayju 95389e0
docs(web): clarify cancellation settles (not rejects) agent runs
murrayju e1e85f7
analytics: record chart_config in MCP analytics events
murrayju a3ae042
docs: move MCP web-bridge detail out of Repository Structure
murrayju 0adcb28
refactor(mcp): make browser command type a named enum
murrayju 129a3f6
refactor(mcp): consolidate duplicate ChartDiagnostic type
murrayju 05b1a23
refactor(mcp): drop redundant newline in visualize summary
murrayju 85b96bd
feat(mcp): reference ECharts docs in chart_config tool descriptions
murrayju cf12ecb
refactor(mcp): hoist browser auth pre-check into ensureStarted
murrayju 8635607
docs(mcp): explain the chartCommand(input) cast
murrayju d44673f
feat(mcp): return structured output from ghost_chart
murrayju 80b3bd4
feat(mcp): split visualization out of ghost_sql into ghost_visualize
murrayju 860b57b
fix(serve): show chart config type diagnostics in the live editor
murrayju f0c9b7a
refactor(mcp): drop command tag from browser-backed query results
murrayju 200b004
loosen rows type to avoid noisy diagnostics in chart_config
murrayju 8aa1d91
docs(mcp): clarify when to use ghost_visualize vs ghost_sql
murrayju 08841f6
refactor(mcp): rely on SDK-applied schema defaults for limit/view
murrayju 1d48ff1
refactor(mcp): drop the view parameter from ghost_visualize
murrayju dbfbc22
wording tweaks
murrayju b3e2258
refactor(mcp): only render a chart when chart_config is provided
murrayju 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
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,197 @@ | ||
| package mcp | ||
|
|
||
| import ( | ||
| "bytes" | ||
| "context" | ||
| "encoding/base64" | ||
| "encoding/json" | ||
| "errors" | ||
| "fmt" | ||
| "log/slog" | ||
| "strings" | ||
| "sync" | ||
|
|
||
| "github.com/modelcontextprotocol/go-sdk/mcp" | ||
|
|
||
| "github.com/timescale/ghost/internal/common" | ||
| "github.com/timescale/ghost/internal/serve" | ||
| ) | ||
|
|
||
| // decodeImageDataURL parses a data URL (e.g. "data:image/png;base64,iVBOR...") | ||
| // into an MCP [mcp.ImageContent]. Returns nil if the string is empty or not a | ||
| // base64 data URL. | ||
| func decodeImageDataURL(dataURL string) (*mcp.ImageContent, error) { | ||
| if dataURL == "" { | ||
| return nil, nil | ||
| } | ||
| const prefix = "data:" | ||
| if !strings.HasPrefix(dataURL, prefix) { | ||
| return nil, errors.New("chart image is not a data URL") | ||
| } | ||
| comma := strings.IndexByte(dataURL, ',') | ||
| if comma < 0 { | ||
| return nil, errors.New("malformed image data URL") | ||
| } | ||
| meta := dataURL[len(prefix):comma] | ||
| mimeType, isBase64, _ := strings.Cut(meta, ";") | ||
| if isBase64 != "base64" { | ||
| return nil, errors.New("image data URL is not base64-encoded") | ||
| } | ||
| data, err := base64.StdEncoding.DecodeString(dataURL[comma+1:]) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to decode image data: %w", err) | ||
| } | ||
| return &mcp.ImageContent{Data: data, MIMEType: mimeType}, nil | ||
| } | ||
|
|
||
| // browserController lazily starts an in-process `ghost serve` web server and | ||
| // drives it via the agent [serve.Bridge]. It is owned by the MCP [Server] and | ||
| // only used by the local (stdio) transport — opening a browser from a remote | ||
| // HTTP server is meaningless, so the visualize/chart/ui_state tools are gated | ||
| // to local mode. | ||
| // | ||
| // The server is started on first use (first visualize/chart/ui_state tool | ||
| // call), the browser is opened when no client is connected, and everything is | ||
| // torn down when the MCP server shuts down. | ||
| type browserController struct { | ||
| app *common.App | ||
| logger *slog.Logger | ||
|
|
||
| mu sync.Mutex | ||
| bridge *serve.Bridge | ||
| store *serve.Store | ||
| server *serve.Server | ||
| } | ||
|
|
||
| func newBrowserController(app *common.App, logger *slog.Logger) *browserController { | ||
| return &browserController{ | ||
| app: app, | ||
| logger: ensureLogger(logger), | ||
| } | ||
| } | ||
|
|
||
| // ensureStarted lazily starts the in-process serve server (bound to an | ||
| // ephemeral loopback port) and returns the bridge. Subsequent calls return the | ||
| // already-running instance. | ||
| func (c *browserController) ensureStarted(ctx context.Context) (*serve.Bridge, error) { | ||
| // Verify the API client is available before starting the server or opening | ||
| // the browser. Without this, a logged-out user gets an opaque "no browser | ||
| // connected" timeout: the web app fails /api/bootstrap and never connects an | ||
| // active client, instead of the real auth/config error surfacing here. This | ||
| // runs on every call (before the already-started early-return) so expired | ||
| // credentials are caught too. | ||
| if _, _, err := c.app.GetClient(); err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| c.mu.Lock() | ||
| defer c.mu.Unlock() | ||
|
|
||
| if c.bridge != nil { | ||
| return c.bridge, nil | ||
| } | ||
|
|
||
| bridge := serve.NewBridge() | ||
| configDir := c.app.GetConfig().ConfigDir | ||
| store := serve.NewStore(configDir, c.logger) | ||
| handler := serve.NewHandler(serve.HandlerConfig{ | ||
| App: c.app, | ||
| Store: store, | ||
| Logger: c.logger, | ||
| Bridge: bridge, | ||
| }) | ||
| server := serve.NewServer("127.0.0.1", 0, handler.Handler(), c.logger) | ||
| if err := server.Start(ctx); err != nil { | ||
| store.Close() | ||
| return nil, fmt.Errorf("failed to start web server: %w", err) | ||
| } | ||
|
|
||
| c.bridge = bridge | ||
| c.store = store | ||
| c.server = server | ||
| c.logger.Info("Started in-process web UI for agent visualization", slog.String("url", server.URL())) | ||
| return bridge, nil | ||
| } | ||
|
|
||
| // url returns the URL of the running server, or "" if it hasn't been started. | ||
| func (c *browserController) url() string { | ||
| c.mu.Lock() | ||
| defer c.mu.Unlock() | ||
| if c.server == nil { | ||
| return "" | ||
| } | ||
| return c.server.URL() | ||
| } | ||
|
|
||
| // ensureClient starts the server (if needed), opens a browser when no client is | ||
| // connected, and waits for an active client to be ready to receive commands. | ||
| func (c *browserController) ensureClient(ctx context.Context) (*serve.Bridge, error) { | ||
| bridge, err := c.ensureStarted(ctx) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| if !bridge.HasActiveClient() { | ||
| url := c.url() | ||
| c.logger.Info("Opening browser for agent visualization", slog.String("url", url)) | ||
| if err := common.OpenBrowser(url); err != nil { | ||
| c.logger.Warn("Failed to open browser; open it manually", | ||
| slog.String("url", url), | ||
| slog.Any("error", err), | ||
| ) | ||
| } | ||
| if err := bridge.WaitForActiveClient(ctx); err != nil { | ||
| if errors.Is(err, serve.ErrNoActiveClient) { | ||
| return nil, fmt.Errorf("no browser connected to %s; open it to enable visualization", url) | ||
| } | ||
| return nil, err | ||
| } | ||
| } | ||
|
|
||
| return bridge, nil | ||
| } | ||
|
|
||
| // request dispatches a command to the active browser client and unmarshals the | ||
| // JSON response into out (which may be nil to ignore the response body). | ||
| func (c *browserController) request(ctx context.Context, commandType browserCommand, payload any, out any) error { | ||
| bridge, err := c.ensureClient(ctx) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| data, err := bridge.Request(ctx, string(commandType), payload) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| if out != nil && len(data) > 0 { | ||
| // Decode numbers as json.Number, not float64, so cell values keep their | ||
| // exact literal text. Plain float64 decoding would re-render large or | ||
| // whole numbers in exponent form (e.g. 10000000 -> "1e+07") when | ||
| // stringified, diverging from the server-side query path's text output. | ||
| dec := json.NewDecoder(bytes.NewReader(data)) | ||
| dec.UseNumber() | ||
| if err := dec.Decode(out); err != nil { | ||
| return fmt.Errorf("failed to parse browser response: %w", err) | ||
| } | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| // Close tears down the running server and store. Safe to call when nothing was | ||
| // started. | ||
| func (c *browserController) Close() error { | ||
| c.mu.Lock() | ||
| defer c.mu.Unlock() | ||
|
|
||
| if c.server == nil { | ||
| return nil | ||
| } | ||
| err := c.server.Close() | ||
| // Store.Close() returns no error — it logs any session-teardown failures | ||
| // internally. | ||
| c.store.Close() | ||
| c.server = nil | ||
| c.store = nil | ||
| c.bridge = nil | ||
| return err | ||
| } | ||
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason we use
127.0.0.1instead of justlocalhosthere (and ininternal/cmd/serve.go)? The MCP server itself useslocalhostwhen using the HTTP transport, so we're being somewhat inconsistent. I'm not sure of the pros/cons of either (maybelocalhostcan use IPv6 addresses too, or something like that?), but I feel like I seelocalhostmore often.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, there is a difference.
localhostcan mean IPv4 and/or IPv6. Claude tells me that go will only bind to one of the two addresses (order is platform dependent).I think if you use
localhoston both ends, it should work. But if we bindlocalhostand the user tries127.0.0.1in the browser, it might not. Whereas if we bind127.0.0.1, both should work in the browser. The counterpoint is that thenhttp://::1doesn't work... but I don't think many people use that.I agree with Claude that it is safer to just use
127.0.0.1in both the bind and url that we display/open. But also, if you feel strongly about consistency, I don't think this is super important.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a slight preference for
localhost, but mostly just because I think it looks better in the URL bar lol. Your call.