Improve Lakebase skill: synced tables docs, apps decision gate, and connectivity

[pkosiec]

Summary

• Synced tables docs: Replace reverse-etl.md with comprehensive synced-tables.md covering CLI syntax, sync modes, prerequisites, constraints, and worked examples
• Apps decision gate: Add Step 0 to Development Workflow — presents Synced Tables vs Analytics comparison to the user before scaffolding any app that reads UC data
• Lakebase SKILL.md: Add psql workflow, GRANT commands for app SPs, expanded troubleshooting
• Connectivity: Add Python app connection pattern, DNS workaround
• Serverless migration: New skill (Add databricks-serverless-migration skill #55)

Screenshots

Case 1:

Case 2:

Sample prompts

I want to build a Databricks App for our inventory team to quickly search parts by part key and see supplier
  info, retail price, and availability. Data is in Unity Catalog (samples.tpch.part and samples.tpch.partsupp).

  Prefix all created resources with pktest3.

  Can you build this end to end?

I want to build a Databricks App that shows sales trends by region. I have    
order data in Unity Catalog                                                     
  (samples.tpch.orders and samples.tpch.customer).                              
                                                                                
  Show charts of revenue by nation and order priority breakdown.                
                                                                                
  Prefix all created resources with pktest4.

Test plan

• Agent presents comparison table for UC-reading apps and waits for user choice
• Agent skips gate for non-UC apps (CRUD, Genie, Model Serving)
• python3 scripts/skills.py validate passes

[jamesbroadhead]

Stacked a follow-up at #59 — adds the SP-role-creation block for AppKit Lakebase apps (the case where the SP fails with password authentication failed on first deploy because no Postgres role exists yet). Branched off your branch so the diff stays focused; happy to fold it in if you'd rather have it as part of this PR.

[keugenek]

please fix 'synced tables' term to be more specific before merging

[keugenek]

@pkosiec is it clear enough for AI when you say 'synced tables' - maybe there is more desriptive term which is more specific?

[pkosiec]

unfortunately I don't think so, it's the official name: https://docs.databricks.com/aws/en/oltp/instances/sync-data/sync-table - I will use "Lakebase synced tables" then.

From my testing an agent wasn't confused with the term but I agree, let's make it more clear 👍

[keugenek]

these timings instead better be derived from git not from the contents

[pkosiec]

Good point. Looked into deriving timestamps from git — the issue is that git log only knows about committed state, so you'd need a two-step workflow: commit skill changes first, then run the script to regenerate manifest.json, then commit again (or amend). That's friction for every skill edit.

Current st_mtime approach isn't perfect (varies across machines after clone/rebase), but validate already strips updated_at from comparison, so they're effectively cosmetic. The trade-off doesn't seem worth the workflow change right now 🤔

[keugenek]

Nitpick Review — databricks/databricks-agent-skills PR #56

PR: #56 — Improve Lakebase skill: synced tables docs, apps decision gate, and connectivity
Author: pkosiec (Pawel Kosiec)
Base → Head: main ← pkosiec/lakebase-synced-tables
Diff: 10 files, +463 / −321
Verdict: ❌ Changes requested — three blocking regressions in build/manifest tooling, plus design and content issues that should be addressed before merge.

Files reviewed (10)

File	Type
manifest.json	config
scripts/skills.py	python
skills/databricks-apps/SKILL.md	docs
skills/databricks-apps/references/appkit/jobs.md	deleted (-141)
skills/databricks-apps/references/appkit/lakebase.md	docs
skills/databricks-apps/references/appkit/overview.md	docs
skills/databricks-lakebase/SKILL.md	docs (major rewrite)
skills/databricks-lakebase/references/connectivity.md	docs
skills/databricks-lakebase/references/reverse-etl.md	deleted (-103)
skills/databricks-lakebase/references/synced-tables.md	new (+214)

Reviewed by three parallel agents: Security, Correctness, Design.

---

🔴 Must Fix (CRITICAL / HIGH)

1. scripts/skills.py — iter_skill_files filter was removed without replacement

• Severity: HIGH (Correctness CRITICAL · Design MEDIUM · Security LOW — all three reviewers flagged this)
• Where: scripts/skills.py:98 (get_skill_updated_at) and :183 (generate_manifest)
• Issue: The deleted helper filtered out dot-prefixed paths (.DS_Store, .git), __pycache__/, and *.pyc. The replacement uses bare rglob("*") at both call sites. On any macOS dev machine, Finder will drop .DS_Store into skill directories and they will (a) get added to the manifest's files list, (b) bump updated_at non-deterministically, and (c) potentially expose accidentally-committed dotfiles (e.g. .env) in the published manifest.
• Fix: Restore the filter inline or via a helper:

  if any(part.startswith('.') for part in f.relative_to(skill_dir).parts):
      continue
  if f.suffix == '.pyc' or '__pycache__' in f.parts:
      continue

2. manifest.json — updated_at timestamps rolled backward across multiple skills

• Severity: HIGH (Correctness HIGH · Design HIGH)
• Where: Top-level and at least 6 skills (databricks-apps 2026-04-28 → 2026-04-27; databricks-core, databricks-dabs, databricks-jobs, databricks-model-serving, databricks-pipelines all 2026-04-28 → 2026-04-23)
• Issue: The manifest appears to have been regenerated against a stale checkout, so skills not changed in this PR now claim to be older than they were on main. CI's validate mode strips updated_at before comparison, so it won't catch this. Any consumer that uses these timestamps for cache-busting will incorrectly believe the skills are stale.
• Fix: Regenerate manifest.json against the actual branch HEAD (after fixing finding Refactor: move skills to skills/ directory #1). Confirm no unrelated updated_at values move backward versus main.

3. scripts/skills.py:47–50 — databricks-proto-first entry added but skill directory does not exist

• Severity: HIGH (Correctness HIGH · Design MEDIUM)
• Where: SKILL_METADATA dict
• Issue: New entry "databricks-proto-first" has no matching skills/databricks-proto-first/ directory. The generator only validates the on-disk → metadata direction (raises if a directory has no metadata) but not metadata → on-disk, so the dead entry is silently dropped today. This will cause confusion when the skill is added (silent conflict or surprising activation) and is misleading to future contributors.
• Fix: Either commit the skills/databricks-proto-first/ directory in this PR or remove the dict entry. Add an assertion that every SKILL_METADATA key resolves to an existing directory.

4. skills/databricks-apps/SKILL.md Decision Gate placement

• Severity: HIGH (Design)
• Where: SKILL.md:60–76
• Issue: The new Decision Gate sits inside the "Development Workflow" section, after the "Required Reading by Phase" table and "Generic Guidelines". An LLM that reads the skill top-down will hit scaffolding instructions (~line 20) before encountering the gate (line 60), defeating the gate's purpose.
• Fix: Promote the Decision Gate to a top-level section directly after the skill description, or at minimum add an inline **⚠️ Decision Gate required — see below** to the "Scaffolding" row of the Required Reading table.

5. skills/databricks-apps/references/appkit/lakebase.md ↔ synced-tables.md content duplicated

• Severity: HIGH (Design)
• Where: appkit/lakebase.md:180–193 vs databricks-lakebase/references/synced-tables.md (whole file)
• Issue: The "Use synced tables when / Do NOT use" guidance is duplicated in two skills and will drift. Single source of truth should be the lakebase skill.
• Fix: In appkit/lakebase.md, replace the duplicated lists with a one-line pointer ("see databricks-lakebase/references/synced-tables.md for full guidance") and keep only the AppKit-integration-specific notes.

---

🟡 Should Fix (MEDIUM)

• [MEDIUM/security] connectivity.md:155–159 — resolve_host() passes raw dig +short first line to hostaddr without IP validation; CNAME, warning text, or empty output silently flows through. Fix: validate with socket.inet_pton(...); raise ValueError otherwise.
• [MEDIUM/security] connectivity.md:84 — Pattern 3 promotes export LAKEBASE_PG_URL="postgresql://user:password@..." (creds in shell history/process listing) without a clear "local-dev only / use .env excluded from VCS / don't put real prod creds here" callout. Fix: add an explicit warning + .env/pg_service.conf alternative.
• [MEDIUM/correctness] databricks-lakebase/SKILL.md:233 — Bash snippet parses CLI JSON via ['status']['hosts']['host'] while Python examples (connectivity.md:22, 33) use the SDK attribute path. The CLI JSON shape isn't verified in the docs; if hosts is an array or named differently, the snippet fails silently. Fix: verify against databricks postgres get-endpoint -o json and add a comment showing the expected shape.
• [MEDIUM/correctness] databricks-apps/SKILL.md "When to Use What" routing table — has rows for Analytics, Lakebase CRUD, Genie, Model Serving but no row for Synced Tables / low-latency UC reads, which the new Decision Gate routes to. Fix: add a bullet pointing low-latency UC reads at --features lakebase + databricks-lakebase skill.
• [MEDIUM/design] appkit/lakebase.md:228–230 — two consecutive callout blocks repeat the same instruction ("use databricks postgres create-synced-table, see lakebase skill, grant SP"). Fix: delete one.
• [MEDIUM/design] appkit/overview.md:11–16 — two --features lakebase rows (Synced Tables vs Lakebase OLTP) with identical Init command but different --set requirements. An LLM cannot disambiguate them from the table alone. Fix: include the full init command (with required --set flags) in each row, or annotate that OLTP requires extra flags.

---

🟢 Consider (LOW / NIT)

• [LOW/security] appkit/lakebase.md:193 — UC FGAC bypass for synced tables is buried in a "Do NOT use" bullet list. Promote to a dedicated > **Security note** callout in the "How It Works" section.
• [LOW/security] databricks-lakebase/SKILL.md:241–243 — The example GRANT SELECT ON ALL TABLES IN SCHEMA public … ALTER DEFAULT PRIVILEGES grants the SP read on every current and future object in public, which is broader than most users likely intend. Recommend a dedicated schema for synced tables.
• [LOW/correctness] appkit/lakebase.md:463, :484 — references the lakebase SKILL.md "Grant app SP access to synced tables" section, but that text isn't an ##/### heading — it's a bold label inside "Other Workflows". Anchor links won't resolve. Fix: promote to a real heading.
• [LOW/design] databricks-lakebase/SKILL.md:3 — frontmatter description is ~253 chars, ~4× the length of sibling skills (databricks-jobs ~85, databricks-pipelines ~90). Trim to a single sentence; routing models don't benefit from prose.
• [LOW/design] databricks-apps/SKILL.md:72 — introduces the novel pattern *Agent notes (do not show to user):* with no precedent or convention defined elsewhere. Establish the convention (scope, who honors it) or drop it.
• [NIT/design] Deletion of references/appkit/jobs.md (141 lines) leaves no in-skill answer to "trigger/monitor a Lakeflow job from my app". Add a 10-line stub or a one-line pointer in overview.md to npx @databricks/appkit docs Jobs plugin + the databricks-jobs skill.
• [NIT/design] databricks-lakebase/SKILL.md:28 says "Synced tables (reverse ETL)" while synced-tables.md:5 correctly says "Previously known as Reverse ETL." Use the deprecated-name framing in both places, or drop the parenthetical from SKILL.md.

---

What was checked and found clean

• No hardcoded credentials, no eval/exec/shell=True, no yaml.load without SafeLoader, no pickle, no sslmode=disable patterns.
• subprocess.run calls in scripts/skills.py use list form and don't incorporate user input.
• All files listed in manifest.json exist on disk; no phantom entries for deleted jobs.md / reverse-etl.md.
• No remaining markdown links to the deleted reference files.
• Cross-skill relative path ../../../databricks-lakebase/references/synced-tables.md (appkit/lakebase.md:228) resolves on disk.
• SQL in synced-tables.md examples is valid Postgres syntax; CLI commands use the new databricks postgres create-synced-table (consistent with the Autoscaling context).
• Autoscaling Max−Min ≤ 16 CU constraint, removed from SKILL.md, is preserved in references/computes-and-scaling.md.
• Decision Gate's A/B conditions are mutually exclusive; the "skip for pure CRUD" escape hatch is logically sound.

---

Reviewed in parallel by Security · Correctness · Design agents (Sonnet 4.6) on 2026-04-30.
Diff: 1103 lines, 55KB, against origin/main.