fix(cloud-sync): drop CLI flag hints from library messages (#579) · deepnote/deepnote@7c1d02f · GitHub
Skip to content

Commit 7c1d02f

Browse files
tkislanclaude
andauthored
fix(cloud-sync): drop CLI flag hints from library messages (#579)
* fix(cloud-sync): drop CLI flag hints from library messages publishApp and syncWorkspace told library callers to pass CLI flags (--no-sync-root, --prune, --url) or named them (--path, --all-files). Callers have no flags; they need syncRoot: false, prune: true or baseUrl. The library now ends those messages before the hint and marks the condition in a field instead: - PublishError (and PublishMirrorError) carry an optional PublishErrorCode: 'sync-root-unusable' | 'project-not-in-sync-root'. - The kept-deleted-files warning event carries code 'deleted-files-kept'. - The pruned and missing-in-cloud outcome details no longer mention --prune. - The --path, --all-files and --url wording becomes neutral. `deepnote publish` and `deepnote sync` add the same hints back, so their stdout, stderr and -o json output are unchanged except for the three neutral strings (target path, working-file transfer limit, API URL). Closes #578 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VEcD7PPguvSkpmDm77ADFa * feat(cloud-sync): return structured option hints for the CLI to translate Alongside the codes, the library now says which option resolves each condition, as an OptionHint ({ option: 'syncRoot', value: false } or { option: 'prune', value: true }): - PublishError.hint, derived from its code - the deleted-files-kept warning event - the missing-in-cloud outcome kept because prune is off The CLI maps a hint to its flag in one place (cliFlagFor) and builds the same messages as before from it. `deepnote sync` no longer infers the --prune hint from its own option, and drops the hint from the outcomes it prints, so `-o json` output is unchanged. * refactor(cloud-sync): type the publish hints per error code PUBLISH_ERROR_HINTS is `as const satisfies`, PublishError.hint is the exact PublishErrorHint (exported), cliFlagFor returns the literal flag for the hint's option, and the CLI's publish sentences take the flag of their own code. Output is unchanged. Trim code comments that restated the code. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VEcD7PPguvSkpmDm77ADFa * docs: drop stale and restating option-hint comments ProjectSyncOutcome is no longer the `-o json` shape: the CLI strips `hint` and folds its flag into `detail`. The keptDeletedFilesHint doc spoke of several hints for one sentence. The other three comments restated the code or the OptionHint and PublishErrorHint contracts. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VEcD7PPguvSkpmDm77ADFa * refactor(cloud-sync): type sync hints as prune-only and correct hint docs The deleted-files-kept warning is now its own SyncEvent member with a required prune hint, and ProjectSyncOutcome.hint is the prune hint only. A syncRoot hint on either carrier no longer compiles, so the CLI drops its `event.hint` guard and the test for a warning with a code but no hint, a shape the library cannot emit. Correct the docs that overstated behavior: pruning a missing-in-cloud project whose path a live project reuses only drops its manifest record; pushing an edited kept file raises a working-files-changed conflict rather than restoring it; PublishMirrorError is rethrown only while the folder is resolved and reported as mirror-incomplete while updating it. Drop comments that restated the code. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VEcD7PPguvSkpmDm77ADFa * test(cli): drop sync test rows that only echo mocked library messages The large.bin outcome row and the different-workspace refusal row passed a copy of the reworded library message through the mocked syncWorkspace and expected it back, so they could not catch the library returning to the old flag wording; the cloud-sync tests cover that. The existing 'boom' and 'an Error' rows already cover pass-through, and the large.bin row gave it.each a second test named "renders a 'error' outcome as its progress line". Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VEcD7PPguvSkpmDm77ADFa * test(cloud-sync): assert the full different-workspace refusal message rejects.toThrow('<string>') passes whenever the thrown message contains the string, so a flag hint appended to the refusal (for example "Pass --url to fix.") went unnoticed. toThrow(new Error(...)) requires the exact message. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VEcD7PPguvSkpmDm77ADFa * Small fixes --------- Co-authored-by: Claude Sonnet 5.5 <noreply@anthropic.com>
1 parent 60b744f commit 7c1d02f

15 files changed

Lines changed: 501 additions & 62 deletions

‎packages/cli/src/commands/publish.test.ts‎

Lines changed: 30 additions & 0 deletions

‎packages/cli/src/commands/publish.ts‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ import {
44
type PublishAppResult,
55
PublishDivergedError,
66
PublishError,
7+
type PublishErrorCode,
8+
type PublishErrorHint,
79
publishApp,
810
type SyncRootOption,
911
} from '@deepnote/cloud-sync'
@@ -13,6 +15,7 @@ import dotenv from 'dotenv'
1315
import { ExitCode } from '../exit-codes'
1416
import { debug, getChalk, log, error as logError, warn } from '../output'
1517
import { MissingTokenError, resolveToken } from '../utils/auth'
18+
import { type CliFlag, cliFlagFor } from '../utils/option-hint'
1619
import { embeddedApiAccessNote } from '../utils/static-site-api-access'
1720

1821
interface PublishOptions {
@@ -27,6 +30,18 @@ interface PublishOptions {
2730
force: boolean
2831
}
2932

33+
const PUBLISH_ERROR_SENTENCES: { [C in PublishErrorCode]: (flag: CliFlag<PublishErrorHint<C>['option']>) => string } = {
34+
'sync-root-unusable': flag => ` Pass ${flag} to publish without updating the workspace.`,
35+
'project-not-in-sync-root': flag => ` Sync the workspace first, or pass ${flag} to publish without updating it.`,
36+
}
37+
38+
function publishErrorSuffix(error: unknown): string {
39+
if (!(error instanceof PublishError) || !error.code || !error.hint) {
40+
return ''
41+
}
42+
return PUBLISH_ERROR_SENTENCES[error.code](cliFlagFor(error.hint))
43+
}
44+
3045
export function createPublishAction(program: Command) {
3146
return async (dir: string, options: PublishOptions) => {
3247
const c = getChalk()
@@ -102,7 +117,7 @@ export function createPublishAction(program: Command) {
102117
onEvent: renderEvent,
103118
})
104119
} catch (error) {
105-
const message = error instanceof Error ? error.message : String(error)
120+
const message = (error instanceof Error ? error.message : String(error)) + publishErrorSuffix(error)
106121
if (error instanceof PublishDivergedError) {
107122
logError(
108123
`${message}. Run \`deepnote sync --all-files\` to bring the changes down, ` +

‎packages/cli/src/commands/sync.test.ts‎

Lines changed: 145 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -286,14 +286,15 @@ describe('deepnote sync events', () => {
286286
line: '⚠ skipped Team/Alpha — modified both locally and in the cloud',
287287
},
288288
{
289-
outcome: { ...outcome, action: 'pruned', detail: 'no longer in the cloud; removed locally (--prune)' },
289+
outcome: { ...outcome, action: 'pruned', detail: 'no longer in the cloud; removed locally' },
290290
line: '✕ pruned Team/Alpha — no longer in the cloud; removed locally (--prune)',
291291
},
292292
{
293293
outcome: {
294294
...outcome,
295295
action: 'missing-in-cloud',
296-
detail: 'no longer in the cloud; kept locally (use --prune to remove)',
296+
detail: 'no longer in the cloud; kept locally',
297+
hint: { option: 'prune', value: true },
297298
},
298299
line: '? missing Team/Alpha — no longer in the cloud; kept locally (use --prune to remove)',
299300
},
@@ -311,6 +312,38 @@ describe('deepnote sync events', () => {
311312
}
312313
)
313314

315+
it('does not hint at --prune for a missing project whose path a current cloud project uses', async () => {
316+
emitAll([
317+
{
318+
kind: 'project-outcome',
319+
outcome: {
320+
...outcome,
321+
action: 'missing-in-cloud',
322+
detail: 'no longer in the cloud; kept local path used by a current cloud project',
323+
},
324+
},
325+
])
326+
327+
await sync('--prune')
328+
329+
expect(logged[0]).toBe(
330+
'? missing Team/Alpha — no longer in the cloud; kept local path used by a current cloud project'
331+
)
332+
})
333+
334+
it('adds no suffix to a missing project without a hint, though --prune was not passed', async () => {
335+
emitAll([
336+
{
337+
kind: 'project-outcome',
338+
outcome: { ...outcome, action: 'missing-in-cloud', detail: 'no longer in the cloud; kept locally' },
339+
},
340+
])
341+
342+
await sync()
343+
344+
expect(logged[0]).toBe('? missing Team/Alpha — no longer in the cloud; kept locally')
345+
})
346+
314347
it('suppresses the listing and outcome lines under -o json, leaving the JSON document alone on stdout', async () => {
315348
emitAll([
316349
{ kind: 'listing-projects', baseUrl: 'https://api.example.com' },
@@ -340,6 +373,27 @@ describe('deepnote sync events', () => {
340373
expect(errored).toEqual(['Skipping file with unsafe path in "C": ../evil.csv'])
341374
})
342375

376+
it.each([[[]], [['-o', 'json']]])(
377+
'adds the --prune hint to a kept-deleted-files warning on stderr (flags %j)',
378+
async flags => {
379+
emitAll([
380+
{
381+
kind: 'warning',
382+
code: 'deleted-files-kept',
383+
hint: { option: 'prune', value: true },
384+
message: '2 files in "Alpha" deleted in Deepnote but kept locally: a.txt, b.txt.',
385+
},
386+
])
387+
388+
await sync(...flags)
389+
390+
expect(errored).toEqual([
391+
'2 files in "Alpha" deleted in Deepnote but kept locally: a.txt, b.txt. ' +
392+
'Run sync --prune to remove them; pushing an edited copy restores that file in Deepnote.',
393+
])
394+
}
395+
)
396+
343397
it('reports file transfers as debug output only', async () => {
344398
emitAll([
345399
{ kind: 'file-transferred', direction: 'download', projectName: 'Alpha', path: 'data/a.csv', bytes: 12 },
@@ -538,6 +592,95 @@ describe('deepnote sync result handling', () => {
538592
])
539593
})
540594

595+
describe('prune hints in the result', () => {
596+
const outcome = { projectId: 'p1', name: 'Alpha', path: 'Team/Alpha' } as const
597+
const missing = {
598+
...outcome,
599+
action: 'missing-in-cloud',
600+
detail: 'no longer in the cloud; kept locally',
601+
hint: { option: 'prune', value: true },
602+
} as const
603+
const pruned = { ...outcome, action: 'pruned', detail: 'no longer in the cloud; removed locally' } as const
604+
const keptForLiveProject = {
605+
...outcome,
606+
action: 'missing-in-cloud',
607+
detail: 'no longer in the cloud; kept local path used by a current cloud project',
608+
} as const
609+
610+
function printedProjects(): { detail?: string }[] {
611+
return JSON.parse(logged[logged.length - 1]).projects
612+
}
613+
614+
it.each([
615+
[
616+
'a hinted missing project',
617+
missing,
618+
[],
619+
{
620+
...outcome,
621+
action: 'missing-in-cloud',
622+
detail: 'no longer in the cloud; kept locally (use --prune to remove)',
623+
},
624+
],
625+
[
626+
'a pruned project',
627+
pruned,
628+
['--prune'],
629+
{ ...outcome, action: 'pruned', detail: 'no longer in the cloud; removed locally (--prune)' },
630+
],
631+
])('adds the flag to %s in the -o json document', async (_name, projectOutcome, flags, expected) => {
632+
mockedSyncWorkspace.mockResolvedValue(resultOf({ projects: [projectOutcome] }))
633+
634+
await sync('-o', 'json', ...flags)
635+
636+
expect(printedProjects()).toEqual([expected])
637+
})
638+
639+
it('keeps the hint out of the -o json document and the key order of the outcome', async () => {
640+
mockedSyncWorkspace.mockResolvedValue(resultOf({ projects: [missing] }))
641+
642+
await sync('-o', 'json')
643+
644+
expect(Object.keys(printedProjects()[0])).toEqual(['projectId', 'name', 'path', 'action', 'detail'])
645+
})
646+
647+
it('leaves the kept-for-a-live-project detail alone under --prune', async () => {
648+
mockedSyncWorkspace.mockResolvedValue(resultOf({ projects: [keptForLiveProject] }))
649+
650+
await sync('-o', 'json', '--prune')
651+
652+
expect(printedProjects()).toEqual([keptForLiveProject])
653+
})
654+
655+
it('leaves the details of other outcomes alone', async () => {
656+
const projects = [
657+
{ ...outcome, action: 'pulled', detail: 'moved from Old/Alpha' },
658+
{ ...outcome, action: 'error', detail: 'boom' },
659+
{ ...outcome, action: 'unchanged' },
660+
] as const
661+
mockedSyncWorkspace.mockResolvedValue(resultOf({ projects: [...projects] }))
662+
663+
await sync('-o', 'json')
664+
665+
expect(printedProjects()).toEqual(projects)
666+
})
667+
668+
it('returns from runSync the same outcomes that -o json prints', async () => {
669+
mockedSyncWorkspace.mockResolvedValue(resultOf({ projects: [missing] }))
670+
671+
const result = await runSync(tempDir, { token: 'tok' })
672+
673+
expect(result.projects).toEqual([
674+
{
675+
...outcome,
676+
action: 'missing-in-cloud',
677+
detail: 'no longer in the cloud; kept locally (use --prune to remove)',
678+
},
679+
])
680+
expect(result.projects[0]).not.toHaveProperty('hint')
681+
})
682+
})
683+
541684
it('still sets exit code 1 for an unsuccessful result under -o json', async () => {
542685
mockedSyncWorkspace.mockResolvedValue(failing)
543686

‎packages/cli/src/commands/sync.ts‎

Lines changed: 25 additions & 3 deletions

0 commit comments

Comments
 (0)