From 1e606bf0f355d3f07409592404c4f08a137aa0c5 Mon Sep 17 00:00:00 2001 From: Hermes Date: Sat, 3 Oct 2026 22:55:22 -0700 Subject: [PATCH] P7 r3 fix: single-preview contract (syncAccess delegates drift-check+preview+push to pushIfNoDrift; acl_preview_failed mapped non-retryable) + test updates --- .../network-provider-tailscale.test.js | 17 ++++++++------ .../src/network/providers/tailscale.js | 23 +++++++------------ 2 files changed, 18 insertions(+), 22 deletions(-) diff --git a/dashcaddy-api/__tests__/network-provider-tailscale.test.js b/dashcaddy-api/__tests__/network-provider-tailscale.test.js index 278024f..f03bffe 100644 --- a/dashcaddy-api/__tests__/network-provider-tailscale.test.js +++ b/dashcaddy-api/__tests__/network-provider-tailscale.test.js @@ -314,7 +314,7 @@ describe('tailscale provider: syncAccess() (DC-153 contract tests)', () => { hostDeviceId: 'dev-host', }); - test('happy path: compiles → previews → pushes, returns {pushed, hash, usersAffected}', async () => { + test('happy path: compiles → drift-guarded push (preview happens EXACTLY once, inside push), returns {pushed, hash, usersAffected}', async () => { const aclSync = makeAclSync(); const p = createTailscaleProvider({ coordClient: makeCoord(), aclSync }); const res = await p.syncAccess(input()); @@ -331,8 +331,9 @@ describe('tailscale provider: syncAccess() (DC-153 contract tests)', () => { }) ); // Preview runs on the compiled policy, BEFORE the push. - expect(aclSync.preview).toHaveBeenCalledTimes(1); - expect(aclSync.preview).toHaveBeenCalledWith({ acls: [] }); + // syncAccess no longer previews explicitly — engine.push's internal + // pushIfNoDrift does drift-check → preview → push (single preview). + expect(aclSync.preview).not.toHaveBeenCalled(); expect(aclSync.push).toHaveBeenCalledWith({ acls: [] }); }); @@ -352,10 +353,12 @@ describe('tailscale provider: syncAccess() (DC-153 contract tests)', () => { expect(aclSync.push).not.toHaveBeenCalled(); }); - test('preview errors[] → ProviderError acl_preview_failed, NOTHING pushed', async () => { + test('preview errors[] (thrown inside drift-guarded push) → ProviderError acl_preview_failed, NOTHING pushed', async () => { + // syncAccess no longer previews explicitly; pushIfNoDrift previews + // internally, so the internal preview failure surfaces THROUGH push. const aclSync = makeAclSync({ - preview: jest.fn(() => Promise.reject(new AclPushError('ACL preview reported errors', { - code: 'acl_rejected', + push: jest.fn(() => Promise.reject(new AclPushError('ACL preview reported errors', { + code: 'acl_preview_failed', cause: ['bad dst'], }))), }); @@ -365,7 +368,7 @@ describe('tailscale provider: syncAccess() (DC-153 contract tests)', () => { code: 'acl_preview_failed', retryable: false, }); - expect(aclSync.push).not.toHaveBeenCalled(); + expect(aclSync.push).toHaveBeenCalledTimes(1); }); test('push API failure (acl_rejected) → ProviderError bad_input, mapped not leaked', async () => { diff --git a/dashcaddy-api/src/network/providers/tailscale.js b/dashcaddy-api/src/network/providers/tailscale.js index 847e891..02db422 100644 --- a/dashcaddy-api/src/network/providers/tailscale.js +++ b/dashcaddy-api/src/network/providers/tailscale.js @@ -334,8 +334,9 @@ function createTailscaleProvider({ * DC-148 via src/network/acl-sync.js). Safety sequence, spec §4: * 1. compileAndValidate — LockoutError → ProviderError 'lockout_guard' * (retryable:false), nothing compiled-past let alone pushed - * 2. previewPolicy — ANY preview error → ProviderError - * 'acl_preview_failed', NOTHING pushed + * 2. drift guard + preview + push via pushIfNoDrift — ANY preview error + * → ProviderError 'acl_preview_failed', NOTHING pushed; a drifted + * remote → 'acl_drift_detected'; the policy is previewed EXACTLY once * 3. pushPolicy — API failure maps through the usual coord vocabulary; * success → { pushed: true, hash, usersAffected } * @@ -400,19 +401,9 @@ function createTailscaleProvider({ throw toProviderError(err); } - try { - // ANY preview error refuses the push (AclPushError from acl-push). - await engine.preview(compiled.policy); - } catch (err) { - if (err instanceof AclPushError) { - throw new ProviderError( - 'tailscale: ACL preview failed — refusing to push (' + err.message + ')', - { code: 'acl_preview_failed', retryable: false, cause: err } - ); - } - throw toProviderError(err); - } - + // Push goes through the drift guard: pushIfNoDrift performs + // drift-check → preview → push in ONE pass, so the policy is previewed + // EXACTLY once (a preview error throws before anything is pushed). let pushed; try { pushed = await engine.push(compiled.policy); @@ -434,11 +425,13 @@ function createTailscaleProvider({ let code = 'server_error'; if (err.code === 'acl_rejected') code = 'bad_input'; else if (err.code === 'rate_limited') code = 'rate_limited'; + else if (err.code === 'acl_preview_failed') code = 'acl_preview_failed'; const opts = { code, cause: err }; opts.retryable = err.code === 'rate_limited' || String(err.code).startsWith('http_429') || code === 'server_error'; + if (code === 'acl_preview_failed') opts.retryable = false; throw new ProviderError('tailscale: ACL push failed — ' + err.message, opts); } throw toProviderError(err);