Skip to content

fix: make docker smoke CI-runnable and side-by-side safe#6

Merged
ethanj merged 3 commits intomainfrom
fix/docker-smoke-ci-safe
Apr 16, 2026
Merged

fix: make docker smoke CI-runnable and side-by-side safe#6
ethanj merged 3 commits intomainfrom
fix/docker-smoke-ci-safe

Conversation

@ethanj
Copy link
Copy Markdown
Contributor

@ethanj ethanj commented Apr 16, 2026

Summary

  • make docker smoke choose safe host ports for side-by-side/local CI use
  • align smoke test with the real stats route contract
  • remove smoke compose port duplication so script-selected ports take effect

Validation

  • npm run test:docker-smoke reported all passed on branch work before selective commit
  • commit: 73a6f90
  • branch: fix/docker-smoke-ci-safe

ethanj and others added 3 commits April 15, 2026 22:42
Five functional gaps and one test file identified by PR #3 audit:

1. docker-compose.yml: add restart: unless-stopped to postgres service
   (matches app restart policy; prevents DB death from crashing the stack)

2. src/db/repository-read.ts: add sourceSite + episodeId optional filters
   to listMemories() — dynamic parameterized WHERE clauses

3. src/db/memory-repository.ts: thread sourceSite + episodeId through
   repository wrapper

4. src/services/memory-crud.ts + memory-service.ts: thread the same
   params through the crud helper and service facade list() method

5. src/services/memory-ingest.ts: new performStoreVerbatim() — stores
   content as a single memory without fact extraction, for user-created
   text/file uploads

6. src/services/memory-service.ts: add storeVerbatim() facade method

7. src/routes/memories.ts:
   - Add UUID_REGEX constant
   - Add requireUuidParam() and optionalUuidQuery() helpers
   - UUID-validate /:id param on GET, DELETE, and /:id/audit routes
     (was previously letting "not-a-uuid" reach the DB as 500)
   - Add source_site and episode_id filters to GET /list
   - Add skip_extraction branch to POST /ingest/quick routing to
     storeVerbatim instead of quickIngest

8. src/__tests__/route-validation.test.ts: new 129-line test file
   covering UUID validation, filter behavior, and skip_extraction path.
   Mocks embedText to avoid hitting the real embedding provider with
   the CI placeholder OPENAI_API_KEY.

Validation:
- npx tsc --noEmit: clean
- pnpm test: 876 passing, 0 failed (was 869; +7 new tests)
- npx fallow --no-cache: 0 above threshold, maintainability 91.0

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
PR #2 merged into research main on 2026-04-14 ~09 UTC, but the core
extraction (7a9b2d5) branched from research commit 3ac0471 which
predates PR #2's merge commit 791c68e. Verified via
`git merge-base --is-ancestor 791c68e 3ac0471` → not ancestor.

Two functional/quality gaps ported:

1. src/services/memory-ingest.ts — fast-AUDN dedup now returns the
   existing memory ID on skip instead of null. Integration sync
   callers rely on this to link to the canonical memory when the
   ingested fact is a near-duplicate.

2. src/routes/route-errors.ts — server-side logging improvements:
   - String(err ?? 'Internal server error') coercion for non-Error
     throwables
   - [status] prefix in the log line for quick severity scanning
   - Full stack trace logged when available

Deliberately NOT ported: PR #2's change to expose err.message to
clients for 500s. Core's consolidated handler returns a generic
'Internal server error' which is the safer default and doesn't leak
internals. Only the server-side log line gets richer.

Validation:
- npx tsc --noEmit: clean
- pnpm test: 876/876 passing
- npx fallow --no-cache: 0 above threshold, maintainability 91.0

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@ethanj ethanj merged commit a5bef7c into main Apr 16, 2026
1 check failed
@ethanj ethanj deleted the fix/docker-smoke-ci-safe branch April 16, 2026 07:49
ethanj added a commit that referenced this pull request Apr 16, 2026
PR #6 changed docker-compose.yml ports from "3050:3050" to
"${APP_PORT:-3050}:3050" for side-by-side CI safety. The regex in
deployment-config.test.ts:105 was written for the bare-digit shape and
silently failed on the env-var substitution form, leaving the app-port
exposure assertion broken since that PR landed.

Update the assertion to use a new composePortBindingRegex(internalPort)
helper that accepts either a literal external port or a
${VAR:-default} shell-variable substitution. The helper makes the
pattern intent explicit and prevents the same drift from recurring on
future compose changes.

Restores the deployment-config test suite to a clean baseline (15/15
pass). Recommended in the Phase 1A follow-up analysis as the smallest
move to restore baseline integrity before Phase 1B.

Known follow-up: src/__tests__/route-validation.test.ts
"skip_extraction (storeVerbatim)" returns 500 in some local
environments. Out of scope for this fix; investigate separately before
declaring full suite green.

Validation:
- npx tsc --noEmit: clean
- npx vitest run src/__tests__/deployment-config.test.ts: 15/15 pass
- fallow --no-cache: 0 above threshold, maintainability 90.9 unchanged

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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.

1 participant