Uh oh!
There was an error while loading. Please reload this page.
WIP: feat: add volume serving plugin - #6
Closed
fjakobs wants to merge 1 commit into
Closed
Conversation
fjakobsforce-pushed
the
wip/volume-serving
branch
from
February 24, 2026 16:58
a61dae5 to
7f161e0Compare5 tasks
6 tasks
MarioCadenas
commented
May 12, 2026
Collaborator
closing as we already shipped a files plugin |
atilafassina added a commit
that referenced
this pull request
May 20, 2026
CI sync:template diff check caught two source/template drifts on 4ae1741: 1. lakebase manifest had been moved must -> should in the source, but the committed template still carried the rule under must. Resyncing now that sync:template is invoked as part of pnpm build. 2. TEMPLATE_SCAFFOLDING declared --template-dir and --config-dir as required: true, but the committed template had them as required: false (per MarioCadenas on PR #261#6 — these flags aren't actually required by databricks apps init). Flipping the source constant to match; resync now produces consistent output. After this commit, source and template are in lockstep; CI sync:template diff check should pass. Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu>
atilafassina added a commit
that referenced
this pull request
May 22, 2026
* feat: extend plugin and template manifest schemas with discovery, postScaffold, and scaffolding Xavier loop: iteration 1 — Phase 1 (Schema Definitions & Type Generation) Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu> * feat: add origin computation, scaffolding descriptor, and v2.0 template manifest emission Xavier loop: iteration 2 — Phase 2 (Origin Computation & Sync Enrichment) Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu> * feat: add semantic validation for dependsOn cycles, discovery profile, and postScaffold Xavier loop: iteration 3 — Phase 3 (Semantic Validation) Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu> * feat: annotate core plugin manifests with discovery descriptors and postScaffold steps Xavier loop: iteration 4 — Phase 4 (Core Plugin Manifest Annotations) Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu> * fix: inline template schema resourceFieldEntry and resourceRequirement for origin support JSON Schema Draft-07 additionalProperties:false blocks allOf composition. Inlined both defs in template schema so origin validates correctly. Xavier loop: iteration 5 — Phase 5 (Integration & Backpressure) Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu> * chore: address code reviews * chore: untrack .claude/scheduled_tasks.lock Accidentally committed in a1c30e3; it's ephemeral Claude Code loop state, not source. Flagged in PR #261 review. Signed-off-by: Atila Fassina <atila@fassina.eu> * feat: introduce zod-canonical manifest schema (phase 1) Add Zod schemas mirroring the existing plugin and template manifest JSON Schemas, the @standard-schema/spec dep that consumer code will use in phase 2, and a Zod→JSON Schema generator wired into build:package and generate:types. AJV continues to run in validate-manifest.ts; this is purely additive groundwork. Parity test uses fixture equivalence (Strategy B) — Zod 4's toJSONSchema emits per-type permission constraints as oneOf-of-discriminated-variants while the hand-written schema uses allOf+if/then over $defs/$ref, so byte parity is structurally infeasible. The test asserts AJV-with-legacy and Zod-with-new return matching accept/reject verdicts on the four core plugin manifests plus 5 synthetic plugin and 3 synthetic template fixtures (12 cases total). Build-pipeline byproducts of running pnpm build && pnpm docs:build cleanly are also captured: docs/static/schemas/plugin-manifest.schema.json loses a description field that copy-schemas.ts overwrote from the package-internal source (where the description was never present), and template/appkit.plugins.json gains origin enrichment on jobs.id and serving.name fields the parent PRD's enrichFieldsWithOrigin pass missed. Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu> * refactor: switch validation runtime to standard schema (phase 2) Replace AJV with Zod-via-Standard-Schema in validate-manifest.ts. The CLI validator now calls `~standard.validate` against the Zod schemas authored in phase 1; consumer code never imports zod directly. Cycle detection, dangling-reference checks, and the <PROFILE> placeholder constraint move from the standalone runSemanticValidation pass into Zod refinements co-located with the shape: - resourceRequirementSchema gains a superRefine running DFS over discovery.dependsOn — dangling refs emit at fields.<name>.discovery.dependsOn, cycles emit at the resource root with the existing 'a → b → c → a' chain. - discoveryDescriptorSchema.refine() enforces the <PROFILE> placeholder on cliCommand. - postScaffoldStepSchema.instruction tightens to z.string().min(1). origin-drift detection (validateDiscoveryOrigin) is dropped — origin becomes a transform in phase 3, eliminating the desync surface entirely. validate-manifest.ts shrinks from 498 to 177 lines: loadSchema, getPluginValidator, getTemplateValidator, the AJV compile cache, the JSON-pointer humanizer, the AJV error formatter, runSemanticValidation, validateDependsOn, validateDiscoveryProfile, validateDiscoveryOrigin, validatePostScaffold, and formatSemanticIssues all delete. Tests rewrite to drive validateManifest end-to-end and assert on the resulting SemanticIssue shape. validateManifest returns the original input object as `manifest` rather than result.output — Zod parsing is used purely as a verifier here so property order is preserved for round-trip writers like add-resource. Phase 3 will introduce the first real transform (origin), at which point output-vs-input distinction becomes intentional. ajv and ajv-formats remain in packages/shared/package.json for the phase-1 parity test (deletes in phase 5). Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu> * refactor: derive origin via zod transform (phase 3) templateFieldEntrySchema gains a .transform() that computes origin from localOnly/value/resolve and emits it on every parse. origin is accepted as optional input but always overwritten — drift-by-construction is now structurally impossible. The new sync.test.ts case verifies this: a field with value: "5432" and stale input origin: "user" parses to "static". enrichFieldsWithOrigin and its mutation pass delete from sync.ts. The template manifest is now built by parsing each field through templateFieldEntrySchema before serialization. Per-field parse (rather than whole-manifest parse) is chosen because Zod 4's strict-object parse reorders keys aggressively, churning resource and plugin entries; per-field parse leaves the surrounding structure in input order. Sync output is byte-identical to phase 2 (md5 verified). computeOrigin and Origin type delete from manifest-types.ts and sync.ts. The replacement, computeOriginFromField, is private to manifest.ts and invoked only by the transform — nothing else in the codebase needs origin computation now that validateDiscoveryOrigin (deleted in phase 2) is gone. generate-json-schema.ts passes io: "input" to z.toJSONSchema so the transform doesn't break schema emission. The published JSON Schema describes what plugin authors write (no origin slot), not the transformed output — exactly the right semantic for IDE intellisense and external validators. template/appkit.plugins.json regenerated by build pipeline (pre-existing drift, not introduced here). Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu> * feat: reshape discovery as discriminated union (phase 4) Replace the free-form discoveryDescriptorSchema with a discriminated union on `type`: - kind variant: { type: "kind", resourceKind, select?, display?, dependsOn?, shortcut? } resourceKind enum is the closed set of databricks resources AppKit owns command templates for: warehouse, genie_space, postgres_branch, postgres_database, volume. - cli variant: { type: "cli", cliCommand, selectField, displayField?, dependsOn?, shortcut? } preserves the existing free-form shape as an escape hatch for bespoke resources not yet in the kind enum. The <PROFILE> placeholder .refine() moves from the top-level descriptor to the cli variant only — kind has no cliCommand to validate. The cycle and dangling-reference DFS on resourceRequirementSchema reads field.discovery?.dependsOn generically and continues to work across both variants. A typed RESOURCE_KIND_COMMANDS map ships next to the schema. Each entry declares the CLI command template (with <PROFILE> placeholder + optional {<fieldName>} placeholders for dependsOn substitution) and an optional unwrap path for wrapped responses. Volume's catalog/schema parent context is documented in a code comment as a phase 6 MUST-rule concern, not a schema construct. The four core plugin manifests migrate to the kind variant: - analytics: { type: "kind", resourceKind: "warehouse" } - genie: { type: "kind", resourceKind: "genie_space" } - lakebase: branch → postgres_branch, database → postgres_database (dependsOn: "branch") - files: { type: "kind", resourceKind: "volume", select: "full_name" } Lakebase carries select: "name" (non-default for postgres_branch and postgres_database); files carries select: "full_name". Defaults are kind- specific identifiers and live in the command map / runner (out of scope). The Zod-derived ResourceRequirement is a discriminated union (per-type permission tightness baked in). Two consumer interface declarations in packages/shared/src/plugin.ts and packages/appkit/src/registry/types.ts previously did `interface ResourceRequirement extends GeneratedResourceRequirement` — TS interfaces cannot extend union types. Both flatten to structural interfaces with `permission: string`. This matches the previous consumer-facing shape (legacy generated permission was already loose string); per-variant tightness is enforced at schema parse time, where it belongs. add-resource.ts's literal entry construction casts to ResourceRequirement for the same reason. manifest-types.ts re-export source switches from the legacy plugin-manifest.generated to the canonical Zod schemas/manifest. The generated.ts file is now orphaned (no source imports it) and deletes in phase 5. The phase 1 parity test (json-schema-parity.test.ts) deletes — it asserted AJV-with-legacy-schema and Zod-with-new-schema return matching verdicts on the four core plugin manifests, but those manifests now use type: "kind" which the legacy schema doesn't understand. The test was the phase-1 transition gate; once the contract intentionally diverges it loses meaning. template/appkit.plugins.json and docs/docs/api/appkit/* re-emitted by the build pipeline. Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu> * chore: delete legacy json-schema pipeline (phase 5) Now that Zod is canonical and the validation runtime calls ~standard.validate, the legacy artifacts have nothing left to do. This phase deletes them and the build steps that produced them. Deleted: - packages/shared/src/schemas/plugin-manifest.generated.ts (orphaned since phase 4 switched manifest-types.ts re-exports to Zod). - packages/shared/src/schemas/plugin-manifest.schema.json, template-plugins.schema.json (legacy hand-written JSON Schema; Zod is now the source, JSON Schema is generated into docs/static/schemas by tools/generate-json-schema.ts). - tools/generate-schema-types.ts (JSON Schema → TS codegen, replaced by tools/generate-json-schema.ts going the other way). - docs/scripts/copy-schemas.ts (copied the legacy schemas to docs/static, now no-op). Dependencies removed from packages/shared: - ajv, ajv-formats — runtime validator gone in phase 2. - json-schema-to-typescript — codegen tool gone above. Build pipeline updated: - root package.json generate:types and packages/shared build:package scripts drop generate-schema-types.ts. - packages/shared/tsdown.config.ts drops the copy: block (the .json files no longer exist). - docs/package.json drops the copy-schemas script and removes it from the gen chain. - knip.json drops json-schema-to-typescript from ignoreDependencies. - .github/workflows/ci.yml `Check generated types are up to date` step drops the deleted plugin-manifest.generated.ts and adds docs/static/schemas/*.schema.json (now owned by generate-json-schema.ts). Source migrations to Zod: - schema-resources.ts: was reading plugin-manifest.schema.json at runtime to derive resource type options and per-type permissions. Now imports the per-type permission schemas and resourceTypeSchema from the Zod module and reads .options. No filesystem reads, no caching, no defensive null branches — values are module-level constants now. Public API preserved. - tools/generate-registry-types.ts: hidden consumer that also read the legacy JSON schema. Same migration. - packages/shared/src/cli/commands/plugin/manifest-types.ts: shrunk to a thin re-export shim of z.infer types and StandardSchemaV1. Type-level fix: - TemplatePlugin / TemplateResourceRequirement / TemplateFieldEntry / TemplatePluginsManifest type aliases switched to z.input instead of z.infer/z.output. The field-level origin transform makes origin REQUIRED on z.output, but consumer code (sync.ts) constructs template plugins without origin before writeManifest runs the transform at write time. z.input gives the pre-transform shape, matching the runtime invariant. Stale JSDoc references to GeneratedPluginManifest and plugin-manifest.generated.ts updated. The published JSON Schema URL is unchanged. Plugin authors' VSCode intellisense continues to work; the docs/static/schemas/*.json files are now byte-stable across runs (generated solely by the Zod-fed generate-json-schema.ts) and contain the new discriminated-union discovery shape. Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu> * feat: harden scaffolding directives (phase 6) Final phase of the manifest zod refactor. Three small, targeted changes to the scaffolding descriptor: 1. Rule items are constrained to ≤ 120 chars via z.string().max(120) on both rules.never[] and rules.must[]. The schema can't validate prose content but it can stop a directive from growing into a paragraph — enforces the "short directive" intent at the only boundary the schema can express. 2. TEMPLATE_SCAFFOLDING moves from sync.ts into the schema module. The constant lives next to the schemas it conforms to, with a `satisfies z.infer<typeof scaffoldingDescriptorSchema>` clause for compile-time validation against the input shape. sync.ts imports it. 3. New MUST rule directive describing volume parent-context handling: "When discovering volume resources, prompt the user for catalog and schema before listing volumes." The kind variant for `volume` doesn't model catalog/schema parents in the schema (per PRD design decision #7 — hierarchical context as MUST rule, not schema structure); this directive carries the requirement to LLM scaffolding agents instead. Tests added under "scaffolding rule item maxLength (Phase 6)": - never[]/must[] items exceeding 120 chars produce errors with the right path and message. - 120 chars exactly is accepted (≤ semantics). - A mixed-length array flags only the offending entry. - TEMPLATE_SCAFFOLDING parses cleanly against scaffoldingDescriptorSchema. - The synced template manifest carries the new volume MUST rule string. template/appkit.plugins.json regenerated by sync:template — the new rule string is now in scaffolding.rules.must. Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu> * fix: dedupe zod resolution for json-schema generator CI's `Check generated types are up to date` step was failing because two zod versions live in the workspace: - 4.1.13 hoisted at root (transitive via clean-app's eslint-plugin-react-hooks → zod-validation-error peer) - 4.3.6 in packages/shared (explicit dep) `tools/generate-json-schema.ts` imports zod from its own location, which resolves to root's 4.1.13. `packages/shared/src/schemas/manifest.ts` imports zod, resolving to shared's 4.3.6. The two runtimes operate on each other's schema objects, and the older zod's `toJSONSchema` doesn't extract all the constraints (pattern, minLength, propertyNames) that the newer zod baked into the schemas. CI's pnpm install resolves them consistently and emits the richer output, which then drifts from what's committed. Adding zod@4.3.6 as a root devDependency makes pnpm hoist the matching version to the top-level node_modules. The generator now resolves the same zod runtime as the schema module, and the JSON Schema output is byte-stable across local and CI. Regenerated docs/static/schemas/*.schema.json carry the now-emitted constraints (~135 minLength/pattern entries on plugin-manifest, similar on template-plugins). The constraints were always in the Zod schemas since phase 1 — they just weren't surviving the cross-version emit. Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu> * fix: address review findings — cli command syntax + lakebase parent + strict config Three real bugs flagged by the multi-model review of PR #261, fixed in this iteration. (CRITICAL cliCommand RCE hardening + HIGH z.lazy() perf deferred to follow-up PRs.) 1. RESOURCE_KIND_COMMANDS strings now match the real Databricks CLI (verified against v0.299.0 --help output): - `genie list` → `genie list-spaces` - `volumes list <catalog>.<schema>` → `volumes list {catalog} {schema}` (two separate positionals, prompted via the volume MUST rule) - `postgres list-branches` → `postgres list-branches {project}` with a project parent (covered by Fix 2 below) 2. Lakebase branch discovery is now actually runnable: - resourceKindSchema gains `postgres_project`. RESOURCE_KIND_COMMANDS gains the corresponding `databricks postgres list-projects` entry. - lakebase/manifest.json gains a new `project` field with `discovery: { type: "kind", resourceKind: "postgres_project", select: "name" }`. - The existing `branch` field's discovery adds `dependsOn: "project"`, so the parent project name flows into the branch listing command. 3. configSchemaPropertySchema and configSchemaSchema gain `.strict()`, so plugin config-schema typos no longer pass validation silently. `additionalProperties` (a standard JSON Schema keyword used by three core plugins — serving, vector-search, genie — inside nested property entries) is added explicitly as `z.union([z.boolean(), configSchemaPropertySchema]).optional()` so those manifests keep validating; this is a deliberate canonical addition, not a loosening of strict mode. Auto-regenerated by the build pipeline: - docs/static/schemas/{plugin-manifest,template-plugins}.schema.json - template/appkit.plugins.json Backpressure: typecheck=0, test=0 (108 files / 2136 tests), build=0, docs:build=0, knip=0, check:fix=0. Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu> * chore: mark auto-generated artifacts as linguist-generated GitHub collapses these in the diff view by default and excludes them from language stats. The files are emitted by the build pipeline and should not need reviewer attention. - docs/static/schemas/*.schema.json — emitted by tools/generate-json-schema.ts - template/appkit.plugins.json — emitted by pnpm sync:template - packages/appkit/src/registry/types.generated.ts — generate-registry-types.ts - packages/appkit/src/plugins/*-exports.generated.ts — generate-plugin-entries.ts - docs/docs/api/** — typedoc API reference - pnpm-lock.yaml — pnpm Reduces perceived PR size on this branch by ~10k lines (two regenerated JSON Schema files alone account for ~91% of insertions on PR #261). Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu> * fix: deny shell metacharacters on cli discovery descriptor The `cli` variant of discoveryDescriptor accepts a free-form Databricks CLI command supplied by the plugin author. With no further constraint beyond the existing `<PROFILE>` placeholder check, the shape is open to two unrelated foot-guns reviewers flagged: - output-shape brittleness: a plugin writes `selectField: ".id"` but the CLI returns a wrapped object (e.g. `{warehouses: [...]}`); jq fails silently at scaffold time - shell-injection-if-executed: when an executor lands and passes the string to a shell, statement separators / pipes / command substitution / redirects all become attack surface The `kind` variant addresses both for first-party plugins (AppKit owns the command and unwrap rules). The `cli` variant is the escape hatch for third-party plugins that need bespoke commands. Tighten it cheaply now, before anyone ships against the loose shape: - new SHELL_METACHAR_RE blocks `;`, `|`, `&`, backtick, `$`, newlines on both `cliCommand` and `shortcut`. Angle brackets are still permitted so `<PROFILE>` (and future `<…>` placeholders) work. - describes on cliCommand and the variant overall direct authors to use `kind` for first-party resources and call out that the cli shape is intentionally minimal and may tighten further. Not a security boundary on its own — executors must still spawn(argv) not shell-exec the string. argv-array form, denylist of shell operators in argv, and an output-shape contract are all separate decisions tied to the executor PR. Two new test cases cover the two refinements (cliCommand + shortcut). Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu> * feat: cap postScaffold instruction at 200 chars postScaffoldStepSchema.instruction was bounded below (.min(1)) but had no upper bound, while scaffolding.rules.must[] / never[] are capped at 120 chars per phase 6. Same intent applies to postScaffold instructions — they are checklist items, not prose — so add .max(200) with a parse-time error message. 200 (vs 120 for rules.must/never) allows short imperative sentences with placeholders; the longest existing core-plugin instruction is ~120 chars, so all current instructions fit with headroom. Regenerated docs/static/schemas/*.schema.json carry the new maxLength entry on the postScaffold step's instruction field. Two boundary tests added next to the existing "rejects empty postScaffold instruction" test (rejects 201, accepts exactly 200). Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu> * feat: replace postScaffold with plugin-level scaffolding.rules Delete postScaffold from canonical Zod schema and add plugin-level scaffolding.rules ({must?, should?, never?}, ≤120 chars each) gated by the substitutability principle. Adds parity should[] on template-level scaffoldingRules. Migrates analytics, files, genie, lakebase manifests off postScaffold per the A5 mapping table. xavier loop iteration 1 — phase 1 of plugin-manifest-refactor amendment. Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu> * feat: prune TEMPLATE_SCAFFOLDING.rules.must per substitutability gate Apply the A4 audit from the 2026-05-18 manifest amendment. All 4 prior must[] entries are substitutable (skill content, derivable from requiredByTemplate, derivable from field.env, or belongs on volume discovery descriptor — A6 candidate for a later phase). Net result: must=[], should=[] (parity), never keeps the 3 cross-cutting guardrails. Regression test pins the shape. xavier loop iteration 2 — phase 2 of plugin-manifest-refactor amendment. Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu> * feat: model kind discovery parents for transient query inputs Extend RESOURCE_KIND_COMMANDS value shape with parents?: readonly string[]. parents declares free-text prompts the runner must collect before invoking a kind's listing command — each entry substitutes the matching {name} placeholder in the command template. Unlike dependsOn (which references a sibling field), parents covers transient inputs not persisted as fields. Applies to volume.parents = ['catalog', 'schema'], replacing the prose rule deleted in the A4 audit. Regression tests pin volume's parents shape and confirm no other kind currently uses parents. A6 decisions (b) and (c) accept-as-prose — see ~/.xavier/tasks/plugin-manifest-refactor.md. xavier loop iteration 3 — phase 3 of plugin-manifest-refactor amendment. Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu> * chore(.claude): extend plugin commands with manifest v2.0 semantic checks audit-core-plugin: Step 3.6 — substitutability-gate patterns (permission duplication, existence tautology, inactionable --set, enum-or, length cap); discovery descriptor completeness; RESOURCE_KIND_COMMANDS.parents vs dependsOn consistency. All findings feed Category 1 (Manifest Design). review-core-plugin: Step 5.6 — same gate/discovery checks scoped to changed manifest files only. create-core-plugin: Step 4e.1 — post-scaffold enrichment pass that adds discovery descriptors to user-supplied fields and gates scaffolding.rules against the substitutability principle with 5 reject patterns + 3 legitimate categories. No changes to plugin-best-practices.md or plugin-review-guidance.md reference docs — additions are procedural workflow steps, not narrative rules. Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu> * refactor: sharpen template scaffolding rules for skill conflict detection The original three never entries surfaced false-positive conflicts with plugin setup instructions during integration testing against the databricks-apps skill (Track B PR databricks/databricks-agent-skills#79): - 'Modify files inside the template directory' caught every plugin must rule that edits scaffolded files (genie spaces config, lakebase migrations, analytics queries). The rule was either unreachable (if read as the SDK source-of-truth) or contradictory (if read as the scaffolded output, which the user owns post-init). Deleted. - 'Hardcode workspace-specific values in template files' conflated workspace IDs (which are correctly committed to bundle config) with credentials (which must not be). Replaced with a must/never pair that names the legitimate destinations (app.yaml, databricks.yml, .env) and the leak path (client bundle). - 'Skip resource configuration prompts' conflicted with the --set non-interactive path. Replaced with a should/never pair covering the actual decision-time failures: ask the user when uncertain; never guess when discovery returns zero or multiple options. Net: 0 must + 0 should + 3 never -> 1 must + 1 should + 2 never. All entries describe agent behaviors at decision points (substitutability gate passes), each under 120 chars, and the merged set is precedence- and phase-clean per the skill PR's protocol. Regression test in validate-manifest.test.ts updated to pin the new contents. Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu> * chore: address review comments (#4, #5, #9) - tools/generate-json-schema.ts, tools/generate-registry-types.ts: drop .ts extensions on relative imports — editors with TS LS configured without allowImportingTsExtensions flagged them as errors (per MarioCadenas on PR #261). tsx resolver accepts both forms; the rest of tools/ already imports without extensions. - lakebase/manifest.json: reword the migrations must rule. Previous text ('pnpm drizzle:migrate') referenced a script that doesn't exist in template/package.json AND a package manager (pnpm) the template doesn't use (template scripts use npm throughout). New text is ORM-agnostic: 'After init, run any database migrations for your chosen ORM before first request'. Preserves the don't-forget-migrations directive without presuming a tool the template doesn't ship. Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu> * fix: restore inherited-field descriptions on PluginManifest interface Phase 5 deleted plugin-manifest.generated.ts (the json-schema-to-typescript output that carried JSDoc converted from Zod .describe() strings). The Omit<GeneratedPluginManifest, ...>-based PluginManifest interface in packages/shared/src/plugin.ts then inherited fields from z.infer<typeof pluginManifestSchema> — a computed type with no JSDoc to render. TypeDoc walks the TS surface (not the JSON Schema), so 10 inherited field descriptions silently vanished from docs/docs/api/appkit/Interface.PluginManifest.md. Redeclare the affected fields locally on PluginManifest with JSDoc copied verbatim from the Zod .describe() text. Restores the rendered descriptions in TypeDoc output. Drift risk acknowledged: if Zod .describe() changes, the JSDoc here stays stale until manually synced. A proper follow-up would either (a) re-emit JSDoc'd .d.ts from the regenerated JSON Schema as a codegen artifact pointed at by TypeDoc, or (b) adopt a Zod-aware TypeDoc plugin. Both out of scope for this PR. Addresses PR #261 review comment from MarioCadenas on docs/docs/api/appkit/Interface.PluginManifest.md. Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu> * fix: collapse restored JSDoc to single line for diff-clean docs regen Multi-line JSDoc comments produced multi-line description text in the TypeDoc-rendered markdown, which git diffed against the original single-line descriptions in main. Functionally identical; just whitespace. Collapsing onSetupMessage, hidden, and stability JSDoc to single-line matches the original markdown formatting verbatim. Net change vs main on docs/docs/api/appkit/Interface.PluginManifest.md is now just the See link swap (deleted plugin-manifest.generated.ts -> Zod source of truth pointer). Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu> * fix: align TEMPLATE_SCAFFOLDING flag required to false; resync template CI sync:template diff check caught two source/template drifts on 4ae1741: 1. lakebase manifest had been moved must -> should in the source, but the committed template still carried the rule under must. Resyncing now that sync:template is invoked as part of pnpm build. 2. TEMPLATE_SCAFFOLDING declared --template-dir and --config-dir as required: true, but the committed template had them as required: false (per MarioCadenas on PR #261#6 — these flags aren't actually required by databricks apps init). Flipping the source constant to match; resync now produces consistent output. After this commit, source and template are in lockstep; CI sync:template diff check should pass. Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu> * chore: scrub PRD/phase leakage from source comments Code comments should describe implementation details, not project-management state. Stripped references to phase numbers (Phase 1-6), the substitutability gate amendment, A4/A5/A6 audit candidates, and Track B PR cross-links from: - packages/shared/src/schemas/manifest.ts (header comment) - packages/shared/src/plugin.ts (re-export comment) - packages/shared/src/cli/commands/plugin/manifest-types.ts (shim header) - packages/shared/src/cli/commands/plugin/schema-resources.ts (header) - packages/shared/src/cli/commands/plugin/validate/validate-manifest.test.ts (describe/it names + inline comments) - tools/generate-json-schema.ts (header — dropped the phase narrative entirely) - tools/generate-registry-types.ts (header) Where the original prose carried an implementation fact worth keeping (transform overwrites input, parents replaces dependsOn for volume, strict object rejects postScaffold, etc.), the fact is preserved phase-free. Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu> * fix: curate scaffolding.flags to match real CLI surface (#261) Replace stale --template-dir / --config-dir entries with the canonical databricks apps init flag set, derived from cmd/apps/init.go: --name (required, with kebab-case pattern), --template, --version, --features (with no-whitespace pattern), --set, --output-dir, --description, --run, --auto-approve, --profile Excluded by design: --branch (niche GitHub-template only, mutually exclusive with --version), --deploy (post-creation side effect to shared workspace), --warehouse-id (deprecated), --plugins (hidden alias for --features). Addresses PR feedback on template/appkit.plugins.json scaffolding.flags (thread r3282166233): --name is required by the scaffolder because {{.projectName}} populates package.json, databricks.yml, and .env. Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu> * docs: document v2.0 manifest contract (#370) * docs: document v2.0 manifest contract Add docs/docs/plugins/manifest.md covering the v2.0 plugin manifest: resources, kind/cli discovery descriptors, field dependencies, transient prompts via `parents`, and plugin-level scaffolding rules. Update templates.md to cover the v2.0 template manifest: computed `origin` field, scaffolding descriptor + rules with substitutability gate, and scaffolding.rules propagation from plugin manifests. Update custom-plugins.md to recommend the manifest.json import pattern matching the core plugins. Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu> * docs: clarify scaffolding requirement is CLI-enforced, not JSON-Schema Per Copilot review on #370: the published template-plugins.schema.json marks only `version` and `plugins` as top-level required. `scaffolding` is enforced via Zod superRefine (conditional on version=2.0), which JSON Schema cannot express. Reword both claims so readers validating against the published schema alone are not misled. Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu> * docs: align manifest v2.0 docs with merged PR #261 schema scaffolding.flags in templates.md still showed the pre-curation 3-flag placeholder set (--template-dir, --config-dir, --profile). Replace with three representative real flags plus a reference table covering all 10 canonical flags shipped by TEMPLATE_SCAFFOLDING. lakebase scaffolding rules example in manifest.md still used a `must` with Drizzle-specific wording, but the merged lakebase manifest.json has no `must` and uses generic ORM language under `should`. Sync the JSON example and the substitutability-gate prose example to match. Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu> --------- Signed-off-by: Atila Fassina <atila@fassina.eu> --------- Signed-off-by: Atila Fassina <atila@fassina.eu>
IamGalymzhan added a commit
that referenced
this pull request
Aug 11, 2026
Verified and fixed the findings from an independent code review: - #1 (correctness) expectStream dropped the wire `event:` name when the JSON payload carried its own `type` (spread ran after the assignment). Spread the payload first, then set `type = name ?? parsed.type`, so a frame like `event: error` + `data: {"type":"result"}` reports `error`. Regression test added. - #2 (contract) `@databricks/appkit/testing` eagerly loads vitest via fixtures even for `expectStream`, so vitest is a real requirement. Drop the "optional" peerDependenciesMeta and correct the docs sentence. - #6 (OBO fidelity) the fake `asUser` recorded `asUser: true` unconditionally. Enforce the real `Plugin.asUser` token precondition: a request without `x-forwarded-access-token` throws `missingToken` (missing user id throws too), and the resolved `userId` is recorded on each tool call. Tests now assert both directions (well-formed request vs token-less). - #3 (fidelity) attach() now mirrors AppKit core: registerPlugin plus registerToolProvider for real tool providers, without clobbering injected fakes. getPlugins()/getPluginNames()/hasPlugin() behave as in production. - #12 unknown-tool lookup used `tools[name] === undefined`, so a tool named "constructor"/"toString" hit Object.prototype. Use Object.hasOwn. - #5 drop data-less named SSE frames (real clients ignore them). - #7 re-export the PluginContext type from the testing barrel so MockPluginContext.ctx is nameable through the exports map. - #13 correct the docs: mock.telemetry captures the context's executeTool spans, not plugin-level spans (attachContext rebuilds the plugin's own telemetry). - #4 parseSSEResponse now delegates to the same parseSSEBody as expectStream — one parser, no divergence. All 3 analytics.integration call sites still pass. - #8 reformat template/server/example.test.ts with the template's Prettier so a scaffolded app's `npm run format` passes. - #10 fix the package-doc @example (agentsPlugin._handleStream does not exist). - #11 add kit tests that exercise attach() end-to-end (cache seed, isReady, registration, fake-not-clobbered). Build passes attw + publint; full appkit suite 3125 passed / 1 pre-existing skip. Signed-off-by: Galymzhan <zhangazy2004@gmail.com>
IamGalymzhan added a commit
that referenced
this pull request
Aug 20, 2026
* feat(appkit): make PluginContext telemetry injectable
The testing kit needs to construct a real PluginContext without a live
OpenTelemetry pipeline. Add an optional constructor dependency for the
telemetry provider, defaulting to the shared "plugin-context" provider so
the production path is unchanged. This is the single production edit
required to wrap the real class in tests rather than reimplementing it.
Signed-off-by: Galymzhan <zhangazy2004@gmail.com>
* feat(appkit): ship @databricks/appkit/testing and migrate first stub
Wire the testing kit as a published subpath and prove it against the first
of the two hand-rolled context stubs (the design gate):
- Add ./testing to both exports maps (dev + publishConfig) following the
./type-generator shape, add src/testing/index.ts to the tsdown entry, and
declare vitest as an optional peerDependency. Build passes attw + publint;
dist/testing/{index,mock-plugin-context,expect-stream,fixtures}.{js,d.ts}
are emitted and vitest stays external to the main entry.
- Migrate dispatch-tool-call.test.ts: replace (plugin as any).context =
{ executeTool } with mockPluginContext. executeTool is now the REAL method,
so the forwarded toolCallTimeoutMs is asserted through actual signal
composition, the on-behalf-of (asUser) path is verified, and a new test
proves the forwarded timeout actually aborts a slow toolkit tool end-to-end.
This is the primary win from the plan: executeTool's OBO and timeout paths
gain real assertions instead of a stub that proved nothing.
Signed-off-by: Galymzhan <zhangazy2004@gmail.com>
* test(appkit): migrate route-handler-errors context stub to mockPluginContext
Replace the second and final hand-rolled stub — (plugin as any).context =
{ addRoute } — with the real PluginContext from mockPluginContext. The kit's
route recorder captures raw handlers, so the alias assertion (both
/invocations and /responses mount the same handler reference) holds against
the real class, where forwardAsyncErrors wrapping would otherwise break
reference identity.
Both context stubs the plan identified are now migrated.
Signed-off-by: Galymzhan <zhangazy2004@gmail.com>
* docs(appkit): document the testing kit and ship a template example test
- Add docs/docs/development/testing.md covering mockPluginContext(),
expectStream(), and the fixture helpers, with a full end-to-end example.
Cross-links to local-development, custom-plugins, and execution-context.
- Add template/server/example.test.ts: a self-contained, plugin-agnostic
example that scaffolded apps ship with — it defines a tiny custom plugin
and exercises both mockPluginContext (route recording) and expectStream
(ordered event assertions), running with no workspace or network.
Ships the kit to users, satisfying the plan's acceptance criteria that a
docs page exists and the template carries at least one example test.
Signed-off-by: Galymzhan <zhangazy2004@gmail.com>
* docs(appkit): fix testing-kit examples to instantiate the plugin class
Validation by scaffolding a real app with `databricks apps init` surfaced
that the examples called the `analytics()`/`toPlugin()` factory and then
treated the result as a plugin instance — but a factory returns a
{ plugin, config, name } descriptor for createApp to construct, so
`.attachContext`/handler methods are absent.
Rewrite both the template example test and the docs "Full example" to
instantiate the plugin class directly (`new GreeterPlugin({})`), matching how
the migrated agents suites use the kit. The scaffolded app's `npm test` and
`tsc` both pass against the published `@databricks/appkit/testing` subpath
with no workspace or network.
Signed-off-by: Galymzhan <zhangazy2004@gmail.com>
* refactor(appkit): tighten FakeToolResponse so a missing value is a type error
Drop `undefined` from the static FakeToolValue union. `resolve()` treats an
undefined map entry as "unregistered tool" and throws, so allowing undefined
as a declared response made `{ query: undefined }` a confusing runtime error
instead of a compile error. A function returning undefined still works for the
rare "returns nothing" case. Add a test pinning that a null response is
returned as a value, not misread as a missing tool.
Signed-off-by: Galymzhan <zhangazy2004@gmail.com>
* refactor(appkit): make tools/test-helpers a shim over the shipped testing kit
The plan's step 5 was to MOVE the fixtures into the package, not copy them.
The shipped kit (src/testing/fixtures.ts) duplicated all 15 exports of
tools/test-helpers.ts, which would drift over time. Collapse the original
into a thin re-export of @databricks/appkit/testing so src/testing is the
single source of truth while the 18 existing @tools/test-helpers importers
keep working unchanged.
The re-exported mockServiceContext is now synchronous; every call site either
awaits it (no-op on a non-promise) or reads it through
Awaited<ReturnType<...>>, so all suites pass unchanged (full appkit suite:
3117 passed, 1 pre-existing skip).
Signed-off-by: Galymzhan <zhangazy2004@gmail.com>
* fix(appkit): normalize CRLF in expectStream SSE parsing; sharpen testing docs
Code review follow-ups:
- expectStream's parseSSEBody split frames on \n\n, so a spec-compliant SSE
stream delimited by \r\n\r\n (from a real server) collapsed into one event.
AppKit's own writer uses \n\n so existing tests were unaffected, but
expectStream is public API that accepts any Response. Normalize CRLF to LF
before splitting; add a CRLF regression test.
- Docs: instantiate the plugin CLASS in the attach() snippet (the factory
returns a descriptor, not an instance), and note that the cache attach()
seeds is a per-process singleton shared by tests within a file.
Signed-off-by: Galymzhan <zhangazy2004@gmail.com>
* fix(appkit): resolve repo-wide Biome error blocking CI
CI's "Lint & Type Check" job runs `pnpm run check` over the whole repo, so a
pre-existing lint error unrelated to this branch failed the build:
- remote-tunnel-controller.test.ts had two `afterEach` hooks in one describe
(lint/suspicious/noDuplicateTestHooks, error severity). Merge them into one —
behavior preserved (env reset + console-spy clear both still run after each
test). This file is byte-identical to main; the error predated the branch and
only surfaced because CI lints the entire tree.
Also drop two dead `biome-ignore lint/suspicious/noExplicitAny` suppressions in
the testing kit (fixtures.ts, expect-stream.test.ts): `noExplicitAny` is turned
off repo-wide in biome.json, so the comments had no effect (suppressions/unused
warnings). The invalid-source test now casts through `unknown as never`.
Signed-off-by: Galymzhan <zhangazy2004@gmail.com>
* fix(appkit): address cross-model review findings in the testing kit
Verified and fixed the findings from an independent code review:
- #1 (correctness) expectStream dropped the wire `event:` name when the JSON
payload carried its own `type` (spread ran after the assignment). Spread the
payload first, then set `type = name ?? parsed.type`, so a frame like
`event: error` + `data: {"type":"result"}` reports `error`. Regression test added.
- #2 (contract) `@databricks/appkit/testing` eagerly loads vitest via fixtures
even for `expectStream`, so vitest is a real requirement. Drop the "optional"
peerDependenciesMeta and correct the docs sentence.
- #6 (OBO fidelity) the fake `asUser` recorded `asUser: true` unconditionally.
Enforce the real `Plugin.asUser` token precondition: a request without
`x-forwarded-access-token` throws `missingToken` (missing user id throws too),
and the resolved `userId` is recorded on each tool call. Tests now assert both
directions (well-formed request vs token-less).
- #3 (fidelity) attach() now mirrors AppKit core: registerPlugin plus
registerToolProvider for real tool providers, without clobbering injected
fakes. getPlugins()/getPluginNames()/hasPlugin() behave as in production.
- #12 unknown-tool lookup used `tools[name] === undefined`, so a tool named
"constructor"/"toString" hit Object.prototype. Use Object.hasOwn.
- #5 drop data-less named SSE frames (real clients ignore them).
- #7 re-export the PluginContext type from the testing barrel so
MockPluginContext.ctx is nameable through the exports map.
- #13 correct the docs: mock.telemetry captures the context's executeTool spans,
not plugin-level spans (attachContext rebuilds the plugin's own telemetry).
- #4 parseSSEResponse now delegates to the same parseSSEBody as expectStream —
one parser, no divergence. All 3 analytics.integration call sites still pass.
- #8 reformat template/server/example.test.ts with the template's Prettier so a
scaffolded app's `npm run format` passes.
- #10 fix the package-doc @example (agentsPlugin._handleStream does not exist).
- #11 add kit tests that exercise attach() end-to-end (cache seed, isReady,
registration, fake-not-clobbered).
Build passes attw + publint; full appkit suite 3125 passed / 1 pre-existing skip.
Signed-off-by: Galymzhan <zhangazy2004@gmail.com>
* chore(appkit): drop knip vitest-ignore now that vitest is a real peer dep
With vitest declared as a (non-optional) peerDependency, knip recognizes it as
used, so the earlier ignoreDependencies entry is unnecessary. This reverts
knip.json to its original state.
Signed-off-by: Galymzhan <zhangazy2004@gmail.com>
* fix(appkit): make vitest a normal dependency, not a package-wide peer
A required peerDependency has no per-subpath scope: it applied to the whole
@databricks/appkit package, so every production consumer that never imports
the testing kit got an unsatisfied peer (npm 7+ auto-installs vitest into
their tree; pnpm warns) — a wider blast radius than the eager-import bug it
was meant to fix.
Follow appkit's own precedent instead: `vite` backs the ./type-generator
subpath as a normal `dependency`, installed for everyone but loaded only by
importers of that subpath. Do the same for `vitest` and ./testing. vitest is
referenced solely by dist/testing/fixtures.js, never by the main/plugin/core
entry, so a consumer importing createApp never loads it.
Verified end-to-end: scaffolded an app whose own vitest (4.1.9) differs in
major from appkit's dependency (3.2.4), forcing a nested second copy. The
testing kit's vi.fn()/vi.spyOn() mocks and expect(...).toHaveBeenCalled()
assertions work across the two instances (vi spies carry their own call
state), and npm install emits no peer-dep warning. Build passes attw + publint.
Also fold in the template example's Prettier formatting (template uses Prettier,
not Biome) so a scaffolded app's `npm run format` passes.
Signed-off-by: Galymzhan <zhangazy2004@gmail.com>
* refactor(appkit): rename mockPluginContext to createTestPluginContext
The helper builds the REAL PluginContext with faked edges — it does not mock
the context — so the name was misleading. Rename to createTestPluginContext
(and the MockPluginContext type to TestPluginContext), matching the
create*-for-tests convention, and rename the files to test-plugin-context.ts.
Pre-merge and unreleased, so no external consumers are affected.
Also finish the #13 doc-accuracy fix in the shipped JSDoc (not just the docs
page): the telemetry field comment now states it captures the context's spans
(executeTool), not plugin-internal spans — attachContext rebuilds the plugin's
this.telemetry from the real TelemetryManager. These comments ship in
dist/testing/*.d.ts, so IntelliSense previously showed the unqualified claim.
Build passes attw + publint; full appkit suite 3125 passed / 1 pre-existing skip.
Signed-off-by: Galymzhan <zhangazy2004@gmail.com>
* refactor(appkit): dedupe testing fixtures and tidy test-plugin-context
Behavior-preserving cleanups in the testing kit:
- createMockRequest reuses createMockWorkspaceClient() instead of an inline
copy of the same mock client (verified identical).
- createMockServiceContext / createMockUserContext / mockServiceContext inline
the createMockWorkspaceClient() call into the `||` fallback, so the mock
client is built only when the caller did not supply one.
- The fake asUser view spreads `...base` and overrides executeAgentTool rather
than re-declaring getAgentTools.
- expectStream's isSubsequence breaks once the expected sequence is fully
matched.
No semantic change; typecheck clean and all kit + migrated tests pass.
Signed-off-by: Galymzhan <zhangazy2004@gmail.com>
* fix(appkit): resolve third-review findings in the testing kit
- #1 (P1) The docs called vitest a peer dependency, but the manifest ships it
under `dependencies` (the decision we landed on, matching how appkit ships
`vite` for ./type-generator). Correct the docs to match: appkit installs
vitest for you, and it loads only when you import ./testing. Manifest and
docs now agree.
- #2 (P2) expectStream buffered the source eagerly with no bound, so a
non-terminating stream hung until the runner's own timeout. Add an optional
`{ timeout }` that fails fast with a clear, kit-specific error; document it
and cover both directions with tests.
- #3 (P2) The fake asUser replicates asUser's token precondition but not the
real dev-mode `DEV_OBO_FALLBACK_KEY` OTel marker (a module-private telemetry
detail). Narrow the docs and JSDoc to say so and point users at the recorded
asUser/userId fields instead of isDevOboFallback().
Build passes attw + publint; full appkit suite 3141 passed / 1 pre-existing skip.
Signed-off-by: Galymzhan <zhangazy2004@gmail.com>
* test(appkit): dogfood the testing kit on analytics and genie plugins
Exercise @databricks/appkit/testing against real core plugins to validate it
beyond the two agent proof sites and produce usage references:
- analytics.kit.test.ts: cross-plugin executeTool via createTestPluginContext —
OBO identity (asUser/userId), token-precondition rejection, and per-call
timeout abort. Needs only the kit (no workspace/ServiceContext).
- genie.kit.test.ts: drives the real _handleSendMessage SSE stream and asserts
event order with expectStream(...).toEmit(...).
Both add genuinely new coverage (streamed SSE order + OBO dispatch identity were
untested). Full appkit suite 3145 passed / 1 pre-existing skip.
Developer-experience notes (kit wins + friction, e.g. createMockResponse doesn't
compose with expectStream) captured in internal/ for the milestone review.
Signed-off-by: Galymzhan <zhangazy2004@gmail.com>
* refactor(appkit): address testing-kit review feedback
Resolve the eight review comments on the testing kit:
- createMockResponse now captures written SSE bytes and exposes
sseResponse(); expectStream reads a captured mock response directly, so
streaming-route tests no longer need a hand-rolled bridge.
- Ship vitest as an optional peer dependency (+ devDependency) instead of a
plain runtime dependency, keeping the test framework out of production
installs and deduping to the app's own copy. Ignore it in knip.
- Add an obo option to createMockRequest so on-behalf-of tests set the
forwarded identity headers with one flag.
- Add resetTestCache() to clear the shared cache singleton between tests.
- Use the documented attach() instead of an any-cast in the agents
dispatch tests.
- Drop the unused createMockServiceContext/createMockUserContext builders
from the public surface; keep the service-context builder internal.
- Pin the previously untested edges: the Object.hasOwn tool-lookup guard,
the dev-mode asUser branch, and parseSSEBody's non-object data values.
- Add useServiceContextMock() to register the mock lifecycle in one line,
returning a live accessor.
Dogfood the new helpers in the analytics, genie, and serving suites, and
document them in the testing guide.
Signed-off-by: Galymzhan <zhangazy2004@gmail.com>
* docs(appkit): move the testing guide under Plugins
The testing kit is entirely plugin-scoped (createTestPluginContext,
attach(plugin), plugin route/tool/SSE assertions), and the page's own
cross-links already pointed into plugins/. Move it next to custom-plugins
and fix the relative links. Keep the heading as 'Testing'; the Plugins
section supplies the context.
Signed-off-by: Galymzhan <zhangazy2004@gmail.com>
* test(appkit): fold dogfood tests into plugin suites
Address round-2 review: the kit should be the default way to test a
plugin, not a parallel '*.kit.test.ts' track.
- Fold the three cross-plugin executeTool OBO tests into analytics.test.ts
and delete analytics.kit.test.ts.
- Upgrade genie.test.ts's SSE test to assert event ORDER via
expectStream on genie's real event names (message_start, status,
message_result, query_result), replacing brittle write.mock.calls
substring checks, and delete genie.kit.test.ts.
- Trim the heavy comment narration from the folded-in tests.
- Re-export createTestPluginContext and expectStream from the test-helpers
shim.
- Finish the testing-guide move under plugins/ (sidebar position + links).
Signed-off-by: Galymzhan <zhangazy2004@gmail.com>
* test(appkit): restore toHaveLength(1) on the analytics OBO dispatch test
The dogfood fold trimmed expect(mock.toolCalls).toHaveLength(1), so a
double-dispatch would no longer fail the happy-path test — and it was
inconsistent with the token-less sibling that kept toHaveLength(0).
Restore it.
Signed-off-by: Galymzhan <zhangazy2004@gmail.com>
* test(appkit): re-assert genie SSE payloads after the expectStream swap
The toEmit swap pinned event order but dropped the payload values the old
substring checks covered (conversationId=new-conv-id, status=ASKING_AI),
which aren't asserted elsewhere. Restore them structurally via collect() +
toMatchObject — keeping the ordering guarantee without brittle substrings.
Signed-off-by: Galymzhan <zhangazy2004@gmail.com>
* fix(appkit): drop fabricated workspace-client fields from createMockRequest
createMockRequest returned userWorkspaceClient, serviceWorkspaceClient,
getWarehouseId and getWorkspaceId — fields no production code reads
(plugins resolve those through getWorkspaceClient()/getWarehouseId() from
src/context, which mockServiceContext stands in for). Publishing them via
@databricks/appkit/testing would make four inert fields a permanent public
promise.
The two warehouse cold-start tests (analytics + metric) overrode
mockReq.serviceWorkspaceClient.warehouses.get, which the route never reads
— so they passed on the default RUNNING client without exercising the
warehouse path at all. Route the warehouse client through
mockServiceContext (the real seam) so the tests are live, and drop the
'mock WorkspaceClient' claim from the testing guide.
Signed-off-by: Galymzhan <zhangazy2004@gmail.com>
* chore: drop the .claude ignores and restore the client lockfile
Both were swept into the oxlint merge from a dirty working tree and corrected
later on the branch; folding those corrections in here keeps them out of the
follow-up PR.
The knip `.claude/**` entry and the `**/.claude` ignorePatterns in oxfmt/oxlint
were never needed — nothing in the repo lints or formats that directory. The
`packages/appkit` vitest ignoreDependencies entry stays: vitest is a real
dependency of the testing entry.
apps/dev-playground/client/package-lock.json is restored to origin/main
byte-for-byte; npm had run in that directory and pruned its `extraneous: true`
entries.
Signed-off-by: Galymzhan <zhangazy2004@gmail.com>
* fix(appkit): lowercase mock request header keys, as Express does
`createMockRequest` stored header keys exactly as given while `header()`
lowercased the lookup, so a mixed-case override was unreachable:
createMockRequest({ obo: { userId: "alice" }, headers: { "X-Forwarded-User": "bob" } });
// header("x-forwarded-user") === "alice"
Both keys were kept — ["x-forwarded-access-token", "x-forwarded-user",
"X-Forwarded-User"] — and the lowercase one obo seeded still answered, which
contradicted the "an explicit override wins" contract documented right above it.
Keys are now lowercased on the way in, matching what Node's parser hands
Express. Thanks @pkosiec.
The existing override test passed because it used a lowercase key, so it is now
parametrised over both casings, and the case-insensitivity test additionally
pins that every stored key is lowercase. Reverting the fix fails both.
Signed-off-by: Galymzhan <zhangazy2004@gmail.com>
---------
Signed-off-by: Galymzhan <zhangazy2004@gmail.com>
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
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Serve static files from a UC Volume
Screen.Recording.2025-12-05.at.15.40.18.mov