Repository navigation
fix: harden setup, operations, and readiness - #20
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2c956864c8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (disabledPlugins.has(name)) continue | ||
| if (registry.get(name)) continue | ||
| try { | ||
| const mod = await importPackage(name, projectDir) |
There was a problem hiding this comment.
Normalize legacy plugin identifiers before importing
Existing configurations generated by the pre-change init command contain identifiers such as @opendocuments/parser-pdf, while the installable package is named opendocuments-parser-pdf. Since bootstrap now imports config.plugins directly instead of unconditionally loading the installed parser packages, upgraded developer/enterprise/all projects log module-not-found errors and lose PDF, DOCX, XLSX, or code parsing. Normalize the legacy identifiers or retain a compatibility alias before calling importPackage.
AGENTS.md reference: AGENTS.md:L165-L167
Useful? React with 👍 / 👎.
| const configuredWorkspace = existsSync(currentWorkspacePath) | ||
| ? readFileSync(currentWorkspacePath, 'utf-8').trim() | ||
| : undefined | ||
| cachedCtx = await bootstrap({ | ||
| ...(configuredWorkspace ? { configOverrides: { workspace: configuredWorkspace } } : {}), |
There was a problem hiding this comment.
Scope workspace selection to the current project
When a user switches workspace in project A and then runs any CLI command in project B, this process-wide ~/.opendocuments/current-workspace value overrides project B's configured workspace. Bootstrap then calls ensure() for that unrelated name, silently creates it in B's database, and indexes or searches data in the wrong workspace. Store the selection per instance/data directory, or only apply it after confirming it belongs to the current project.
AGENTS.md reference: AGENTS.md:L302-L302
Useful? React with 👍 / 👎.
| const record = readServerPid(pidFile) | ||
| if (!record || !isRecordedServerProcess(record)) return | ||
| throw new Error('OpenDocuments server is running. Run "opendocuments stop" before backup or restore.') |
There was a problem hiding this comment.
Fail closed on an unreadable PID record
If server.pid exists but is malformed—particularly the plain numeric PID written by the previous CLI—this treats the instance as stopped and proceeds to copy or replace SQLite, WAL, and LanceDB files while that older server may still be running. That can produce an inconsistent backup or corrupt an active restore; unlike a genuinely absent or verified-stale PID, an unverifiable record must abort with the same actionable stop instruction.
AGENTS.md reference: AGENTS.md:L89-L93
Useful? React with 👍 / 👎.
| OPENDOCUMENTS_MODEL_LLM: ${OPENDOCUMENTS_MODEL_LLM:-qwen2.5:14b} | ||
| OPENDOCUMENTS_MODEL_EMBEDDING: ${OPENDOCUMENTS_MODEL_EMBEDDING:-mxbai-embed-large} |
There was a problem hiding this comment.
Avoid Ollama defaults for cloud Compose providers
When users run the newly documented OpenAI Compose command, they override only provider, base URL, and key, so these defaults still send qwen2.5:14b and mxbai-embed-large to OpenAI. The startup embedding probe then fails and the server enters degraded mode. Require provider-appropriate model variables for cloud deployments or make these defaults conditional instead of applying Ollama model names to every provider.
Useful? React with 👍 / 👎.
| const rawKey = ( | ||
| opts.ipOnly ? undefined : c.req.header('x-api-key') | ||
| ) || getClientIp(c, opts.trustedProxy) || 'anonymous' |
There was a problem hiding this comment.
Rate-limit cookie sessions by their authenticated key
For the new browser and OAuth sessions, requests carry opendocuments_session rather than X-API-Key, so this post-auth limiter falls back to the client IP even though c.get('auth') identifies a unique key. All Web UI users behind the same reverse proxy or corporate NAT therefore share the 60-request bucket, and one user's configured per-key limit is applied against the shared count. Use the validated record ID or session key for the non-ipOnly limiter.
Useful? React with 👍 / 👎.
| "@vitejs/plugin-react": "^6.0.4", | ||
| "autoprefixer": "^10.4.0", | ||
| "postcss": "^8.4.0", | ||
| "tailwindcss": "^3.4.0", | ||
| "typescript": "^5.5.0", | ||
| "vite": "^6.0.0" | ||
| "vite": "^8.1.5" |
There was a problem hiding this comment.
Keep the web toolchain compatible with Node 20.0
The repository advertises Node.js >=20.0.0, but the lockfile resolves this dependency to Vite 8.1.5, whose engine is ^20.19.0 || >=22.12.0; @vitejs/plugin-react 6 has the same restriction. Consequently supported Node 20.0–20.18 environments fail installation under engine-strict or cannot reliably build the CLI's now-required bundled Web UI. Retain a compatible Vite version or raise the repository engine floor and CI coverage.
AGENTS.md reference: AGENTS.md:L9-L12
Useful? React with 👍 / 👎.
What changed
Why
Installation and day-two operations could previously report healthy states while models or corpora were unusable, retain reusable browser credentials, or leave restore and process-management edge cases unsafe.
Impact
Operators now get explicit readiness gates, recoverable maintenance workflows, safer session and connector handling, and deployment instructions that match actual runtime behavior.
Validation
npm ci(0 vulnerabilities)npm run build(26/26 tasks)npm run typecheck(50/50 tasks)npm test(51/51 tasks, 648 tests)npm run build --prefix docs-sitedocker compose configgit diff --check