Skip to content

[Do not merge] Trigger CI for Supervisor API PR - #492

Closed
pkosiec wants to merge 9 commits into
mainfrom
pkosiec/temp-supervisor-api
Closed

[Do not merge] Trigger CI for Supervisor API PR#492
pkosiec wants to merge 9 commits into
mainfrom
pkosiec/temp-supervisor-api

Conversation

@pkosiec

@pkosiec pkosiec commented Jul 28, 2026

Copy link
Copy Markdown
Member

Apply Mario's defensive/correctness fixes from the supervisor API adapter
review without touching the public API shape (sections 1-8 will land in a
stacked branch). Highlights:

High
- Route the three SSE error-leak sites in supervisor-api.ts (streamBody
  catch, mapEvent "error", output_item.done with id="error") through a
  single emitError helper that returns a stable client-facing code
  (`Supervisor API error (transport|upstream_failed|upstream_tool|
  upstream_unknown)`) and logs the verbose detail server-side only.
  Addresses CWE-209 verbatim-upstream-error-text leak.

Medium
- Gate the terminal {status:"complete"} emission on lastCompleted.status
  / .error / .incomplete_details so a `response.completed` with a nested
  failed status no longer silently succeeds; surface as upstream_failed
  instead. Regression tests added.
- Skip the terminal error in the streamBody catch when signal.aborted —
  consumer-initiated aborts now end with a clean stop, not a contradictory
  terminal error event. Regression test added.
- Tighten the output_item.done error match: require item.type === "error"
  (or pair the reserved id="error" with a non-message type) so a stray
  assistant message with id="error" is not mis-classified.
- Add maxLineChars / maxBufferChars caps to readSseEvents with 1 MiB / 8
  MiB defaults; throw on overflow. Addresses CWE-770. Tests added.
- Docs: add a CWE-1427 callout warning that hosted-tool `description` is a
  prompt-injection sink — do not derive it from untrusted input.
- Redact the no-delta warning log: summariseErrorPayload extracts a short
  `type: message` line; full payload only via DEBUG. Addresses CWE-532.
- Gate the buffer-level CRLF normalize in sse-reader on `\r` presence to
  skip the regex on LF-only steady state.

Low
- mapEvent("error") fallback no longer wraps "Unknown error" with literal
  JSON quotes (uses string branch).
- Drop the misleading "we await the factory at module init" comment in
  dev-playground; the code never awaits.
- Fix @example imports in supervisor-api.ts JSDoc to use
  @databricks/appkit/beta (the actual public re-export).
- Replace trimStart() with single-U+0020 strip in sse-reader per the SSE
  spec; remove the now-dead per-line `\r$` strip after the buffer-level
  CRLF normalise.
- Flag streamPath as @internal in connectors/serving/client.ts noting the
  CWE-918 SSRF risk if it ever leaks to user-controlled input.
- Add JSDoc warning to workspaceClient on SupervisorApiAdapterOptions:
  passing a per-request OBO client would leak identity across requests
  (CWE-664).

Signed-off-by: Hubert Zub <hubert.zub@databricks.com>
Signed-off-by: Hubert Zub <hubert.zub@databricks.com>
Address structural feedback from Mario Cadenas' review of PR #345 (sections
1-8). Stacked on top of the §9 defensive fixes commit.

API changes (BETA surface only):
- Add `DatabricksAdapter.fromSupervisorApi` static factory for discoverability
  alongside `.fromChatCompletions`.
- Shrink `SupervisorApiAdapterOptions` to `{ model, workspaceClient? }`; tools
  no longer live on the adapter.
- Hosted tools (`supervisorTools.*`) now return tagged `HostedSupervisorTool`
  records and accept named options instead of positional args.
- Declare hosted tools on the agent's `tools` map (same place as function
  tools / sub-agents); the agents plugin and `runAgent` route them to the
  adapter via the new `AgentInput.extensions[SUPERVISOR_EXTENSION_KEY]`.
- Add capability-negotiation fields to `AgentAdapter`:
  `acceptsExtensions?` + `consumesInputTools?`. The agents plugin and
  `runAgent` warn at registration when adapter capabilities don't match
  declared tools.

Internals:
- Extend `ResolvedToolEntry` / `StandaloneEntry` with a `hosted-supervisor`
  branch; `classifyTool` matches it before MCP hosted-tool rejection so
  standalone `runAgent` supports supervisor tools.
- Defense-in-depth: both indexers throw if a `hosted-supervisor` entry is
  ever dispatched as a callable function.
- `DatabricksAdapter.fromSupervisorApi` uses a dynamic import to avoid
  load-time cycles.

Docs:
- Rewrite supervisor-API section in docs/plugins/agents.md for the new shape.
- Add cross-adapter sub-agent composition note (one-directional: chat
  parents can call supervisor children, not vice-versa, until SA's
  function-call events are routed back through `context.executeTool`).

Playground:
- Update dev-playground supervisor agent to the new shape
  (`DatabricksAdapter.fromSupervisorApi(...)` + tools on `createAgent`).

Tests:
- Rewrite supervisor-api.test.ts factory + adapter tests for the new shape.
- Add `isSupervisorTool` and `DatabricksAdapter.fromSupervisorApi` tests.
- New regression tests in `run-agent.test.ts` covering the
  hosted-supervisor extension-routing path and both capability-mismatch
  warnings.
- New agents-plugin tests covering the same warning paths and the new
  `hosted-supervisor` tool-index branch.

Signed-off-by: Hubert Zub <hubert.zub@databricks.com>
Signed-off-by: Hubert Zub <hubert.zub@databricks.com>
@pkosiec pkosiec changed the title [Do not merge] [Do not merge] Trigger CI for Supervisor API PR Jul 28, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📦 Bundle size report

Compared against bundle-size-baseline.json (main).

@databricks/appkit

npm tarball (packed): 797 KB (+34 KB) — gzipped download (dist + bin; excludes release-only docs/NOTICE).

dist raw gzip
JS (runtime) 820 KB (+29 KB) 286 KB (+9.9 KB)
Type declarations 302 KB (+17 KB) 103 KB (+5.9 KB)
Source maps 1.6 MB (+65 KB) 536 KB (+21 KB)
Other 11 KB 3.7 KB
Total 2.7 MB (+110 KB) 929 KB (+37 KB)
Per-entry composition (own code — deps external (as shipped))
Entry Initial (gz) Lazy (gz) Total (gz) node_modules (min) Own code (min)
. 86 KB (+10 B) 2.5 KB 89 KB (+10 B) external 281 KB (+42 B)
./beta 44 KB (+4.6 KB) 429 B (+198 B) 45 KB (+4.8 KB) external 129 KB (+9.5 KB)
./type-generator 19 KB 0 B 19 KB external 54 KB

Chunks:

Entry Chunk Load Size (gz)
. index.js initial 82 KB
. utils.js initial 4.0 KB
. remote-tunnel-manager.js lazy 2.5 KB
./beta beta.js initial 29 KB
./beta stream-manager.js initial 5.8 KB
./beta wide-event-emitter.js initial 3.2 KB
./beta databricks.js initial 3.0 KB
./beta configuration.js initial 2.1 KB
./beta service-context.js initial 1.3 KB
./beta client-options.js initial 220 B
./beta supervisor-api.js lazy 184 B
./beta databricks.js lazy 132 B
./beta index.js lazy 113 B
./type-generator index.js initial 19 KB

@databricks/appkit-ui

npm tarball (packed): 305 KB — gzipped download (dist + bin; excludes release-only docs/NOTICE).

dist raw gzip
JS (runtime) 359 KB 119 KB
Type declarations 205 KB 74 KB
Source maps 685 KB 224 KB
CSS 16 KB 3.3 KB
Total 1.2 MB 421 KB
Per-entry composition (consumer bundle — deps bundled, peerDeps external)
Entry Initial (gz) Lazy (gz) Total (gz) node_modules (min) Own code (min)
./js 4.3 KB 49 KB 54 KB 208 KB 12 KB
./js/beta 20 B 0 B 20 B 0 B 0 B
./react 429 KB 49 KB 478 KB 1.3 MB 168 KB
./react/beta 20 B 0 B 20 B 0 B 0 B

Chunks:

Entry Chunk Load Size (gz)
./js index.js initial 4.2 KB
./js chunk initial 120 B
./js apache-arrow lazy 49 KB
./js/beta beta.js initial 20 B
./react index.js initial 427 KB
./react tslib initial 2.1 KB
./react apache-arrow lazy 49 KB
./react/beta beta.js initial 20 B

@github-actions

github-actions Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

🤖 AppKit PR bot

🔬 Run evals

Start an eval for this PR from the evals-monitor app: Go to Evals Monitor →

📦 Try this PR's app template

Scaffolds a new app from this PR's SDK build. Run it in any folder (requires the GitHub CLI — gh auth login — and the Databricks CLI):

gh run download 30355377147 -R databricks/appkit -n appkit-template-0.48.0-pr.edf97e3-pkosiec-temp-supervisor-api-492 -D appkit-pr-492 \
  && unzip -o "appkit-pr-492/appkit-template-0.48.0-pr.edf97e3-pkosiec-temp-supervisor-api-492.zip" -d "appkit-pr-492" \
  && databricks apps init --template "appkit-pr-492"

The template pins @databricks/appkit and @databricks/appkit-ui to tarballs built from this branch, so the scaffolded app runs against this PR's code.

hubertzub-db and others added 3 commits July 28, 2026 12:11
Signed-off-by: Hubert Zub <hubert.zub@databricks.com>
Signed-off-by: Hubert Zub <hubert.zub@databricks.com>
Co-authored-by: Pawel Kosiec <pawel.kosiec@gmail.com>
@pkosiec
pkosiec marked this pull request as ready for review July 28, 2026 11:28
@pkosiec
pkosiec requested a review from a team as a code owner July 28, 2026 11:28
@pkosiec
pkosiec requested a review from ditadi July 28, 2026 11:28
Signed-off-by: Pawel Kosiec <pawel.kosiec@databricks.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants