diff --git a/README.md b/README.md index 5eadaac..11958e7 100644 --- a/README.md +++ b/README.md @@ -487,21 +487,24 @@ EXAMPLES ## `bitsocial challenge remove NAME` -Remove an installed challenge package +Remove an installed challenge package. Refuses while a community's settings.challenges still names it, unless --force is passed ``` USAGE - $ bitsocial challenge remove NAME --pkcRpcUrl [--pkcOptions.dataPath ] + $ bitsocial challenge remove NAME --pkcRpcUrl [--pkcOptions.dataPath ] [--force] ARGUMENTS NAME The challenge package name (e.g., my-challenge or @scope/my-challenge) FLAGS + --force Remove without checking whether communities still use it (the daemon is not contacted + for the check) --pkcOptions.dataPath= Data path where challenges are installed --pkcRpcUrl= (required) [default: ws://localhost:9138/] URL to PKC RPC DESCRIPTION - Remove an installed challenge package + Remove an installed challenge package. Refuses while a community's settings.challenges still names it, unless --force + is passed ALIASES $ bitsocial challenge uninstall @@ -512,27 +515,32 @@ EXAMPLES $ bitsocial challenge remove my-challenge $ bitsocial challenge remove @scope/my-challenge + + $ bitsocial challenge remove my-challenge --force ``` _See code: [src/cli/commands/challenge/remove.ts](https://github.com/bitsocialnet/bitsocial-cli/blob/v0.19.108/src/cli/commands/challenge/remove.ts)_ ## `bitsocial challenge rm NAME` -Remove an installed challenge package +Remove an installed challenge package. Refuses while a community's settings.challenges still names it, unless --force is passed ``` USAGE - $ bitsocial challenge rm NAME --pkcRpcUrl [--pkcOptions.dataPath ] + $ bitsocial challenge rm NAME --pkcRpcUrl [--pkcOptions.dataPath ] [--force] ARGUMENTS NAME The challenge package name (e.g., my-challenge or @scope/my-challenge) FLAGS + --force Remove without checking whether communities still use it (the daemon is not contacted + for the check) --pkcOptions.dataPath= Data path where challenges are installed --pkcRpcUrl= (required) [default: ws://localhost:9138/] URL to PKC RPC DESCRIPTION - Remove an installed challenge package + Remove an installed challenge package. Refuses while a community's settings.challenges still names it, unless --force + is passed ALIASES $ bitsocial challenge uninstall @@ -543,25 +551,30 @@ EXAMPLES $ bitsocial challenge remove my-challenge $ bitsocial challenge remove @scope/my-challenge + + $ bitsocial challenge remove my-challenge --force ``` ## `bitsocial challenge un NAME` -Remove an installed challenge package +Remove an installed challenge package. Refuses while a community's settings.challenges still names it, unless --force is passed ``` USAGE - $ bitsocial challenge un NAME --pkcRpcUrl [--pkcOptions.dataPath ] + $ bitsocial challenge un NAME --pkcRpcUrl [--pkcOptions.dataPath ] [--force] ARGUMENTS NAME The challenge package name (e.g., my-challenge or @scope/my-challenge) FLAGS + --force Remove without checking whether communities still use it (the daemon is not contacted + for the check) --pkcOptions.dataPath= Data path where challenges are installed --pkcRpcUrl= (required) [default: ws://localhost:9138/] URL to PKC RPC DESCRIPTION - Remove an installed challenge package + Remove an installed challenge package. Refuses while a community's settings.challenges still names it, unless --force + is passed ALIASES $ bitsocial challenge uninstall @@ -572,25 +585,30 @@ EXAMPLES $ bitsocial challenge remove my-challenge $ bitsocial challenge remove @scope/my-challenge + + $ bitsocial challenge remove my-challenge --force ``` ## `bitsocial challenge uninstall NAME` -Remove an installed challenge package +Remove an installed challenge package. Refuses while a community's settings.challenges still names it, unless --force is passed ``` USAGE - $ bitsocial challenge uninstall NAME --pkcRpcUrl [--pkcOptions.dataPath ] + $ bitsocial challenge uninstall NAME --pkcRpcUrl [--pkcOptions.dataPath ] [--force] ARGUMENTS NAME The challenge package name (e.g., my-challenge or @scope/my-challenge) FLAGS + --force Remove without checking whether communities still use it (the daemon is not contacted + for the check) --pkcOptions.dataPath= Data path where challenges are installed --pkcRpcUrl= (required) [default: ws://localhost:9138/] URL to PKC RPC DESCRIPTION - Remove an installed challenge package + Remove an installed challenge package. Refuses while a community's settings.challenges still names it, unless --force + is passed ALIASES $ bitsocial challenge uninstall @@ -601,6 +619,8 @@ EXAMPLES $ bitsocial challenge remove my-challenge $ bitsocial challenge remove @scope/my-challenge + + $ bitsocial challenge remove my-challenge --force ``` ## `bitsocial community create` @@ -1160,7 +1180,8 @@ ARGUMENTS NAME The page sort package name (e.g., my-page-sort or @scope/my-page-sort) FLAGS - --force Remove even if communities still use it, or if the daemon cannot be reached to check + --force Remove without checking whether communities still use it (the daemon is not contacted + for the check) --pkcOptions.dataPath= Data path where page sorts are installed --pkcRpcUrl= (required) [default: ws://localhost:9138/] URL to PKC RPC @@ -1193,7 +1214,8 @@ ARGUMENTS NAME The page sort package name (e.g., my-page-sort or @scope/my-page-sort) FLAGS - --force Remove even if communities still use it, or if the daemon cannot be reached to check + --force Remove without checking whether communities still use it (the daemon is not contacted + for the check) --pkcOptions.dataPath= Data path where page sorts are installed --pkcRpcUrl= (required) [default: ws://localhost:9138/] URL to PKC RPC @@ -1224,7 +1246,8 @@ ARGUMENTS NAME The page sort package name (e.g., my-page-sort or @scope/my-page-sort) FLAGS - --force Remove even if communities still use it, or if the daemon cannot be reached to check + --force Remove without checking whether communities still use it (the daemon is not contacted + for the check) --pkcOptions.dataPath= Data path where page sorts are installed --pkcRpcUrl= (required) [default: ws://localhost:9138/] URL to PKC RPC @@ -1255,7 +1278,8 @@ ARGUMENTS NAME The page sort package name (e.g., my-page-sort or @scope/my-page-sort) FLAGS - --force Remove even if communities still use it, or if the daemon cannot be reached to check + --force Remove without checking whether communities still use it (the daemon is not contacted + for the check) --pkcOptions.dataPath= Data path where page sorts are installed --pkcRpcUrl= (required) [default: ws://localhost:9138/] URL to PKC RPC diff --git a/docs/challenges.md b/docs/challenges.md index c9f0189..b23a2fb 100644 --- a/docs/challenges.md +++ b/docs/challenges.md @@ -22,7 +22,7 @@ bitsocial challenge install https://example.com/my-challenge-1.0.0.tar.gz bitsocial challenge install ./my-local-challenge ``` -All three commands take `--pkcOptions.dataPath` (default: the platform data dir from `env-paths("bitsocial")`, e.g. `~/.local/share/bitsocial` on Linux). `install` and `remove` also take `--pkcRpcUrl` (default `ws://localhost:9138`) to find the daemon to reload. +All three commands take `--pkcOptions.dataPath` (default: the platform data dir from `env-paths("bitsocial")`, e.g. `~/.local/share/bitsocial` on Linux). `install` and `remove` also take `--pkcRpcUrl` (default `ws://localhost:9138`) to find the daemon to reload (and, for `remove`, to check which communities use the package). `remove` takes the package name, not the spec you installed from: `bitsocial challenge remove @bitsocial/mintpass-challenge`. @@ -93,7 +93,27 @@ For larger changes, write the settings to a file and pass `--jsonFile `; C ## Removing -`challenge remove` deletes the directory and reloads the daemon. It does **not** check whether a community still names the package in `settings.challenges`. Check first with `bitsocial community get
`. (`page-sort remove` does refuse in that case; see [page-sorts.md](page-sorts.md#removing).) +```bash +bitsocial challenge remove my-challenge +``` + +`remove` first checks that the package is installed, without contacting the daemon. Then it connects to the daemon at `--pkcRpcUrl` and checks `settings.challenges` in every community, running or stopped. If any entry names the package, it refuses and lists them (oclif prefixes each output line with ` › `): + +``` +Error: Challenge "my-challenge" is still used by: + - mycommunity.bso settings.challenges[1] +Change or remove those entries first, e.g. bitsocial community edit
--jsonFile , +or pass --force to remove it anyway (those communities reject every publication and fail to start until their settings are fixed). +``` + +Otherwise it deletes the directory and reloads the daemon. + +The reason: pkc-js can't load a challenge whose name is no longer registered. A running community keeps advertising the old challenge but rejects every publication (`One of the community challenges is misconfigured`), and on its next start (including a daemon restart) `start()` throws and the community stays offline until its settings are changed. + +- **Only `name` entries count.** An entry with a `path` loads that file, so it doesn't use the package. +- **A package named after a pkc-js built-in** (`question`, `text-math`, `fail`, `blacklist`, `whitelist`, `publication-match`) is refused the same way. Removing it hands the built-in back, so nothing breaks, but those communities switch to the built-in challenge. The refusal says so. +- **The daemon must be running** for the check. If it can't be reached, `remove` refuses and says so. +- **`--force`** skips the check and doesn't contact the daemon for it. After removing, it prints a warning that any community still naming the package rejects every publication and fails to start (or, for a built-in name, that it now uses the built-in). ## Troubleshooting diff --git a/docs/page-sorts.md b/docs/page-sorts.md index 91baa24..e29458b 100644 --- a/docs/page-sorts.md +++ b/docs/page-sorts.md @@ -99,7 +99,7 @@ Quote the flags in the shell: the `[0]` brackets are glob characters, and zsh re bitsocial page-sort remove @pkcprotocol/active-page-sort ``` -Before deleting, `remove` connects to the daemon at `--pkcRpcUrl` and checks every community's `settings.pages`. If any entry names the package, it refuses and lists them (oclif prefixes each output line with ` › `): +`remove` first checks that the package is installed, without contacting the daemon. Then it connects to the daemon at `--pkcRpcUrl` and checks every community's `settings.pages`. If any entry names the package, it refuses and lists them (oclif prefixes each output line with ` › `): ``` Error: Page sort "@pkcprotocol/active-page-sort" is still used by: @@ -111,7 +111,7 @@ or pass --force to remove it anyway (the communities keep publishing without thi The reason: a community whose `settings.pages` names a missing sort keeps publishing but skips that entry and logs an error on every start, so the board silently loses the sort. - **The daemon must be running** for the check. If it cannot be reached, `remove` refuses and says so. -- **`--force`** skips the check, both for in-use packages and for an unreachable daemon. +- **`--force`** skips the check and doesn't contact the daemon for it. After removing, it prints a warning that any community still naming the package keeps publishing without this sort. ## Troubleshooting diff --git a/src/challenge-packages/challenge-utils.ts b/src/challenge-packages/challenge-utils.ts index 81a72fb..e6c3c4a 100644 --- a/src/challenge-packages/challenge-utils.ts +++ b/src/challenge-packages/challenge-utils.ts @@ -61,3 +61,12 @@ export const hashChallengePackageContents = hashPackageContents; export const loadChallengesIntoPKC = (dataPath?: string): Promise => loadPackagesIntoPKC(CHALLENGE_PACKAGE_KIND, dataPath); + +/** Where a community's settings.challenges names `challengeName`, e.g. "challenges[1]". An entry with a path loads that file instead. */ +export function findChallengeUsages(challengeSettings: { name?: string; path?: string }[] | undefined, challengeName: string): string[] { + const usages: string[] = []; + challengeSettings?.forEach((entry, index) => { + if (entry?.name === challengeName && !entry.path) usages.push(`challenges[${index}]`); + }); + return usages; +} diff --git a/src/cli/commands/challenge/remove.ts b/src/cli/commands/challenge/remove.ts index b08aa73..2f619b1 100644 --- a/src/cli/commands/challenge/remove.ts +++ b/src/cli/commands/challenge/remove.ts @@ -1,11 +1,20 @@ import { Args, Flags } from "@oclif/core"; import { BaseCommand } from "../../base-command.js"; import defaults from "../../../common-utils/defaults.js"; -import { removePackage, PackageNotInstalledError, InvalidPackageNameError } from "../../../common-utils/package-registry.js"; -import { CHALLENGE_PACKAGE_KIND, reloadChallengesInDaemon } from "../../../challenge-packages/challenge-utils.js"; +import { + removePackage, + assertPackageInstalled, + collectPackageUsages, + isBuiltInPackageName, + PackageNotInstalledError, + InvalidPackageNameError, + assertValidPackageName +} from "../../../common-utils/package-registry.js"; +import { CHALLENGE_PACKAGE_KIND, findChallengeUsages, reloadChallengesInDaemon } from "../../../challenge-packages/challenge-utils.js"; export default class Remove extends BaseCommand { - static override description = "Remove an installed challenge package"; + static override description = + "Remove an installed challenge package. Refuses while a community's settings.challenges still names it, unless --force is passed"; static override aliases = ["challenge:uninstall", "challenge:rm", "challenge:un"]; @@ -20,12 +29,17 @@ export default class Remove extends BaseCommand { "pkcOptions.dataPath": Flags.directory({ description: "Data path where challenges are installed", required: false + }), + force: Flags.boolean({ + description: "Remove without checking whether communities still use it (the daemon is not contacted for the check)", + default: false }) }; static override examples = [ "bitsocial challenge remove my-challenge", - "bitsocial challenge remove @scope/my-challenge" + "bitsocial challenge remove @scope/my-challenge", + "bitsocial challenge remove my-challenge --force" ]; async run(): Promise { @@ -33,7 +47,13 @@ export default class Remove extends BaseCommand { const dataPath = flags["pkcOptions.dataPath"] || defaults.PKC_DATA_PATH; let removed: { version?: string }; + let builtIn: boolean; try { + assertValidPackageName(args.name); + // Checked before the daemon, so a misspelled name says so rather than that the daemon is down + await assertPackageInstalled(CHALLENGE_PACKAGE_KIND, { name: args.name, dataPath }); + builtIn = await isBuiltInPackageName(CHALLENGE_PACKAGE_KIND, args.name); + if (!flags.force) await this._refuseIfInUse(args.name, builtIn, flags.pkcRpcUrl.toString()); removed = await removePackage(CHALLENGE_PACKAGE_KIND, { name: args.name, dataPath }); } catch (err) { if (err instanceof PackageNotInstalledError || err instanceof InvalidPackageNameError) this.error(err.message); @@ -41,9 +61,44 @@ export default class Remove extends BaseCommand { } this.log(`removed ${args.name}${removed.version ? `@${removed.version}` : ""}`); + if (flags.force) + this.warn( + `Skipped the in-use check. A community whose settings.challenges still names "${args.name}" ` + + (builtIn + ? `now uses the pkc-js built-in challenge of the same name.` + : `rejects every publication and fails to start until that entry is changed.`) + ); // Best-effort reload in the daemon at --pkcRpcUrl, so the removal takes effect // without a restart await reloadChallengesInDaemon(flags.pkcRpcUrl); } + + // A community whose settings.challenges names an unregistered challenge rejects every publication, and pkc-js + // throws on its next start, so it goes offline (issue #176). A name that shadows a pkc-js built-in falls back to + // the built-in instead, which still changes how the community moderates. Ask the daemon which communities use it. + private async _refuseIfInUse(name: string, builtIn: boolean, pkcRpcUrl: string): Promise { + let pkc: Awaited>; + try { + pkc = await this._connectToPkcRpc(pkcRpcUrl); + } catch (err) { + this.error( + `Could not check whether any community uses "${name}": ${err instanceof Error ? err.message : String(err)}\n` + + `Start the daemon and retry, or pass --force to remove it without checking.` + ); + } + const usages = await collectPackageUsages(pkc, (settings) => + findChallengeUsages(settings?.challenges, name).map((usage) => `settings.${usage}`) + ); + if (usages.length === 0) return; + this.error( + `Challenge "${name}" is still used by:\n` + + usages.map((usage) => ` - ${usage}`).join("\n") + + `\nChange or remove those entries first, e.g. bitsocial community edit
--jsonFile ,` + + ` or pass --force to remove it anyway ` + + (builtIn + ? `(after removal, "${name}" falls back to the pkc-js built-in challenge of the same name).` + : `(those communities reject every publication and fail to start until their settings are fixed).`) + ); + } } diff --git a/src/cli/commands/page-sort/remove.ts b/src/cli/commands/page-sort/remove.ts index e5774f3..217df03 100644 --- a/src/cli/commands/page-sort/remove.ts +++ b/src/cli/commands/page-sort/remove.ts @@ -1,7 +1,14 @@ import { Args, Flags } from "@oclif/core"; import { BaseCommand } from "../../base-command.js"; import defaults from "../../../common-utils/defaults.js"; -import { removePackage, PackageNotInstalledError, InvalidPackageNameError, assertValidPackageName } from "../../../common-utils/package-registry.js"; +import { + removePackage, + assertPackageInstalled, + collectPackageUsages, + PackageNotInstalledError, + InvalidPackageNameError, + assertValidPackageName +} from "../../../common-utils/package-registry.js"; import { PAGE_SORT_PACKAGE_KIND, findPageSortUsages, reloadPageSortsInDaemon } from "../../../page-sort-packages/page-sort-utils.js"; export default class Remove extends BaseCommand { @@ -23,7 +30,7 @@ export default class Remove extends BaseCommand { required: false }), force: Flags.boolean({ - description: "Remove even if communities still use it, or if the daemon cannot be reached to check", + description: "Remove without checking whether communities still use it (the daemon is not contacted for the check)", default: false }) }; @@ -37,17 +44,12 @@ export default class Remove extends BaseCommand { const { args, flags } = await this.parse(Remove); const dataPath = flags["pkcOptions.dataPath"] || defaults.PKC_DATA_PATH; - try { - assertValidPackageName(args.name); - } catch (err) { - if (err instanceof InvalidPackageNameError) this.error(err.message); - throw err; - } - - if (!flags.force) await this._refuseIfInUse(args.name, flags.pkcRpcUrl.toString()); - let removed: { version?: string }; try { + assertValidPackageName(args.name); + // Checked before the daemon, so a misspelled name says so rather than that the daemon is down + await assertPackageInstalled(PAGE_SORT_PACKAGE_KIND, { name: args.name, dataPath }); + if (!flags.force) await this._refuseIfInUse(args.name, flags.pkcRpcUrl.toString()); removed = await removePackage(PAGE_SORT_PACKAGE_KIND, { name: args.name, dataPath }); } catch (err) { if (err instanceof PackageNotInstalledError || err instanceof InvalidPackageNameError) this.error(err.message); @@ -55,6 +57,10 @@ export default class Remove extends BaseCommand { } this.log(`removed ${args.name}${removed.version ? `@${removed.version}` : ""}`); + if (flags.force) + this.warn( + `Skipped the in-use check. A community whose settings.pages still names "${args.name}" keeps publishing without this sort.` + ); // Best-effort reload in the daemon at --pkcRpcUrl, so the removal takes effect // without a restart @@ -73,15 +79,9 @@ export default class Remove extends BaseCommand { `Start the daemon and retry, or pass --force to remove it without checking.` ); } - const usages: string[] = []; - try { - for (const address of pkc.communities) { - const community = await pkc.createCommunity({ address }); - for (const usage of findPageSortUsages(community.settings?.pages, name)) usages.push(`${address} settings.pages.${usage}`); - } - } finally { - await pkc.destroy(); - } + const usages = await collectPackageUsages(pkc, (settings) => + findPageSortUsages(settings?.pages, name).map((usage) => `settings.pages.${usage}`) + ); if (usages.length === 0) return; this.error( `Page sort "${name}" is still used by:\n` + diff --git a/src/common-utils/package-registry.ts b/src/common-utils/package-registry.ts index ff8e54c..9de1824 100644 --- a/src/common-utils/package-registry.ts +++ b/src/common-utils/package-registry.ts @@ -257,15 +257,20 @@ export async function installPackage( export class PackageNotInstalledError extends Error {} -/** Remove ${dataPath}//. Returns the removed version, if its package.json was readable. */ -export async function removePackage(kind: PackageKind, { name, dataPath }: { name: string; dataPath: string }): Promise<{ version?: string }> { +/** Throw PackageNotInstalledError unless ${dataPath}// exists. Returns that directory. */ +export async function assertPackageInstalled(kind: PackageKind, { name, dataPath }: { name: string; dataPath: string }): Promise { const packageDir = packageNameToDir(getPackagesDir(kind, dataPath), name); - try { await fs.access(packageDir); } catch { throw new PackageNotInstalledError(`${kind.label[0].toUpperCase()}${kind.label.slice(1)} "${name}" is not installed.`); } + return packageDir; +} + +/** Remove ${dataPath}//. Returns the removed version, if its package.json was readable. */ +export async function removePackage(kind: PackageKind, { name, dataPath }: { name: string; dataPath: string }): Promise<{ version?: string }> { + const packageDir = await assertPackageInstalled(kind, { name, dataPath }); // Read the installed version for the success message (best-effort) let version: string | undefined; @@ -293,6 +298,31 @@ export async function removePackage(kind: PackageKind, { name, dataPath }: { nam return { version }; } +/** The part of a connected pkc-js RPC client that collectPackageUsages reads. */ +export interface PackageUsageSource { + communities: string[]; + createCommunity(options: { address: string }): Promise<{ settings?: unknown }>; + destroy(): Promise; +} + +/** + * Every place a community on the daemon names a package, as "
". `findUsages` maps one community's + * settings to the usages within it, e.g. ["settings.challenges[1]"]. Stopped communities are checked too, since they + * load the package on their next start. Destroys `pkc` when done. + */ +export async function collectPackageUsages(pkc: PackageUsageSource, findUsages: (settings: any) => string[]): Promise { + const usages: string[] = []; + try { + for (const address of pkc.communities) { + const community = await pkc.createCommunity({ address }); + for (const usage of findUsages(community.settings)) usages.push(`${address} ${usage}`); + } + } finally { + await pkc.destroy(); + } + return usages; +} + /** * HTTP url of a daemon's reload endpoint for `kind`, derived from its --pkcRpcUrl. The endpoint * is served by the same http server as the RPC socket, and its local-only variant requires the @@ -413,6 +443,17 @@ export async function importPackageFactory(kind: PackageKind, packageDir: string */ const shadowedByRegistry = new Map>(); +/** + * Whether pkc-js itself ships `name` in the `kind` registry, so removing a package of that name hands the built-in + * back instead of leaving the name unregistered. Holds in the daemon too, where the package may shadow the built-in. + */ +export async function isBuiltInPackageName(kind: PackageKind, name: string): Promise { + const shadowed = shadowedByRegistry.get(kind.registryKey)?.get(name); + if (shadowed) return shadowed.hadPrevious; + const PKC = await import("@pkcprotocol/pkc-js"); + return Object.hasOwn((PKC.default as any)[kind.registryKey] ?? {}, name); +} + export async function loadPackagesIntoPKC(kind: PackageKind, dataPath?: string): Promise { const packages = await listInstalledPackages(kind, dataPath); diff --git a/test/cli/challenge-integration.test.ts b/test/cli/challenge-integration.test.ts index 7293dd4..4d96177 100644 --- a/test/cli/challenge-integration.test.ts +++ b/test/cli/challenge-integration.test.ts @@ -106,6 +106,7 @@ async function publishCommentWithChallenge(opts: { challengeSuccess: boolean; challengeText?: string; challengeErrors?: (string | undefined)[]; + reason?: string; }> { const { pkc, communityAddress, challengeAnswer, timeoutMs = 60000 } = opts; const signer = await pkc.createSigner(); @@ -138,7 +139,8 @@ async function publishCommentWithChallenge(opts: { resolve({ challengeSuccess: verification.challengeSuccess, challengeText, - challengeErrors: verification.challengeErrors + challengeErrors: verification.challengeErrors, + reason: verification.reason }); }); @@ -512,6 +514,65 @@ describe("challenge integration tests", { timeout: 600_000 }, () => { }); }); + // Issue #176: `challenge remove` deletes a package that a community's settings.challenges still names + describe("challenge remove while a community uses the package", () => { + it("refuses and lists every community entry that names the package", { timeout: 180_000 }, async () => { + const name = "test-in-use-challenge"; + const srcDir = path.join(randomDirectory(), name); + await createMinimalChallengeDir(srcDir, name, {}); + const install = await runBitsocialChallenge(["install", srcDir, "--pkcOptions.dataPath", dataPath, "--pkcRpcUrl", rpcWsUrl]); + expect(install.exitCode).toBe(0); + + const sub = await pkc.createCommunity(); + await sub.edit({ settings: { challenges: [{ name: "question", options: { question: "q?", answer: "a" } }, { name }] } }); + + const refused = await runBitsocialChallenge(["remove", name, "--pkcOptions.dataPath", dataPath, "--pkcRpcUrl", rpcWsUrl]); + expect(refused.exitCode).not.toBe(0); + // oclif wraps error output at the terminal width and prefixes each line with "›" ("»" on Windows) + const refusal = refused.stderr.replace(/[›»]/g, " ").replace(/\s+/g, " "); + expect(refusal).toContain(`${sub.address} settings.challenges[1]`); + expect(fsSync.existsSync(path.join(dataPath, "challenges", name))).toBe(true); + }); + + // What the refusal guards against. The package is removed the way `challenge remove` does it (delete the + // directory, then reload the daemon), so this keeps holding once the command refuses + it("a community whose challenge package is gone rejects every publication and cannot start again", { timeout: 180_000 }, async () => { + const name = "test-removed-challenge"; + const srcDir = path.join(randomDirectory(), name); + await createMinimalChallengeDir(srcDir, name, { challenge: "7+7", answer: "14" }); + const install = await runBitsocialChallenge(["install", srcDir, "--pkcOptions.dataPath", dataPath, "--pkcRpcUrl", rpcWsUrl]); + expect(install.exitCode).toBe(0); + + const sub = await pkc.createCommunity(); + await sub.edit({ settings: { challenges: [{ name }] } }); + // Once the package is gone, pkc-js with pkcprotocol/pkc-js#429 emits ERR_COMMUNITY_CHALLENGES_FAILED_TO_LOAD + // on the running community; an unhandled "error" event would throw + sub.on("error", () => {}); + await sub.start(); + expect(await waitForCondition(() => !!(sub as any).updatedAt, 60000, 500)).toBe(true); + + await fsPromise.rm(path.join(dataPath, "challenges", name), { recursive: true }); + const reloadRes = await fetch(`http://localhost:${RPC_PORT}/api/challenges/reload`, { method: "POST" }); + expect(reloadRes.status).toBe(200); + + // The published record still advertises the challenge, but no answer can pass it + expect(sub.challenges).toEqual([{ challenge: "7+7", type: "text/plain" }]); + const result = await publishCommentWithChallenge({ pkc, communityAddress: sub.address, challengeAnswer: "14" }); + expect(result.challengeSuccess).toBe(false); + expect(result.reason).toContain("One of the community challenges is misconfigured"); + + // A restart (or a daemon restart) can't load the challenge, so the community goes offline. pkc-js keeps that + // on purpose but names the error (pkcprotocol/pkc-js#429): once a release with it is installed, start() + // rejects with ERR_COMMUNITY_CHALLENGES_FAILED_TO_LOAD and the publish reason above names the challenge, so + // update these assertions in that bump + await sub.stop(); + const restarted = await pkc.createCommunity({ address: sub.address }); + restarted.on("error", () => {}); + await expect(restarted.start()).rejects.toThrow(/expected function, received undefined/); + expect(await waitForCondition(() => restarted.startedState === "failed", 20000, 200)).toBe(true); + }); + }); + // Issue #164: `page-sort install` on a running daemon registers the package, and the reload pushes the new // settings.pageSorts to already-connected RPC clients, which otherwise reject an unknown name client-side // (ERR_RPC_CLIENT_PAGE_SORT_NAME_NOT_AVAILABLE_ON_SERVER) diff --git a/test/cli/challenge.test.ts b/test/cli/challenge.test.ts index 4e3d8fa..96fd530 100644 --- a/test/cli/challenge.test.ts +++ b/test/cli/challenge.test.ts @@ -1,11 +1,15 @@ import { ChildProcess, spawn, execSync } from "child_process"; -import { describe, it, beforeAll, afterAll, expect } from "vitest"; +import { describe, it, beforeAll, afterAll, afterEach, expect } from "vitest"; +//@ts-ignore +import Sinon from "sinon"; import { directory as randomDirectory } from "tempy"; import fsPromise from "fs/promises"; import fs from "fs"; import path from "path"; import dns from "node:dns"; -import { ensureNpmAvailable } from "../../src/challenge-packages/challenge-utils.js"; +import { ensureNpmAvailable, findChallengeUsages } from "../../src/challenge-packages/challenge-utils.js"; +import { clearPkcRpcConnectOverride, setPkcRpcConnectOverride } from "../helpers/pkc-test-overrides.js"; +import { runCliCommand } from "../helpers/run-cli.js"; dns.setDefaultResultOrder("ipv4first"); // Helper to create a minimal challenge package directory @@ -357,6 +361,12 @@ describe("bitsocial challenge list", () => { }); }); +// No daemon listens on this port, so remove's in-use check cannot connect +const NO_DAEMON_RPC_URL = "ws://127.0.0.1:1"; + +// oclif wraps warnings and errors at the terminal width and prefixes each line with "›" ("»" on Windows) +const unwrap = (output: string) => output.replace(/[›»]/g, " ").replace(/\s+/g, " "); + describe("bitsocial challenge remove", () => { it("refuses a name that escapes the challenges dir instead of deleting another data dir", async () => { const dataPath = randomDirectory(); @@ -369,14 +379,37 @@ describe("bitsocial challenge remove", () => { expect(fs.existsSync(communityDb)).toBe(true); }); - it("removes an installed challenge", async () => { + it("refuses to remove when the daemon cannot be reached to check usage", async () => { + const dataPath = randomDirectory(); + const challengeDir = path.join(dataPath, "challenges", "test-challenge"); + await createMinimalChallengeDir(challengeDir, "test-challenge", { version: "1.0.0" }); + + const result = await runBitsocialChallenge(["remove", "test-challenge", "--pkcOptions.dataPath", dataPath, "--pkcRpcUrl", NO_DAEMON_RPC_URL]); + expect(result.exitCode).not.toBe(0); + expect(unwrap(result.stderr)).toContain('Could not check whether any community uses "test-challenge"'); + expect(result.stderr).toContain("--force"); + expect(fs.existsSync(challengeDir)).toBe(true); + }); + + it("removes with --force without contacting the daemon, and warns that the check was skipped", async () => { const dataPath = randomDirectory(); const challengeDir = path.join(dataPath, "challenges", "test-challenge"); await createMinimalChallengeDir(challengeDir, "test-challenge", { version: "1.0.0" }); - const result = await runBitsocialChallenge(["remove", "test-challenge", "--pkcOptions.dataPath", dataPath]); + const result = await runBitsocialChallenge([ + "remove", + "test-challenge", + "--force", + "--pkcOptions.dataPath", + dataPath, + "--pkcRpcUrl", + NO_DAEMON_RPC_URL + ]); expect(result.exitCode).toBe(0); expect(result.stdout).toContain("removed test-challenge@1.0.0"); + expect(unwrap(result.stderr)).toContain( + 'Skipped the in-use check. A community whose settings.challenges still names "test-challenge" rejects every publication and fails to start' + ); // Verify it's gone await expect(fsPromise.access(challengeDir)).rejects.toThrow(); @@ -390,8 +423,11 @@ describe("bitsocial challenge remove", () => { const result = await runBitsocialChallenge([ "remove", "@test-scope/my-challenge", + "--force", "--pkcOptions.dataPath", - dataPath + dataPath, + "--pkcRpcUrl", + NO_DAEMON_RPC_URL ]); expect(result.exitCode).toBe(0); expect(result.stdout).toContain("removed @test-scope/my-challenge@1.0.0"); @@ -401,12 +437,13 @@ describe("bitsocial challenge remove", () => { await expect(fsPromise.access(scopeDir)).rejects.toThrow(); }); - it("errors on non-existent challenge", async () => { + // Checked before the daemon, so the error names the real problem rather than an unreachable daemon + it("errors on non-existent challenge without contacting the daemon", async () => { const dataPath = randomDirectory(); - const result = await runBitsocialChallenge(["remove", "nonexistent", "--pkcOptions.dataPath", dataPath]); + const result = await runBitsocialChallenge(["remove", "nonexistent", "--pkcOptions.dataPath", dataPath, "--pkcRpcUrl", NO_DAEMON_RPC_URL]); expect(result.exitCode).not.toBe(0); - const combined = `${result.stdout}\n${result.stderr}`; - expect(combined).toContain("not installed"); + expect(result.stderr).toContain('Challenge "nonexistent" is not installed.'); + expect(result.stderr).not.toContain("Could not check"); }); it("works via the uninstall alias", async () => { @@ -414,9 +451,107 @@ describe("bitsocial challenge remove", () => { const challengeDir = path.join(dataPath, "challenges", "test-challenge"); await createMinimalChallengeDir(challengeDir, "test-challenge", { version: "1.0.0" }); - const result = await runBitsocialChallenge(["uninstall", "test-challenge", "--pkcOptions.dataPath", dataPath]); + const result = await runBitsocialChallenge([ + "uninstall", + "test-challenge", + "--force", + "--pkcOptions.dataPath", + dataPath, + "--pkcRpcUrl", + NO_DAEMON_RPC_URL + ]); expect(result.exitCode).toBe(0); expect(result.stdout).toContain("removed test-challenge@1.0.0"); await expect(fsPromise.access(challengeDir)).rejects.toThrow(); }); }); + +// Issue #176: a community whose settings.challenges names a removed package rejects every publication and fails to start +describe("bitsocial challenge remove: in-use check against the daemon", () => { + const sandbox = Sinon.createSandbox(); + + const settingsByAddress: Record = { + "board.bso": { challenges: [{ name: "question", options: { question: "q?", answer: "a" } }, { name: "test-challenge" }] }, + "other.bso": { challenges: [{ name: "test-challenge", exclude: [{ role: ["moderator"] }] }] }, + // A path entry loads that file, not the registered package + "by-path.bso": { challenges: [{ name: "test-challenge", path: "/some/challenge.js" }] }, + "unrelated.bso": {} + }; + + const installAt = async (dataPath: string, name: string) => + createMinimalChallengeDir(path.join(dataPath, "challenges", name), name, { version: "1.0.0" }); + + afterEach(() => clearPkcRpcConnectOverride()); + afterAll(() => sandbox.restore()); + + const fakeDaemon = (settings: Record) => + setPkcRpcConnectOverride( + sandbox.fake.resolves({ + communities: Object.keys(settings), + createCommunity: async ({ address }: { address: string }) => ({ address, settings: settings[address] }), + destroy: async () => {} + }) + ); + + it("findChallengeUsages reports each index that names the package, skipping path entries", () => { + expect(findChallengeUsages([{ name: "a" }, { name: "b" }, { name: "b", path: "/x.js" }, { name: "b" }], "b")).toEqual([ + "challenges[1]", + "challenges[3]" + ]); + expect(findChallengeUsages(undefined, "b")).toEqual([]); + }); + + it("refuses and lists every community entry that names the package", async () => { + const dataPath = randomDirectory(); + await installAt(dataPath, "test-challenge"); + fakeDaemon(settingsByAddress); + const { result } = await runCliCommand(["challenge", "remove", "test-challenge", "--pkcOptions.dataPath", dataPath]); + const message = unwrap(result.error?.message ?? ""); + expect(message).toContain('Challenge "test-challenge" is still used by:'); + expect(message).toContain("board.bso settings.challenges[1]"); + expect(message).toContain("other.bso settings.challenges[0]"); + expect(message).not.toContain("by-path.bso"); + expect(message).not.toContain("unrelated.bso"); + expect(message).toContain("reject every publication and fail to start"); + expect(fs.existsSync(path.join(dataPath, "challenges", "test-challenge"))).toBe(true); + }); + + it("removes when no community uses it", async () => { + const dataPath = randomDirectory(); + await installAt(dataPath, "test-challenge"); + fakeDaemon({ "unrelated.bso": {}, "by-path.bso": settingsByAddress["by-path.bso"] }); + const { result, stdout } = await runCliCommand([ + "challenge", + "remove", + "test-challenge", + "--pkcOptions.dataPath", + dataPath, + "--pkcRpcUrl", + NO_DAEMON_RPC_URL + ]); + expect(result.error).toBeUndefined(); + expect(stdout).toContain("removed test-challenge@1.0.0"); + expect(fs.existsSync(path.join(dataPath, "challenges", "test-challenge"))).toBe(false); + }); + + // Removing a package named after a pkc-js built-in hands the built-in back: nothing breaks, but the community's + // challenge changes, so it still refuses and says what removal would do + it("refuses a package that shadows a pkc-js built-in, and says it would fall back to the built-in", async () => { + const dataPath = randomDirectory(); + await installAt(dataPath, "question"); + fakeDaemon(settingsByAddress); + const { result } = await runCliCommand(["challenge", "remove", "question", "--pkcOptions.dataPath", dataPath]); + const message = unwrap(result.error?.message ?? ""); + expect(message).toContain("board.bso settings.challenges[0]"); + expect(message).toContain('falls back to the pkc-js built-in challenge of the same name'); + expect(fs.existsSync(path.join(dataPath, "challenges", "question"))).toBe(true); + }); + + it("--force on a package that shadows a built-in warns that communities now use the built-in", async () => { + const dataPath = randomDirectory(); + await installAt(dataPath, "question"); + const result = await runBitsocialChallenge(["remove", "question", "--force", "--pkcOptions.dataPath", dataPath, "--pkcRpcUrl", NO_DAEMON_RPC_URL]); + expect(result.exitCode).toBe(0); + expect(unwrap(result.stderr)).toContain('still names "question" now uses the pkc-js built-in challenge of the same name'); + }); +}); diff --git a/test/cli/command-completion-time.test.ts b/test/cli/command-completion-time.test.ts index 0cacdc9..de653f6 100644 --- a/test/cli/command-completion-time.test.ts +++ b/test/cli/command-completion-time.test.ts @@ -325,9 +325,10 @@ describe("challenge commands complete within 10s", () => { expect(result.stdout).toContain("test-challenge"); }); - it("challenge remove completes within 10s", { timeout: 30_000 }, async () => { + // No daemon runs here, so without --force remove would wait on the in-use check's connection (issue #176) + it("challenge remove --force completes within 10s", { timeout: 30_000 }, async () => { const result = await runBitsocialCommand( - ["challenge", "remove", "test-challenge", "--pkcOptions.dataPath", dataPath] + ["challenge", "remove", "test-challenge", "--force", "--pkcOptions.dataPath", dataPath] ); expect(result.exitCode, `stderr: ${result.stderr}\nstdout: ${result.stdout}`).toBe(0); expect(result.stdout).toContain("removed test-challenge@1.0.0"); diff --git a/test/cli/page-sort.test.ts b/test/cli/page-sort.test.ts index 287b088..bc04358 100644 --- a/test/cli/page-sort.test.ts +++ b/test/cli/page-sort.test.ts @@ -173,13 +173,19 @@ describe("bitsocial page-sort install / list / remove", () => { ]); expect(remove.exitCode).toBe(0); expect(remove.stdout).toContain("removed @test-scope/scoped-page-sort@2.0.0"); + // oclif wraps warnings at the terminal width and prefixes each line with "›" ("»" on Windows) + expect(remove.stderr.replace(/[›»]/g, " ").replace(/\s+/g, " ")).toContain( + 'Skipped the in-use check. A community whose settings.pages still names "@test-scope/scoped-page-sort" keeps publishing without this sort.' + ); expect(fs.existsSync(path.join(dataPath, "page-sorts", "@test-scope"))).toBe(false); }); - it("errors on a page sort that is not installed", async () => { - const remove = await runBitsocialPageSort(["remove", "nope", "--force", "--pkcOptions.dataPath", randomDirectory()]); + // Checked before the daemon, so the error names the real problem rather than an unreachable daemon + it("errors on a page sort that is not installed without contacting the daemon", async () => { + const remove = await runBitsocialPageSort(["remove", "nope", "--pkcOptions.dataPath", randomDirectory(), "--pkcRpcUrl", NO_DAEMON_RPC_URL]); expect(remove.exitCode).not.toBe(0); expect(remove.stderr).toContain('Page sort "nope" is not installed.'); + expect(remove.stderr).not.toContain("Could not check"); }); });