app store: ask before repointing a service that is already connected
Found by breaking it. Installing a sidecar that was already configured replaced its connection row with the new install's, silently repointing a working service somewhere else — during testing that took the live Transmission from :9091 to a scratch container on :18092, and the only symptom was that it stopped working. Install now blocks instead of overwriting, naming both URLs and offering the choice. Blocked rather than failed because there is a sensible answer and the user is the only one who has it: keep what is there, or reinstall with `replaceConnection` to change it deliberately. Harmless on a fresh machine; this is entirely for the one with an existing setup. Verified against the live row: an install pointed at a different URL is refused and the original connection is still there afterwards. Also makes "do you already have one?" structural rather than a UI convention. Three tests: anything that can provision must also offer `existing`, `existing` must come first in `modes` since that is the order the prompt uses, and it must ask for a URL. A new entry added later cannot quietly offer only "provision one for me" — which is how someone with a working Immich ends up with a second one and finds out when two libraries disagree. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -152,3 +152,35 @@ describe('the UI manifest each sidecar carries', () => {
|
|||||||
expect(new Set(roots).size).toBe(roots.length);
|
expect(new Set(roots).size).toBe(roots.length);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe('the "do you already have one?" question', () => {
|
||||||
|
it('is always offered by anything that can provision', () => {
|
||||||
|
// The rule: a sidecar that would start a container must first let the user point at one they
|
||||||
|
// already run. Offering only 'provisioned' means someone with a working Immich gets a second one,
|
||||||
|
// and only finds out when two libraries disagree.
|
||||||
|
//
|
||||||
|
// Enforced here rather than trusted to the UI, because a new entry added later would otherwise
|
||||||
|
// silently skip the question.
|
||||||
|
for (const entry of CATALOGUE) {
|
||||||
|
if (!entry.modes.includes('provisioned')) continue;
|
||||||
|
expect(entry.modes).toContain('existing');
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
it('offers "existing" first, so the prompt leads with it', () => {
|
||||||
|
// `modes` is the order the UI presents them in. Leading with "provision one for me" invites a
|
||||||
|
// second instance from someone who already has one.
|
||||||
|
for (const entry of CATALOGUE) {
|
||||||
|
if (!entry.modes.includes('existing')) continue;
|
||||||
|
expect(entry.modes[0]).toBe('existing');
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
it('asks for a URL wherever an existing instance can be named', () => {
|
||||||
|
// Without one there is nothing to connect to, and "I already have it" cannot be answered.
|
||||||
|
for (const entry of CATALOGUE) {
|
||||||
|
if (!entry.modes.includes('existing')) continue;
|
||||||
|
expect(entry.existingFields?.some((f) => f.key === 'url' && f.required)).toBe(true);
|
||||||
|
}
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
@@ -1,5 +1,5 @@
|
|||||||
import { join } from 'node:path';
|
import { join } from 'node:path';
|
||||||
import { getOwnerUser, saveServiceConnection } from 'officerdb';
|
import { getOwnerUser, saveServiceConnection, getServiceConnection } from 'officerdb';
|
||||||
import type { InstallEffects, StepContext } from './installer';
|
import type { InstallEffects, StepContext } from './installer';
|
||||||
import { preflight } from './preflight';
|
import { preflight } from './preflight';
|
||||||
import { runSetupScript } from './run-script';
|
import { runSetupScript } from './run-script';
|
||||||
@@ -67,6 +67,26 @@ export function createEffects(): InstallEffects {
|
|||||||
const owner = await getOwnerUser();
|
const owner = await getOwnerUser();
|
||||||
if (!owner) throw new Error('no owner account');
|
if (!owner) throw new Error('no owner account');
|
||||||
|
|
||||||
|
// Never overwrite a connection the owner already has, unless they said so.
|
||||||
|
//
|
||||||
|
// Found the hard way: installing a sidecar that was already configured replaced its connection
|
||||||
|
// row with the new install's, silently repointing a working service at something else. On a fresh
|
||||||
|
// machine that is harmless; on one with an existing setup it breaks a feature that was fine, and
|
||||||
|
// the only symptom is that it stops working.
|
||||||
|
//
|
||||||
|
// Blocking rather than failing, because there is a sensible answer and the user is the only one
|
||||||
|
// who has it: keep what is there, or replace it deliberately.
|
||||||
|
const current = await getServiceConnection(owner.id, ctx.entry.id as never);
|
||||||
|
if (current && current.url && current.url !== url && ctx.values.replaceConnection !== 'true') {
|
||||||
|
return {
|
||||||
|
status: 'blocked',
|
||||||
|
reason:
|
||||||
|
`${ctx.entry.label} is already connected to ${current.url}. Installing would repoint it to ` +
|
||||||
|
`${url}. Keep the existing connection, or reinstall with "replace connection" to change it.`,
|
||||||
|
completeAt: current.url,
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
// The OWNER's row: it carries the URL and IS the instance. Members get their own rows later, with
|
// The OWNER's row: it carries the URL and IS the instance. Members get their own rows later, with
|
||||||
// a null url that inherits this one — see members.ts and the service_connections schema.
|
// a null url that inherits this one — see members.ts and the service_connections schema.
|
||||||
await saveServiceConnection({
|
await saveServiceConnection({
|
||||||
|
|||||||
Reference in New Issue
Block a user