Skip to content

Commit acd19ee

Browse files
pmaxhoganclaude
andcommitted
fix(ui): M6 recheck-1 frontend - preview-by-token/id, idempotent wizard, empty PKCE secret
R1-P1-2: previewExclusions DTO now carries localPathToken (new candidate) or sourceId (existing source) instead of a raw localPath; AddSourceWizard sends the dialog token, SourceTable's edit-exclusions sends the source id. R1-P2-3: setup store createFirstSource is idempotent - it short-circuits when sourceId is already set, so re-entering the encryption step (Back from confirm, then Next) does not re-call add_source with a consumed token. R1-P2-4: CredentialsWalkthrough canSubmit requires only a non-empty client ID (a PKCE installed-app client legitimately has an empty secret). Tests: SourceTable preview-by-sourceId, createFirstSource re-entry is a no-op, empty-secret submit allowed + forwarded, blocked with no client ID. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012CyiRqk2DVwmJjEu5gcD1m
1 parent c93bbe0 commit acd19ee

7 files changed

Lines changed: 111 additions & 12 deletions

File tree

ui/src/__tests__/settings-components.test.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -209,11 +209,11 @@ describe("SourceTable", () => {
209209
const editor = wrapper.get('[data-testid="exclusion-editor"]');
210210
expect(invokeMock).toHaveBeenCalledWith(
211211
"preview_exclusions",
212-
// The `previewExclusions` IPC wrapper nests the request under `req`
213-
// (matching the Rust `preview_exclusions(req: ExclusionPreviewRequest)`
214-
// signature), so the localPath lives at `req.localPath`.
212+
// R1-P1-2: an EXISTING source is previewed by its id (the backend resolves
213+
// the local path from SQLite), NEVER a raw webview path. The wrapper nests
214+
// the request under `req` (matching the Rust signature).
215215
expect.objectContaining({
216-
req: expect.objectContaining({ localPath: "/home/u/docs" }),
216+
req: expect.objectContaining({ sourceId: "src-1" }),
217217
}),
218218
);
219219
const excludeArea = editor.findAll("textarea")[1];

ui/src/__tests__/setup-wizard.test.ts

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ vi.mock("@tauri-apps/api/event", () => ({
3434

3535
import { i18n } from "../i18n";
3636
import SetupWizard from "../views/SetupWizard.vue";
37+
import CredentialsWalkthrough from "../components/CredentialsWalkthrough.vue";
3738
import { useSetupStore } from "../stores/setup";
3839
import { useSourcesStore } from "../stores/sources";
3940

@@ -189,6 +190,36 @@ describe("setup store OAuth sequence (SPEC s11.1)", () => {
189190
expect(sources.sources).toHaveLength(1);
190191
});
191192

193+
it("createFirstSource is idempotent - re-entry does not re-call add_source (R1-P2-3)", async () => {
194+
// The one-shot folder dialog token is CONSUMED by the backend on the first
195+
// add_source. Re-entering the encryption step (Back from confirm, then Next
196+
// again) must NOT re-call add_source - it would fail with a stale token and
197+
// wedge the wizard. Assert the second createFirstSource is a no-op.
198+
const setup = useSetupStore();
199+
setup.accountId = "acct-1";
200+
setup.localPath = "/home/user/Docs";
201+
setup.localPathToken = "tok-folder";
202+
setup.driveFolderId = "drive-folder-1";
203+
setup.driveFolderPath = "/Backups/Docs";
204+
setup.encryptionEnabled = true;
205+
206+
await setup.createFirstSource();
207+
expect(setup.sourceId).toBe("src-1");
208+
const addCallsAfterFirst = invokeMock.mock.calls.filter(
209+
(c) => c[0] === "add_source",
210+
).length;
211+
expect(addCallsAfterFirst).toBe(1);
212+
213+
// Re-enter the step: createFirstSource must short-circuit (no second add).
214+
await setup.createFirstSource();
215+
const addCallsAfterSecond = invokeMock.mock.calls.filter(
216+
(c) => c[0] === "add_source",
217+
).length;
218+
expect(addCallsAfterSecond).toBe(1);
219+
expect(setup.errorCode).toBeNull();
220+
expect(setup.sourceId).toBe("src-1");
221+
});
222+
192223
it("startInitialSync scopes sync_now to the new source", async () => {
193224
const setup = useSetupStore();
194225
setup.sourceId = "src-1";
@@ -335,3 +366,54 @@ describe("SetupWizard walks all five steps (DESIGN s8.5)", () => {
335366
]);
336367
});
337368
});
369+
370+
describe("CredentialsWalkthrough empty-secret (R1-P2-4, DESIGN s6.1)", () => {
371+
it("allows submit with a client ID and an EMPTY client secret", async () => {
372+
// A PKCE installed-app client legitimately has no secret. The sign-in button
373+
// must enable on a non-empty client ID ALONE, and submitting must pass the
374+
// (empty) secret straight through to the backend.
375+
installFakeBackend();
376+
const wrapper = mount(CredentialsWalkthrough, {
377+
global: { plugins: [i18n] },
378+
});
379+
await flushPromises();
380+
381+
const inputs = wrapper.findAll("input");
382+
// Client ID only; leave the secret EMPTY.
383+
await inputs[0].setValue("my-installed-app-client-id");
384+
await flushPromises();
385+
386+
const signInBtn = wrapper
387+
.findAll("button")
388+
.find((b) => b.text() === i18n.global.t("wizard.step2.signInButton"));
389+
expect(signInBtn).toBeTruthy();
390+
// R1-P2-4: enabled despite the empty secret.
391+
expect(signInBtn!.attributes("disabled")).toBeUndefined();
392+
393+
await signInBtn!.trigger("click");
394+
await flushPromises();
395+
396+
// The empty secret was forwarded as-is (trimmed empty string).
397+
expect(invokeMock).toHaveBeenCalledWith("submit_oauth_credentials", {
398+
session: FAKE_SESSION,
399+
clientId: "my-installed-app-client-id",
400+
clientSecret: "",
401+
});
402+
});
403+
404+
it("still blocks submit when the client ID is empty", async () => {
405+
installFakeBackend();
406+
const wrapper = mount(CredentialsWalkthrough, {
407+
global: { plugins: [i18n] },
408+
});
409+
await flushPromises();
410+
const inputs = wrapper.findAll("input");
411+
// Secret present but NO client ID -> still blocked.
412+
await inputs[1].setValue("some-secret");
413+
await flushPromises();
414+
const signInBtn = wrapper
415+
.findAll("button")
416+
.find((b) => b.text() === i18n.global.t("wizard.step2.signInButton"));
417+
expect(signInBtn!.attributes("disabled")).toBeDefined();
418+
});
419+
});

ui/src/components/AddSourceWizard.vue

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -205,12 +205,14 @@ async function goToCrumb(index: number): Promise<void> {
205205
}
206206
207207
async function loadPreview(): Promise<void> {
208-
if (localPath.value === null) return;
208+
// R1-P1-2: preview by the backend-minted dialog TOKEN (not a raw path). The
209+
// token is peeked non-consumingly, so add_source still gets its single use.
210+
if (localPathToken.value === null) return;
209211
previewLoading.value = true;
210212
errorMessage.value = null;
211213
try {
212214
preview.value = await ipc.previewExclusions({
213-
localPath: localPath.value,
215+
localPathToken: localPathToken.value,
214216
respectGitignore: respectGitignore.value,
215217
includePatterns: includePatterns.value,
216218
excludePatterns: excludePatterns.value,

ui/src/components/CredentialsWalkthrough.vue

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,11 +29,11 @@ const clientSecret = ref("");
2929
3030
let unlisten: UnlistenFn | null = null;
3131
32+
// R1-P2-4 (DESIGN s6.1): a PKCE installed-app client legitimately has an EMPTY
33+
// secret, so only a non-empty client ID is required to submit. The secret is
34+
// passed through as-is (possibly empty).
3235
const canSubmit = computed(
33-
() =>
34-
clientId.value.trim().length > 0 &&
35-
clientSecret.value.trim().length > 0 &&
36-
!setup.busy,
36+
() => clientId.value.trim().length > 0 && !setup.busy,
3737
);
3838
3939
/** Human-facing status line for the in-flight OAuth handshake. */

ui/src/components/SourceTable.vue

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,8 +82,10 @@ function cancelEdit(): void {
8282
async function loadEditPreview(source: SourceDto): Promise<void> {
8383
editPreviewLoading.value = true;
8484
try {
85+
// R1-P1-2 (SPEC s11.6.1): preview an EXISTING source by its id - the backend
86+
// resolves the local path from SQLite, never from a webview-supplied string.
8587
editPreview.value = await ipc.previewExclusions({
86-
localPath: source.localPath,
88+
sourceId: source.id,
8789
respectGitignore: editRespectGitignore.value,
8890
includePatterns: splitPatterns(editIncludeText.value),
8991
excludePatterns: splitPatterns(editExcludeText.value),

ui/src/ipc/types.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,12 @@ export interface DriveFolderListing {
119119
}
120120

121121
export interface ExclusionPreviewRequest {
122-
localPath: string;
122+
// R1-P1-2 (SPEC s11.6.1): the preview root is NEVER a raw webview path. Pass
123+
// EITHER the one-shot dialog token (a NEW candidate folder, from
124+
// pickFolderDialog) OR an existing source id; the backend resolves the path
125+
// from the token binding / SQLite. Exactly one must be set.
126+
localPathToken?: string | null;
127+
sourceId?: string | null;
123128
respectGitignore: boolean;
124129
includePatterns: string[];
125130
excludePatterns: string[];

ui/src/stores/setup.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,14 @@ export const useSetupStore = defineStore("setup", () => {
195195
* path + a picked Drive destination.
196196
*/
197197
async function createFirstSource(): Promise<void> {
198+
// R1-P2-3: idempotent. The one-shot folder dialog token is CONSUMED by the
199+
// backend on the first add_source, so re-entering the encryption step (Back
200+
// from confirm, then Next again) must NOT re-call add_source - it would fail
201+
// with a stale/consumed token and wedge the wizard. If the source already
202+
// exists, this is a no-op (the staged phrase + ack state are preserved).
203+
if (sourceId.value !== null) {
204+
return;
205+
}
198206
busy.value = true;
199207
errorCode.value = null;
200208
try {

0 commit comments

Comments
 (0)