fix: raw SQL from db export -o {data} envelope; warn when create silently relinks directory - #211
fix: raw SQL from db export -o {data} envelope; warn when create silently relinks directory#211agent-zhang-beihai[bot] wants to merge 2 commits into
Conversation
…te relinks the directory
- db export: the backend can wrap exports in a {format, data} JSON
envelope; only {format, content} was unwrapped, so '-o file.sql'
wrote the envelope instead of raw SQL. Accept a string 'data' key
too (extractExportContent) so the written file is directly usable.
- create: linking a blank project in cwd silently overwrote an
existing .insforge/project.json pointing at a different project,
so later db query/export commands hit the wrong project unnoticed.
Detect the relink (detectRelink) and print a prominent LINK CHANGED
warning; --json output gains a linkChanged field.
- Tests: new export.test.ts (envelope data/content unwrap + raw
passthrough via the real command), detectRelink cases in
create.test.ts.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
jwfing
left a comment
There was a problem hiding this comment.
Review — fix: raw SQL from db export -o {data} envelope; warn on silent relink
Summary: Two focused, well-tested bug fixes — unwrapping the {data} export envelope and warning on a silent create relink; both implementations are correct and in-scope, with no blocking issues.
Requirements context
No matching spec/plan found under docs/specs/ (the only specs present are 2026-03-27-diagnose-* and 2026-04-17-db-migrations-command-design.md; there is no docs/superpowers/). Assessed against the PR description and the referenced Linear feedback 4b503324 alone.
Findings
Critical
(none) — both reported frictions are addressed and neither change alters existing flags, defaults, or output shapes.
Suggestion
- Functionality — JSON-format exports may still write the envelope (
src/commands/db/export.ts:14-18).extractExportContentonly unwraps stringcontent/data. The PR description claims "-onow writes raw SQL/JSON", but if the backend returns a JSON export as{ format: 'json', data: { … } }(object-valueddata),extractExportContentreturnsnulland the-ofile gets the full envelope again — i.e. the JSON half of the reported bug would be unfixed. Your own testreturns null for non-string content/datapins this behavior for{ format: 'json', data: { tables: [] } }, and there is no--format json -oend-to-end test. Worth confirming the backend's actual json-export response shape; ifdatais an object there, this case needs handling (e.g.JSON.stringifythe object payload). Note this limitation also existed for the pre-existingcontentbranch, so it is not a regression — just an unclosed gap relative to the PR's stated scope.
Information
- Software engineering —
--jsonoutput path is unchanged and untested (src/commands/db/export.ts:73-77). In--jsonmode the command emitsoutputJson(meta ?? { content }), i.e. only{ format, tables }for an envelope response — the actual SQL/data is dropped — and--jsoncombined with-oreturns early at line 76 without writing the file. This is pre-existing behavior, not introduced here, and out of this PR's scope; flagging only because it is adjacent to the changed code and a one-line regression test would pin it. - Good structure. Extracting
detectRelink(src/commands/create.ts:139-149) andextractExportContentinto exported pure functions with focused unit tests matches the file's existing "logic extracted into pure functions" convention. The template-vs-blank distinction is correctly load-bearing on theprocess.chdir(projectDir)atsrc/commands/create.ts:353— for templatesdetectRelinkruns against the freshly-created empty subdir (→ no warning), for blank projects against the real cwd link (→ warning). The--jsonlinkChangedfield (create.ts:538-543) exposes only the user's own previous project id/name; no secrets/PII leaked, andapi_keyis not logged. - Test convention nit (
src/commands/db/export.test.ts:44-48). Returning() => rmSync(dir, …)frombeforeEachas teardown is valid vitest, but this pattern isn't used elsewhere in the suite; an explicitafterEachwould read more consistently. Harmless as-is.
Security
No security-relevant regressions: no new user input reaches SQL/shell/HTTP unparameterized (export body is built from existing opts and posted as before), no secrets/tokens newly logged or returned, and no auth/authz checks touched (requireAuth still gates both commands).
Performance
No concerns: both additions are O(1) — a single extra getProjectConfig() file read on the create path and a two-key type check on export. No new loops, queries, or blocking I/O.
Verdict
approved (informational — the human still approves via the approve flow). Zero Critical findings; the two Suggestions/Information notes are non-blocking. The core fixes are correct, minimally scoped, and backed by tests that exercise the real command paths.
Greptile SummaryThis PR improves database export response handling and reports when project creation changes the current directory’s link.
Confidence Score: 3/5The PR is not yet safe to merge because malformed link files can orphan newly created projects and valid raw JSON exports can still be silently truncated. The relink check reads and parses the old configuration only after creating the remote project, with no recovery for malformed JSON, while export envelope detection still treats ordinary string-valued data or content fields as wrappers and discards the rest of the document. Files Needing Attention: src/commands/create.ts, src/commands/db/export.ts
|
| Filename | Overview |
|---|---|
| src/commands/create.ts | Adds relink detection and reporting, but malformed existing project configuration can still abort after remote project creation. |
| src/commands/db/export.ts | Adds support for data-keyed envelopes, but envelope detection still truncates matching raw JSON documents. |
| src/commands/create.test.ts | Adds focused unit coverage for relink comparison but does not exercise malformed on-disk configuration. |
| src/commands/db/export.test.ts | Covers supported wrappers and plain-text fallback but not raw JSON containing string-valued data or content fields. |
Reviews (2): Last reviewed commit: "fix(db): preserve export content in JSON..." | Re-trigger Greptile
| // pointing at another project is about to be silently overwritten — | ||
| // detect it BEFORE saving so we can warn loudly (otherwise later | ||
| // db query/export commands hit the wrong project unnoticed). | ||
| const previousLink = detectRelink(getProjectConfig(), project.id); |
There was a problem hiding this comment.
Malformed link aborts creation
When an existing .insforge/project.json contains malformed JSON, the new getProjectConfig() call throws after the remote project has been created but before saveProjectConfig() replaces the file, causing the command to exit with the remote project left unlinked from the working directory.
Knowledge Base Used: Project Lifecycle: Create, Link, List, and Branch
| if (typeof parsed.content === 'string') return parsed.content; | ||
| if (typeof parsed.data === 'string') return parsed.data; |
There was a problem hiding this comment.
Envelope detection matches raw JSON
A raw JSON export with a top-level string-valued data or content property is treated as an envelope, so the output retains only that property instead of preserving the complete JSON document; require envelope-specific metadata such as format before unwrapping it.
Knowledge Base Used: Database commands (insforge db ...)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
jwfing
left a comment
There was a problem hiding this comment.
Review — #211 fix: raw SQL from db export -o {data} envelope; warn when create silently relinks directory
Summary: Tight, well-scoped two-part bugfix (export envelope unwrap + silent-relink warning) with good extracted-function unit tests; no blocking issues found.
Requirements context
No matching spec/plan found under docs/specs/ — the only specs present are 2026-03-27-diagnose-* and 2026-04-17-db-migrations-command-design.md, neither of which covers db export or create relink. Assessed against the PR description and linked feedback (4b503324) alone. This repo does not use docs/superpowers/.
Findings
Critical
(none)
Suggestion
-
[software engineering / test coverage]
src/commands/create.ts:373— the relink wiring is untested.detectRelink()has good pure-function tests (create.test.ts:154-171), but nothing verifies the two things that actually make this fix work: thatgetProjectConfig()is read beforesaveProjectConfig()overwrites it, and that the warning /linkChangedfield fires. The original bug class is precisely "detect after save" — if a future refactor moved line 373 below line 383,previous.project_idwould already equalproject.idanddetectRelinkwould silently returnnull, with all current tests still green.export.test.tsshows the command-level pattern (mockossFetch/requireAuth, driveparseAsync); an analogous integration test asserting thelinkChangedJSON on a pre-linked dir would lock in the regression guard. -
[functionality]
src/commands/db/export.ts:14-18—extractExportContentonly unwraps whencontent/datais a string. For--format json, if the backend wraps the payload as a JSON object ({format, data: {...}}), it returnsnulland-o dump.jsonwrites the whole{format, data, tables}envelope rather than just the data — the same envelope-in-file symptom this PR fixes for SQL.export.test.ts:54shows this is a deliberate, known limitation, and the reported issue was SQL, so blast radius is low. Worth confirming whether json-format exports need the same unwrap, or documenting that they intentionally pass through.
Information
src/commands/db/export.ts:65—parsed.tables as string[](andparsed.format as string) are unchecked casts; a malformed envelope would propagate a non-array intometa.tables. Only consumed for the.lengthcount suffix (:81), so effectively harmless, but the cast hides the assumption.src/commands/db/export.ts:73— the--jsonoutput shape gains acontentfield vs. the previousoutputJson(meta ?? {content})which droppedcontentwhen an envelope was present. This is the intended fix (and matches the cubic summary), just noting the observable shape change for any downstream--jsonconsumer.
Dimensions
- Software engineering: Extracted pure functions + focused tests follow repo conventions;
.jsimport extensions andtypeimports are consistent with the codebase. One integration-coverage gap noted above. - Functionality: Export unwrap and
--jsoncontent-preservation are correct; verifiedgetProjectConfig()reads cwd only (no upward tree-walk inconfig.ts:111-126), so the template-project path (whichchdirs into a fresh empty subdir before linking) correctly produces no false warning, matching the PR claim. - Security: No security-relevant changes — no new user input reaches SQL/shell; the warning only echoes the user's own project id/name; no secrets logged.
writeFileSyncstill targets the user-supplied-opath (unchanged, user-owned). - Performance: No concerns — trivial synchronous string/JSON work, no new I/O in a hot path.
Verdict
approved — zero Critical findings. Two Suggestions and two Information notes for the author's discretion; none blocking. (Human approval via the GitHub approve flow is still required.)
What was broken
Two frictions reported in the same feedback:
db export --format sql -o file.sqlwrote a JSON envelope. The backend can wrap the export payload as{format, data, tables}, butsrc/commands/db/export.tsonly unwrapped the{format, content, tables}shape — anydata-keyed response fell through to the raw-text branch and the.sqlfile ended up containing the JSON envelope instead of usable SQL.insforge createsilently relinked the working directory. Creating a blank project overwrites.insforge/project.jsonin cwd with no notice when the directory was already linked to a different project — subsequentdb query/db exportcommands then hit the wrong project unnoticed (the reporter's prod verification queries silently ran against staging).What changed
src/commands/db/export.ts: new exportedextractExportContent()accepts a stringcontentordatakey when unwrapping the export envelope, so-o file.sqlwrites raw SQL/JSON. Non-envelope responses still pass through verbatim.src/commands/create.ts: new exporteddetectRelink()checks the existing project link beforesaveProjectConfigoverwrites it. On a relink, interactive mode prints a prominentLINK CHANGED: this directory was linked to "<old>" ... and now points to "<new>"warning viaclack.log.warn, and--jsonoutput gains alinkChanged: {previousProjectId, previousProjectName}field. Template projects link inside a freshly created subdirectory, so they're unaffected (correctly no warning).No flags, defaults, or documented output shapes changed, so no
agent-skillsupdate is needed — the fix makes-obehave as the skill already documents.Verification
src/commands/db/export.test.ts: drives the realdb exportcommand with a mockedossFetch—{format, data}envelope → raw SQL written;{format, content}envelope still unwrapped; non-envelope response passes through verbatim; plus unit cases forextractExportContent.detectRelinkcases insrc/commands/create.test.ts: different project → previous link returned; unlinked dir → null; same project → null.npx vitest run→ 646 passed / 13 skipped.npm run lint→ 0 errors (1 pre-existing warning in unrelatedapify.test.ts).Addresses user feedback 4b503324-b3a5-4479-9b0d-7616c97a6dd4 (cli): CORRECTION: db export works — real issue is silently relinking + JSON envelope in -o output
🤖 Generated with Claude Code
Summary by cubic
Fixes db export to unwrap {content|data} envelopes so -o writes raw SQL/JSON and ensures --json includes the exported content. Also warns when create would relink the current directory to a different project and adds linkChanged to --json; addresses Linear feedback 4b503324.
Written for commit 241d7eb. Summary will update on new commits.
Note
Fix db export to unwrap
dataenvelope and warn on silent relink increatedb exportnow correctly handles backend responses that wrap the SQL payload in either acontentordataJSON field via the newextractExportContenthelper in export.ts; previously onlycontentwas handled, so-owould write the raw JSON wrapper instead of SQL.createnow detects when a directory was previously linked to a different project via the newdetectRelinkfunction in create.ts, logging a warning in interactive mode or including alinkChangedobject in JSON output.db export -owith adata-enveloped response now writes raw SQL instead of the JSON envelope.Macroscope summarized 47c6f1c.