Skip to content

fix(db): use http:// for private couchdb internal endpoint URL - #146

Merged
birme merged 1 commit into
mainfrom
fix/couchdb-internal-url-protocol
Jul 17, 2026
Merged

birme merged 1 commit into
mainfrom
fix/couchdb-internal-url-protocol

Conversation

@birme

@birme birme commented Jul 17, 2026

Copy link
Copy Markdown
Member

Summary

setupDatabase() in @osaas/client-db built an https:// connection URL for CouchDB's internal (publicAccess: false) endpoint by reusing DatabaseTypeToProtocol['couchdb'], which is 'https' and correct only for the public ingress URL (port 443, TLS terminated at the ingress). The internal container port 5984 speaks plaintext HTTP — there is no in-cluster TLS termination on that port — so any consumer given this URL fails with a TLS-handshake-on-plaintext-port error.

Fix

  • Added a separate DatabaseTypeToInternalProtocol lookup (currently only overriding couchdb -> http) so the internal-endpoint branch for CouchDB emits http:// instead of reusing the ingress-only DatabaseTypeToProtocol constant.
  • DatabaseTypeToProtocol['couchdb'] is unchanged ('https') since it is still correct for the public path.
  • Other database types (postgres, mysql, redis, clickhouse) are unaffected — their internal paths already use non-HTTP schemes.
  • Added packages/db/src/index.test.ts asserting: private CouchDB URL starts with http://, public CouchDB URL starts with https://.

Versioning

I did not manually bump packages/db/package.json. I checked the history of prior bug-fix commits touching this file (e.g. bb115de "fix: clickhouse js client expect http url") and none of them touched package.json — this repo bumps versions separately via lerna version --conventional-commits in dedicated chore(release): publish commits, driven by the fix:/feat: commit prefix. This commit uses a fix: prefix so it will be picked up as a patch bump by that process.

Fixes #145

Lessons

The repo's packages/*/lib build output is gitignored but was stale in this checkout (missing getInternalEndpoint in @osaas/client-core's compiled lib/, even though it exists in src/), which made the new Jest test fail to type-check until packages/core was rebuilt with tsc. Also, despite the task brief suggesting a version bump, this repo's actual convention (verified via git log on prior fix commits) is to never bump package.json in feature/fix PRs — versioning is automated by lerna version --conventional-commits in separate release commits.

🤖 Generated with Claude Code

Co-Authored-By: Claude noreply@anthropic.com

setupDatabase() built an https:// connection URL for CouchDB's internal
(publicAccess: false) endpoint by reusing DatabaseTypeToProtocol['couchdb'],
which is 'https' and correct only for the public ingress URL (port 443,
TLS-terminated at ingress). The internal container port 5984 speaks plain
HTTP with no in-cluster TLS termination, so consumers got a TLS-handshake-
on-plaintext-port failure.

Introduce a separate DatabaseTypeToInternalProtocol lookup so the internal
CouchDB path emits http:// while the public path keeps the unchanged
DatabaseTypeToProtocol['couchdb'] = 'https' value. Other database types are
unaffected since their internal paths already use non-HTTP schemes.

Fixes #145

Co-Authored-By: Claude <noreply@anthropic.com>
@birme

birme commented Jul 17, 2026

Copy link
Copy Markdown
Member Author

Research Review

Verified

  • Internal endpoint shape: The test mock for getInternalEndpoint ({ serviceDns, ports: [{ name, port, protocol }], publicAccess }) matches the real InternalEndpointInfo interface in packages/core/src/core.ts:455-458 exactly.
  • CouchDB instance shape: AdminPassword and url fields used in the fix and test mocks match the generated client packages/services/src/generated/apache-couchdb.ts (AdminPassword: string, url: string).
  • Scoping: Only the couchdb internal branch (packages/db/src/index.ts:278) was changed to use the new DatabaseTypeToInternalProtocol['couchdb']. The valkey/postgres/mariadb internal branch (line 237) and the clickhouse internal branch (line 304) still correctly use DatabaseTypeToProtocol[type] — confirmed unchanged via grep -n "DatabaseTypeToProtocol" index.ts. The public couchdb branch (line 284-287) doesn't reference either protocol constant at all (parses instance.url directly via new URL()), so it's untouched and correct.
  • DatabaseTypeToProtocol['couchdb'] value: confirmed unchanged ('https'), still exported for any external consumer that reads the map directly.
  • Test actually exercises the bug, not a trivially-true assertion: I checked out the branch, reverted just the one-line fix (DatabaseTypeToInternalProtocol['couchdb'] → DatabaseTypeToProtocol['couchdb']), and reran the test suite — the private-URL test fails against pre-fix code (Expected pattern: /^http:\/\// / Received: "https://...") and passes once the fix is restored. This confirms the test is a real regression guard, not tautological.
  • CI: gh pr checks 146 shows the ts workflow passing.
  • Versioning claim: Confirmed no other fix:-prefixed commit touching packages/db/src/index.ts (e.g. bb115de "clickhouse js client expect http url") bumps package.json; version bumps only appear in separate chore(release): publish commits. The PR correctly leaves package.json untouched.
  • Real-world impact confirmed: osaas-ai's create-database MCP tool (src/mcp/tools/database.ts) explicitly instructs the model to prefer publicAccess: false for My Apps databases, and osaas-service-builder's database-manager.ts uses couchdb (mongodb: 'couchdb') for document storage — both are real, currently-shipping call paths that would hit this bug with publicAccess: false + couchdb.
  • PR body: ## Lessons section is present and filled with a genuinely non-obvious finding (stale packages/core/lib build output required a rebuild for the new test to type-check, plus the verified no-package.json-bump convention). Both required trailers (🤖 Generated with [Claude Code] and Co-Authored-By: Claude <noreply@anthropic.com>) are present in the PR body, and the commit message also carries the co-author trailer.

Adjacent Stubs Detected

None. Swept packages/db/src/index.ts and packages/db/src/index.test.ts for undefined : undefined, || undefined, TODO/FIXME/XXX/HACK, dead-code comments, empty catch blocks, and console.log-only handlers. The only console.log hits are inside a JSDoc @example block (illustrative code, not a real code path) — not a stub.

Orphaned Files Detected

Not applicable — packages/db/src/index.ts is the package's published entry point (@osaas/client-db main module) and index.test.ts is a Jest test file; both are effectively "entry points" for this check.

Impact Analysis

  • Blast radius: Any consumer of @osaas/client-db's setupDatabase(ctx, 'couchdb', name, { publicAccess: false, ... }). Confirmed real consumers: osaas-ai (create-database MCP tool, actively steers users toward publicAccess: false for My Apps), osaas-service-builder (couchdb-as-document-store path), plus demo-video-platform-orchestrator and the client-ts CLI package which import @osaas/client-db (not confirmed whether they hit the couchdb+private path specifically).
  • Downstream consumers: No source changes required in consumers — this is a bug fix within the library's existing contract (setupDatabase return type is still string), so it's a drop-in patch once consumers bump @osaas/client-db.
  • Coordinated changes needed: None for merge. After release, consumers pinned to older @osaas/client-db versions won't get the fix until they bump — worth flagging to osaas-ai/osaas-service-builder maintainers post-release, but not a blocker for this PR.
  • Breaking change risk: Low. Only the previously-broken (TLS-handshake-failing) internal couchdb URL changes shape; no public API signature changes.

Risks Not Tested

  • Live-cluster behavior (i.e., that a real internal CouchDB container on port 5984 actually accepts plaintext HTTP rather than some other protocol nuance) was not verified against a running cluster — only against the documented/assumed behavior described in the issue and PR body.
  • No verification that other consumers (demo-video-platform-orchestrator, client-ts/packages/cli) exercise the couchdb + publicAccess: false path today; blast radius for those two is inferred from import presence only, not confirmed usage.

Research Suggestions

  • No action needed before merge. Post-release, consider a quick check that osaas-ai and osaas-service-builder pick up the patched @osaas/client-db version in their next dependency bump, since both actively use the previously-broken path.

Reviewed by PR Research Validator. This review checks whether code is based on verified sources rather than assumptions.

@birme
birme merged commit 845ce90 into main Jul 17, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

setupDatabase builds https:// URL on internal CouchDB port 5984 (TLS on a plaintext port) for private (publicAccess:false) instances

1 participant