feat: add apps database sync shortcuts for Base-to-database import - #2251
Conversation
Add Base-to-database sync shortcuts for preview, create, list, get, enable, disable, update, and delete flows. Cover OpenAPI request contracts, typed sync error classification, dry-run E2E coverage, and lark-apps skill guidance. Co-authored-by: TRAE CLI <noreply@bytedance.com>
The enable/disable/delete/update sync commands placed task_id (and update's config) in query params, but the OpenAPI contract binds these fields via api.json (request body). BOE testing returned "field validation failed" (99992402) because the body was empty. Move task_id to the request body for enable/disable/delete, and move both task_id and config to the body for update. Dry-run previews now render these under body, and unit tests pin the body binding so a regression to query params fails.
The delete command issued an HTTP DELETE to db/sync_del, but the action-style endpoint is registered as POST (like sync_create and sync_disable). The method mismatch made the gateway return a plaintext 404, surfacing as "API returned a non-object JSON response". Switch the request and dry-run preview to POST, and pin the method in the delete unit tests so a regression to DELETE fails.
…optional contract
The +db-sync-create and +db-sync-update endpoints read env from the request body (peer of config/preview/task_id), not the query string. Placing env in query params left the body env empty, so the server treated every request as online and rejected DDL operations (code 500002776: forbid ddl/dcl operation in online env), making it impossible to create/update sync tasks against a dev environment. Move env into the request body via a new dbEnvBody helper that mirrors dbEnvParams' omit-empty contract, so unset env still lets the server auto-select the branch. Pin the contract in unit and e2e dry-run tests by asserting body.env and that env is absent from query params.
The enable/disable/delete dry-run e2e still asserted the pre-migration wire shape: delete on DELETE and task_id in query params. The shortcuts now POST these actions with task_id in the request body (commits moving task_id and the delete verb), so the stale assertions failed against a current binary. Assert POST + body.task_id and that task_id is absent from query params, pinning the same body-over-query contract the env fix established.
Refine +db-sync-create/update validation, error hints, and docs so AI agents recover from common Base-to-database sync failures without guessing: - source.table.name: document that a user-named table must be set, name takes precedence over the base_url ?table= token; fix test fixtures that used a fictional source.table.url instead of source.base_url. - Preflight source table locate: reject create locally when base_url has no ?table= and source.table.name is empty, pointing at base +table-list. - Online DDL ban: attach a precise hint for code 500002776 + subcode k_dl_4000001 telling multi-env apps to create tables on --environment dev. - Missing record-id column: extend the 500002783 hint to add a unique text column via +db-execute before retrying. - Optional field_maps on create: allow omitted or empty field_maps so the server auto-matches and creates the task; keep update requiring an enabled mapping and still reject an all-disabled array. - Environment default: db-sync commands use online when --environment is omitted; align help text, comments, and skill docs.
📝 WalkthroughWalkthroughAdded seven Apps database-sync shortcuts for creation, inspection, listing, updates, lifecycle operations, and deletion. Added shared configuration validation, API error guidance, Spark error metadata, end-to-end tests, registration, and workflow documentation. ChangesDatabase synchronization
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant AppsDBSyncCreate
participant DataSyncAPI
participant Output
CLI->>AppsDBSyncCreate: validate config and flags
AppsDBSyncCreate->>DataSyncAPI: submit preview or create request
DataSyncAPI-->>AppsDBSyncCreate: return resolved config or task response
AppsDBSyncCreate->>Output: write config or print task summary
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (5)
shortcuts/apps/db_common.go (1)
554-576: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winClassify a non-object
field_mapselement as invalid, not disabled.The loop skips any element that is not a
map[string]interface{}. An array such as["a", "b"]therefore returnsdbSyncFieldMapsDisabled, and the caller reports "has mappings but all are disabled". That message does not describe the fault. ReturndbSyncFieldMapsInvalidfor a non-object element.♻️ Proposed refactor
for _, item := range items { mapping, ok := item.(map[string]interface{}) if !ok { - continue + return dbSyncFieldMapsInvalid }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@shortcuts/apps/db_common.go` around lines 554 - 576, Update classifyFieldMaps so any non-map element encountered in the items loop immediately returns dbSyncFieldMapsInvalid; preserve the existing handling for valid mappings and disabled entries.shortcuts/apps/apps_db_sync_list_get_test.go (1)
23-33: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse
dbSyncFirstDryRunAPIinstead of indexingenv.API[0].The dry-run tests index
env.API[0]directly at Lines 27, 53, and 181. If the envelope carries no API entry, the test panics with an index-out-of-range error instead of reporting the missing call.dbSyncFirstDryRunAPIinshortcuts/apps/apps_db_sync_create_update_test.goalready checks the count and reports the raw stdout. Both files are in packageapps, so the helper is reachable.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@shortcuts/apps/apps_db_sync_list_get_test.go` around lines 23 - 33, Update the dry-run tests in apps_db_sync_list_get_test.go to use the existing dbSyncFirstDryRunAPI helper instead of indexing env.API[0] at the referenced assertions. Preserve the current method, URL, and parameter checks while relying on the helper’s API-count validation and stdout diagnostics.shortcuts/apps/db_common_test.go (1)
303-322: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert category and subtype in the shared validation helper.
The helper asserts
Paramand message text only. The coding guidelines require error-path tests to assert typed metadata (category,subtype,param) and to verify cause preservation. Adderrs.ProblemOfassertions forCategoryandSubtype. Add a cause assertion for the malformed-JSON case at Line 127, whereparseDBSyncConfigFlagattachesWithCause(err).💚 Proposed addition
if validationErr.Param != "--config" { t.Fatalf("Param = %q, want --config", validationErr.Param) } + p, ok := errs.ProblemOf(err) + if !ok || p.Category != errs.CategoryValidation || p.Subtype != errs.SubtypeInvalidArgument { + t.Fatalf("problem = %+v, want validation/invalid_argument", p) + }As per coding guidelines: "Error-path tests must assert typed metadata through
errs.ProblemOf(category,subtype, andparam) and verify cause preservation rather than relying only on message substrings."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@shortcuts/apps/db_common_test.go` around lines 303 - 322, Update assertDBSyncConfigValidation to inspect the typed problem via errs.ProblemOf and assert the expected Category, Subtype, and Param instead of checking Param directly. In the malformed-JSON test around parseDBSyncConfigFlag, assert that the resulting validation error preserves the original parsing error as its cause.Source: Coding guidelines
shortcuts/apps/apps_db_sync_operate_test.go (1)
142-150: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winError-path tests assert message substrings instead of typed metadata. Both tests match on
err.Error()text, so a wording change breaks them and a category or subtype regression does not. The package already provides typed helpers (requireAppsProblem,requireAppsValidationProblem,errs.ProblemOf).
shortcuts/apps/apps_db_sync_operate_test.go#L142-L150: replace the"requires confirmation"substring check withrequireAppsProblem(t, err, errs.CategoryConfirmation)and anerrs.SubtypeConfirmationRequiredassertion.shortcuts/apps/apps_db_sync_list_get_test.go#L86-L97: assert the typed problem category and subtype plus the failing parameter for the invalid--modevalue, and keep the enum text check as a secondary assertion.As per coding guidelines: "Error-path tests must assert typed metadata through
errs.ProblemOf(category,subtype, andparam) and verify cause preservation rather than relying only on message substrings."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@shortcuts/apps/apps_db_sync_operate_test.go` around lines 142 - 150, Update TestAppsDBSyncDeleteRequiresConfirmation in shortcuts/apps/apps_db_sync_operate_test.go:142-150 to validate the typed confirmation problem with requireAppsProblem and errs.CategoryConfirmation, then assert errs.SubtypeConfirmationRequired instead of matching err.Error(). Update the invalid --mode test in shortcuts/apps/apps_db_sync_list_get_test.go:86-97 to assert category, subtype, and failing parameter through the existing typed helpers/errs.ProblemOf, preserve the enum-text check as a secondary assertion, and verify cause preservation as required by the error-path testing guidelines.Source: Coding guidelines
shortcuts/apps/apps_db_sync_create.go (1)
123-138: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse
ResolvePathto report the saved output path.
FileIO.Savereturnsfileio.SaveResult, whose current API only exposes written bytes throughSize(), not the destination path. Report the canonical path after saving, as the interface comment suggests withFileIO.ResolvePath. Drop the deadsavedassignment if the result stays unused.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@shortcuts/apps/apps_db_sync_create.go` around lines 123 - 138, After the successful FileIO.Save call in the output handling flow, call rctx.FileIO().ResolvePath with output and report the resolved canonical path in the preview output. Remove the unused saved variable and retain the existing validation error handling for Save failures.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@shortcuts/apps/apps_db_sync_get.go`:
- Around line 88-93: Update the batch rendering in the surrounding function to
pass schema_only through the existing dashIfEmpty helper instead of fmt.Sprint,
and revise dbSyncSummary to render map entries as deterministic key=value pairs
using sorted keys; add the required sort import and preserve the existing
summary behavior for empty or non-map values.
In `@shortcuts/apps/db_common.go`:
- Around line 449-467: Update the field_maps validation around classifyFieldMaps
so dbSyncFieldMapsInvalid is rejected regardless of requireFieldMaps, including
preview calls to parseDBSyncConfigFlag. Keep the existing disabled and absent
mapping policy checks conditional on requireFieldMaps, while ensuring malformed
values such as objects or null are reported immediately.
In `@skills/lark-apps/references/lark-apps-db.md`:
- Line 40: Update the high-risk operations guidance at
skills/lark-apps/references/lark-apps-db.md:40-40 to require --yes for
+db-sync-create only when it performs creation, excluding --preview; likewise
update the agent rule at skills/lark-apps/references/lark-apps-db.md:275-275 to
explicitly preserve this preview exception.
In `@tests/cli_e2e/apps/apps_db_sync_dryrun_test.go`:
- Around line 197-224: Strengthen the validation tests in
TestAppsDBSyncCreate...RequiresSourceTable and the preceding singular-field-map
test to assert exit code 2, empty stdout, and the structured stderr envelope.
Validate category and subtype via errs.ProblemOf, extract Param with errors.As
into *errs.ValidationError, and assert the original cause is preserved. Avoid
accepting diagnostics from either stream or relying only on message fragments.
---
Nitpick comments:
In `@shortcuts/apps/apps_db_sync_create.go`:
- Around line 123-138: After the successful FileIO.Save call in the output
handling flow, call rctx.FileIO().ResolvePath with output and report the
resolved canonical path in the preview output. Remove the unused saved variable
and retain the existing validation error handling for Save failures.
In `@shortcuts/apps/apps_db_sync_list_get_test.go`:
- Around line 23-33: Update the dry-run tests in apps_db_sync_list_get_test.go
to use the existing dbSyncFirstDryRunAPI helper instead of indexing env.API[0]
at the referenced assertions. Preserve the current method, URL, and parameter
checks while relying on the helper’s API-count validation and stdout
diagnostics.
In `@shortcuts/apps/apps_db_sync_operate_test.go`:
- Around line 142-150: Update TestAppsDBSyncDeleteRequiresConfirmation in
shortcuts/apps/apps_db_sync_operate_test.go:142-150 to validate the typed
confirmation problem with requireAppsProblem and errs.CategoryConfirmation, then
assert errs.SubtypeConfirmationRequired instead of matching err.Error(). Update
the invalid --mode test in shortcuts/apps/apps_db_sync_list_get_test.go:86-97 to
assert category, subtype, and failing parameter through the existing typed
helpers/errs.ProblemOf, preserve the enum-text check as a secondary assertion,
and verify cause preservation as required by the error-path testing guidelines.
In `@shortcuts/apps/db_common_test.go`:
- Around line 303-322: Update assertDBSyncConfigValidation to inspect the typed
problem via errs.ProblemOf and assert the expected Category, Subtype, and Param
instead of checking Param directly. In the malformed-JSON test around
parseDBSyncConfigFlag, assert that the resulting validation error preserves the
original parsing error as its cause.
In `@shortcuts/apps/db_common.go`:
- Around line 554-576: Update classifyFieldMaps so any non-map element
encountered in the items loop immediately returns dbSyncFieldMapsInvalid;
preserve the existing handling for valid mappings and disabled entries.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: cb387b2c-8b7c-4298-834f-9754c16b6479
📒 Files selected for processing (17)
internal/errclass/codemeta_spark.gointernal/errclass/codemeta_spark_test.goshortcuts/apps/apps_db_sync_create.goshortcuts/apps/apps_db_sync_create_update_test.goshortcuts/apps/apps_db_sync_get.goshortcuts/apps/apps_db_sync_list.goshortcuts/apps/apps_db_sync_list_get_test.goshortcuts/apps/apps_db_sync_operate.goshortcuts/apps/apps_db_sync_operate_test.goshortcuts/apps/apps_db_sync_update.goshortcuts/apps/db_common.goshortcuts/apps/db_common_test.goshortcuts/apps/shortcuts.goshortcuts/apps/shortcuts_test.goskills/lark-apps/SKILL.mdskills/lark-apps/references/lark-apps-db.mdtests/cli_e2e/apps/apps_db_sync_dryrun_test.go
The backend moved the seven db-sync error codes from the 5000027xx server-error range to the 4000024xx client-input range to reflect that they are client-input errors. Mirror the new codes in the CLI so error classification and recovery hints keep matching: - 500002783 -> 400002477 (mapping invalid) - 500002784 -> 400002478 (target schema mismatch) - 500002785 -> 400002479 (operation not allowed) - 500002786 -> 400002480 (task not found) - 500002787 -> 400002481 (invalid task id) - 500002788 -> 400002482 (source table not found) - 500002789 -> 400002483 (target table not found) Category, subtype, hint text, and behavior are unchanged; 500002776 (online DDL ban) is untouched.
…i-cli # Conflicts: # internal/errclass/codemeta_spark.go # internal/errclass/codemeta_spark_test.go # shortcuts/apps/shortcuts_test.go
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@shortcuts/apps/apps_db_sync_list_get_test.go`:
- Around line 91-96: Update the invalid --mode test assertions to inspect the
typed error via errs.ProblemOf, verifying the expected validation category and
subtype instead of relying only on message text. Use errors.As to extract
*errs.ValidationError and assert its Param is "--mode"; if the validation path
wraps a cause, also verify that cause is preserved while retaining the existing
allowed-value checks as appropriate.
In `@skills/lark-apps/references/lark-apps-db.md`:
- Line 206: Update the “失败恢复” guidance to branch on task mode: recommend
+db-sync-update only when mode=streaming, while for mode=batch instruct the user
to inspect the result and create a new task when needed. Keep the existing log
inspection and authorization guidance, but ensure the fixed command chain does
not suggest +db-sync-update for batch tasks.
- Around line 133-134: Update all sync preview, create, recovery, and update
command examples to pass the same explicit task environment consistently,
including the get commands and commands currently defaulting to online. Preserve
the existing environment value used by each task, and ensure every command in
the documented workflow uses that value rather than omitting --environment.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 2d14581d-5252-4281-851c-b5fd8b973c26
📒 Files selected for processing (17)
internal/errclass/codemeta_spark.gointernal/errclass/codemeta_spark_test.goshortcuts/apps/apps_db_sync_create.goshortcuts/apps/apps_db_sync_create_update_test.goshortcuts/apps/apps_db_sync_get.goshortcuts/apps/apps_db_sync_list.goshortcuts/apps/apps_db_sync_list_get_test.goshortcuts/apps/apps_db_sync_operate.goshortcuts/apps/apps_db_sync_operate_test.goshortcuts/apps/apps_db_sync_update.goshortcuts/apps/db_common.goshortcuts/apps/db_common_test.goshortcuts/apps/shortcuts.goshortcuts/apps/shortcuts_test.goskills/lark-apps/SKILL.mdskills/lark-apps/references/lark-apps-db.mdtests/cli_e2e/apps/apps_db_sync_dryrun_test.go
🚧 Files skipped from review as they are similar to previous changes (12)
- internal/errclass/codemeta_spark.go
- shortcuts/apps/shortcuts.go
- shortcuts/apps/apps_db_sync_list.go
- shortcuts/apps/apps_db_sync_update.go
- shortcuts/apps/db_common.go
- shortcuts/apps/apps_db_sync_create.go
- tests/cli_e2e/apps/apps_db_sync_dryrun_test.go
- shortcuts/apps/apps_db_sync_create_update_test.go
- shortcuts/apps/apps_db_sync_get.go
- shortcuts/apps/db_common_test.go
- shortcuts/apps/apps_db_sync_operate.go
- shortcuts/apps/apps_db_sync_operate_test.go
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@f7048884ba6c8ae4ffa199077622d5a51165d43d🧩 Skill updatenpx skills add larksuite/cli#feat/data-sync-openapi-cli -y -g |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2251 +/- ##
==========================================
+ Coverage 76.36% 76.45% +0.09%
==========================================
Files 1011 1018 +7
Lines 111269 112595 +1326
==========================================
+ Hits 84970 86089 +1119
- Misses 19815 19948 +133
- Partials 6484 6558 +74 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Address review follow-ups on the db-sync shortcuts: - +db-sync-get pretty output no longer prints <nil> for a missing schema_only nor Go map syntax for statistics; render a bare bool and deterministic key=value pairs instead. - Reject a non-array field_maps in +db-sync-create --preview as well as commit, so the malformed shape is caught locally rather than forwarded to the backend. - Clarify in lark-apps-db.md that +db-sync-create --preview needs no confirmation and only a real create requires --yes. - Harden the db-sync dry-run validation tests to assert exit code 2 and the structured stderr envelope (type/subtype/param), and add coverage for the preview non-array field_maps rejection and batch pretty output.
Address the next db-sync review round: - +db-sync-create --preview --output no longer writes a "null" file and exits success when the response omits data.config; project config into a typed object and return internal/invalid_response without writing. - Make the 400002482 code hint command-neutral so +db-sync-update is not steered into a create-only recovery path that risks duplicate tasks. - lark-apps-db.md: carry --environment on the update lifecycle examples and split failure recovery by streaming (can update) vs batch (cannot update; recreate instead), removing the batch/update contradiction.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@skills/lark-apps/references/lark-apps-db.md`:
- Line 211: Update the batch recreation guidance near the “batch 任务” instruction
to require reusing the failed task’s original --environment value for both the
--preview and confirmed +db-sync-create commands. Preserve the existing workflow
of creating a new task rather than updating the original batch.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 743802aa-9f54-4693-9940-cbbb8be24412
📒 Files selected for processing (4)
shortcuts/apps/apps_db_sync_create.goshortcuts/apps/apps_db_sync_create_update_test.goshortcuts/apps/db_common.goskills/lark-apps/references/lark-apps-db.md
🚧 Files skipped from review as they are similar to previous changes (3)
- shortcuts/apps/apps_db_sync_create.go
- shortcuts/apps/apps_db_sync_create_update_test.go
- shortcuts/apps/db_common.go
Summary
Add the
apps +db-sync-*shortcut family for Base-to-database sync: preview, create (batch/streaming), list, get, enable/disable, update, delete. This lets AI agents import a Base table into a Miaoda app database and manage the resulting sync tasks, with validationand error hints tuned so agents recover from common failures without guessing.
Changes
apps +db-sync-create/list/get/enable/disable/update/deleteshortcuts undershortcuts/apps/(env in request body, POST for enable/disable/delete, task_id/config in body).sourcecontract: reject create locally whensource.base_urlhas no?table=andsource.table.nameis empty (point atbase +table-list); document thatsource.table.nametakes precedence over thebase_urltoken.500002776+ subcodek_dl_4000001(multi-env apps must create on--environment dev);500002783hint to add a unique text column via+db-executebefore retrying.field_mapson create: allow omitted or empty so the server auto-matches and creates the task; update still requires an enabled mapping; reject an all-disabled or non-arrayfield_maps.--environmentis omitted; help text, comments, and skill docs aligned.skills/lark-apps/references/lark-apps-db.md, unit contracts inshortcuts/apps/db_common_test.goandapps_db_sync_create_update_test.go, dry-run E2E intests/cli_e2e/apps/apps_db_sync_dryrun_test.go.Test Plan
make unit-testpassed (go test ./shortcuts/apps/...)tests/cli_e2e/appsdb-sync suite)lark-cli apps +db-sync-create ... --dry-runconfirmed create without field_maps passes and CLI does not inject field_maps; update without field_maps is rejectedRelated Issues
N/A
Summary by CodeRabbit