mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-01 06:42:06 +01:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Managed Cloud instances must retain shared objects and secrets across server replacement. > - Implicit storage defaults can select local disk or an unintended bucket. > - Cloud runtime settings now come from a canonical database snapshot. > - This pull request validates bootstrap prerequisites before database setup and effective S3 settings after snapshot resolution. > - The benefit is an actionable refusal before local signing state or HTTP setup, while valid canonical Cloud settings work. ## Linked Issues or Issue Description **What happened?** A managed Cloud instance could start with local disk storage, an implicit bucket, or a local key file. After canonical Cloud configuration landed, checking only environment or file values would also reject a valid database bucket. Shared schema defaults could make an omitted raw bucket appear explicit. **Expected behavior** Managed Cloud requires external PostgreSQL, a stable environment master key, and effective S3 settings with a nonblank bucket. The canonical snapshot remains the configuration authority. Invalid configuration refuses startup before signing or HTTP effects. Self-hosted defaults remain compatible. **Steps to reproduce** 1. Select managed Cloud with its tenant token or managed configuration document. 2. Leave storage as local disk, or omit the S3 bucket from a raw canonical snapshot. 3. Start the server. Startup must refuse. An explicit valid bucket in the canonical snapshot must work without a file or environment bucket. **Paperclip version or commit** Current base: `4edd10804e2b42941aa2038e90c362c13a0b98dc`. **Deployment mode** Managed Cloud. Ordinary self-hosted defaults remain unchanged. Related PRs: #18335 owns canonical Cloud runtime configuration. #18327 owns S3 run logs. #18352 owns credential persistence. #18347 owns cutover checks. This PR adds configuration prerequisites to those existing boundaries. ## What Changed - Validate external PostgreSQL and a stable environment master key at the Cloud bootstrap boundary, before database setup. - Validate effective S3 settings after canonical snapshot publication and before Cloud signing, runtime consumers, or HTTP setup. - Require a nonblank bucket in the Cloud snapshot schema. Reject omitted or blank buckets in every legacy Cloud file before one-time import normalization, even when an environment bucket is supplied. Keep shared self-hosted schema defaults. - Preserve typed bucket provenance at the existing file-read boundary. Accept explicit canonical bucket names, including `paperclip`, and environment settings when the snapshot is null. - Validate effective legacy storage with the existing resolver before seeding. A rejected candidate leaves no canonical rows, so a corrected source file can be imported on retry. Environment overlays are checked but never persisted. - Add real PostgreSQL snapshot and migration-retry tests, startup refusal tests, and environment/file compatibility controls. Document that Cloud always requires its stable environment key, even with an external default secrets provider. ## Verification - Latest focused acceptance passed 199/199 in six files (54.29 seconds), using the command below plus `server/src/__tests__/cloud-configuration-migration-postgres.test.ts`. Real PostgreSQL proves rejected storage leaves all three migration authority rows absent, then a corrected legacy file imports on retry. An external-default-secrets control proves the deliberate unconditional Cloud master-key contract. - Before the peer correction, the frozen product diff passed 178 tests in five files: managed-cloud-prerequisites, cloud-runtime-config, server-startup-feedback-export, config-file, and cloud-configuration-readers. Command: `pnpm exec vitest run server/src/__tests__/{managed-cloud-prerequisites,cloud-runtime-config,server-startup-feedback-export,config-file,cloud-configuration-readers}.test.ts --maxWorkers=1 --no-file-parallelism`. Real PostgreSQL used a qualified POSIX test-only PG18.6 override because the host cannot supply the stock runtime's SysV semaphore allocation. - Coverage includes raw database and legacy omitted/blank bucket rejection, valid database and environment buckets, null snapshots, explicit default-named buckets, self-hosted compatibility, and refusal before local signing mutation. - The real AWS SDK configured-provider control uses a simulated loopback S3 service. It writes, reads from a fresh provider, and deletes an object. It also reads an encrypted credential with the stable environment key. This does not qualify production cloud permissions. - `git diff --check` passed. Final merges add unrelated master changes from #18241, #18363, and #18370; the focused storage product diff is unchanged. - Peer review reproduced a fresh legacy local-disk import that gained the default bucket before an S3 environment overlay. The importer now requires a raw explicit bucket for every legacy Cloud file. Six controls cover omission/blank values with and without an environment bucket; the full focused rerun passed 184/184 across all five files in 194.89 seconds. - Initial revised startup testing passed 167/168: one fixture omitted the database URL and failed before its intended key assertion. The corrected final run passed 178/178. An initial direct server typecheck failed because Runner declarations were not built; this is a setup failure, not a compile pass. Integration verification rebuilds dependencies before full checks. - Prior head `fae1e4aa` passed complete CI with 53 successes, two intended skips, and Greptile 5/5. `de71345f379cecb71dbfb219f35e14f942c10a3f` then passed complete CI (55 successes, three workflow skips, and a neutral external review), with Greptile 5/5 and zero unresolved threads. The external review identified the preseed rejection problem; its correction is committed at `75e04d0d77e19dcef0a8cb070b6b518f0f173b14`. The final merge head is `19b809ea2823a6a4e5ceb48ef2c2bac4721733d6`; [Fresh complete CI](https://github.com/paperclipai/paperclip/actions/runs/41127309762) passed on that head: 55 successful checks, three workflow skips, and one neutral external review. Greptile reviewed this exact head at 5/5 with no actionable issues. Both earlier external review threads are resolved with the tested fixes. Earlier infrastructure retries remain documented in [the first diagnosis](https://github.com/paperclipai/paperclip/pull/18350#issuecomment-6529964690) and [the runner shutdown evidence](https://github.com/paperclipai/paperclip/pull/18350#issuecomment-6530364769). Earlier missing SDK build and fixture metadata failures were repaired before those passes. - Current frozen-head CI first attempt failed one unchanged Antigravity authentication success case in Runner Vitest 1/2 (`outcome=failure`, 1994 tests passed). The same file passed 11/11 locally in 2.33 seconds, and identical Runner source passed the prior complete CI run. Attempt 2 retried only failed jobs (the Runner shard and dependent aggregate). Both passed without a source change. No second retry was used. The exact timing cause is not established. [Failed job](https://github.com/paperclipai/paperclip/actions/runs/41127309762/job/124309181778). - No production data, buckets, or live credentials were used. Complete local repository suites are not duplicated in this worktree; current-head required CI supplies the full suite gate. ## Risks - Managed deployments with local disk, omitted buckets, file-only keys, or missing external database URLs now refuse startup. Operators must supply valid prerequisites before rollout. - Required deployment environment overlays must remain stable across replacements. For example, a saved local-disk provider with an explicit bucket can use an environment S3 provider; removing that required overlay correctly refuses startup until the environment or canonical configuration is corrected. Environment settings are never silently copied into the canonical row. - Effective legacy storage must also pass before first seeding. A bad candidate does not create authority rows. Existing saved rows remain authoritative and can require environment overrides or a canonical configuration change if they are invalid. - A present legacy Cloud file needs an explicit raw bucket even if its saved provider is local disk or its environment supplies a bucket. Correct the file before one-time import, or use the existing absent-file/null-snapshot environment-only setup. Environment overlays are not persisted as operator choices. - A previously stored canonical nonblank `paperclip` bucket is authoritative. Prior normalization erased whether that value was originally explicit. This change cannot recover that provenance and does not reject existing nonblank canonical values. - Validation proves configuration shape, not cloud permissions, database reachability, or key compatibility with existing encrypted data. Keep the master key stable across replacements. - Cloud snapshot JSON parsing is stricter; omitted or blank raw S3 bucket values need correction. No database migration or provider operation changes are added. These prerequisites do not prove that every server persistence path is disposable. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used OpenAI Codex, exact model ID `gpt-6.1-sol`. Context window: unknown. The model used reasoning, repository tools, code execution, and test execution. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing>
129 lines
3.6 KiB
TypeScript
129 lines
3.6 KiB
TypeScript
import fs from "node:fs";
|
|
import os from "node:os";
|
|
import path from "node:path";
|
|
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
|
import { readConfigFile } from "../config-file.js";
|
|
|
|
const ORIGINAL_PAPERCLIP_CONFIG = process.env.PAPERCLIP_CONFIG;
|
|
|
|
function writeConfig(configPath: string, value: unknown): void {
|
|
fs.writeFileSync(configPath, `${JSON.stringify(value, null, 2)}\n`);
|
|
}
|
|
|
|
function escapeRegExp(value: string): string {
|
|
return value.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
|
|
}
|
|
|
|
function minimalConfig(): unknown {
|
|
return {
|
|
$meta: {
|
|
version: 1,
|
|
updatedAt: "2026-07-05T00:00:00.000Z",
|
|
source: "configure",
|
|
},
|
|
database: {
|
|
mode: "embedded-postgres",
|
|
},
|
|
logging: {
|
|
mode: "file",
|
|
},
|
|
server: {},
|
|
};
|
|
}
|
|
|
|
describe("readConfigFile", () => {
|
|
let tempDir: string;
|
|
let configPath: string;
|
|
|
|
beforeEach(() => {
|
|
tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "paperclip-config-file-test-"));
|
|
configPath = path.join(tempDir, "config.json");
|
|
process.env.PAPERCLIP_CONFIG = configPath;
|
|
});
|
|
|
|
afterEach(() => {
|
|
vi.restoreAllMocks();
|
|
if (ORIGINAL_PAPERCLIP_CONFIG === undefined) {
|
|
delete process.env.PAPERCLIP_CONFIG;
|
|
} else {
|
|
process.env.PAPERCLIP_CONFIG = ORIGINAL_PAPERCLIP_CONFIG;
|
|
}
|
|
|
|
fs.rmSync(tempDir, { recursive: true, force: true });
|
|
});
|
|
|
|
it("returns null when the config file does not exist", () => {
|
|
expect(readConfigFile()).toBeNull();
|
|
});
|
|
|
|
it("throws a path-specific error when the config file is invalid JSON", () => {
|
|
fs.writeFileSync(configPath, "{");
|
|
|
|
expect(() => readConfigFile()).toThrow(
|
|
new RegExp(`Invalid Paperclip config at ${escapeRegExp(configPath)}: failed to read or parse JSON`),
|
|
);
|
|
});
|
|
|
|
it("throws a field-specific error when the config file fails schema validation", () => {
|
|
const config = minimalConfig();
|
|
if (typeof config === "object" && config !== null) {
|
|
(config as { $meta: { source: string } }).$meta.source = "edited-by-hand";
|
|
}
|
|
|
|
writeConfig(configPath, config);
|
|
|
|
expect(() => readConfigFile()).toThrow(/Invalid Paperclip config .* \$meta\.source:/);
|
|
});
|
|
|
|
it("parses a valid config file", () => {
|
|
writeConfig(configPath, minimalConfig());
|
|
|
|
expect(readConfigFile()).toMatchObject({
|
|
$meta: {
|
|
source: "configure",
|
|
},
|
|
database: {
|
|
mode: "embedded-postgres",
|
|
},
|
|
logging: {
|
|
mode: "file",
|
|
},
|
|
});
|
|
});
|
|
|
|
it("preserves explicit bucket provenance without changing defaults for existing readers", () => {
|
|
for (const bucket of [undefined, "paperclip"]) {
|
|
writeConfig(configPath, {
|
|
...minimalConfig() as object,
|
|
storage: { provider: "s3", s3: bucket === undefined ? {} : { bucket } },
|
|
});
|
|
expect(readConfigFile()?.storage.s3.bucket).toBe("paperclip");
|
|
expect(readConfigFile({ includeProvenance: true })).toMatchObject({
|
|
config: { storage: { s3: { bucket: "paperclip" } } },
|
|
explicitStorageS3Bucket: bucket !== undefined,
|
|
});
|
|
}
|
|
});
|
|
|
|
it("warns about likely misspellings without stripping them", () => {
|
|
const config = {
|
|
...(minimalConfig() as Record<string, unknown>),
|
|
server: {
|
|
ports: 3200,
|
|
},
|
|
};
|
|
const warn = vi.spyOn(console, "warn").mockImplementation(() => undefined);
|
|
writeConfig(configPath, config);
|
|
|
|
expect(readConfigFile()).toMatchObject({
|
|
server: {
|
|
port: 3100,
|
|
ports: 3200,
|
|
},
|
|
});
|
|
expect(warn).toHaveBeenCalledWith(
|
|
"Unknown config key server.ports; did you mean server.port? It will be preserved.",
|
|
);
|
|
});
|
|
});
|