Skip to content

ci: run the test suite on every push and pull request - #10

Closed
joshberen wants to merge 6 commits into
risunCode:devfrom
joshberen:ci/run-test-suite
Closed

joshberen wants to merge 6 commits into
risunCode:devfrom
joshberen:ci/run-test-suite

Conversation

@joshberen

@joshberen joshberen commented Oct 14, 2026 •

Copy link
Copy Markdown

Runs the test suite on push and pull request, against disposable PostgreSQL and Redis services.

Closed by the author.

`homeDir` re-exported `node:os`'s `homedir()`, which Bun resolves once and
freezes for the process lifetime. The injector tests redirect `HOME` to a temp
directory — the seam this module's own header documents — so every injector
kept reading and writing the operator's real home directory instead. The codex
lifecycle suite failed on a stale real `~/.codex`, and its `apply` wrote the
fixture config into that directory.

Resolve `HOME`/`USERPROFILE` on each call and keep `homedir()` as the fallback
for a process started without either variable.
Three defects kept `bun run test` red on a clean checkout, and two of them hid
each other.

`requireDatabase()` registered a `beforeAll` that called `getTestPool()`
unconditionally, so a machine without `CARTETHYIA_TEST_DATABASE_URL` failed two
suites instead of skipping them — the opposite of this module's promise that an
unconfigured service skips with a stated reason. The suites that call it were
plain `describe`, so they were not skipped either; they now use the `dbDescribe`
primitive that already existed for exactly this.

`.env.test` and `.env.test.example` pointed at
`postgres://postgres@localhost:5432/...`. `pg_hba` requires scram over TCP, so
the passwordless URL failed all twelve database suites at connect time. Both now
carry a dedicated `cartethyia_test` role, and `.env.test.example` uses the
variable name the suites and `test-db:check` actually read — it held the retired
`TEST_DATABASE_URL`, the exact mismatch `test-db-check.ts` documents.

Verified on a local PostgreSQL 18: 1992 backend + 375 dashboard tests pass with
the database reachable, and the same run is green with 142 stated skips when it
is not.
…ktop path

`dropCorruptAttachments`'s two capture-based tests read
`C:\Users\Aria\Desktop\awok..txt`, an absolute path to the author's desktop that
never entered the repository. They cannot pass anywhere else, so the suite was
red on every other machine.

The defect they pinned — an IDAT chunk whose stored CRC disagrees with its bytes
— is already reproduced from this file's PNG primitives by the
`png_crc_mismatch` test, and the well-formed counterpart by the valid 1x1
fixture. The rationale moves onto that test so the regression's origin is not
lost with the file path.
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ⚠️ Failed 2026-10-14T04:42:51.463129Z e6d8db3 PR opened
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

`bun run test` failed intermittently with `40P01 deadlock detected`. The server
log names both sides: a migration suite's `ALTER TABLE ... ADD COLUMN` holds
`provider_routing_settings` and waits for `provider_accounts`, while a tenant
teardown's `on delete cascade` holds `provider_accounts` and waits for
`provider_routing_settings`.

Neither side is wrong and no lock order fixes it. The DDL transaction must
change two tables, so it can only ever hold one of the two locks while it waits
for the other; the cascade's order across the eleven tenant-scoped tables is the
server's choice. Measured on a pristine `origin/dev`, 3 runs in 14 failed, each
one a different suite — whichever transaction the server picked as the victim.

The runner now executes `test/persistence/migration-*.test.ts` first, with
`--parallel` off, and excludes them from the parallel pass. With no other
connection open, no other transaction can hold the lock the DDL waits for, so
the inversion cannot form. The two passes together still run every backend file
once.

`SCHEMA_DDL_DIR`/`SCHEMA_DDL_FILE` pick the files for the serial pass and
`SCHEMA_DDL_IGNORE` is the same predicate for `--path-ignore-patterns`; the glob
must stay rooted with `**` because the flag matches paths as `bun test`
discovered them, relative to the working directory. An absolute pattern matches
nothing, which runs those files twice and restores the deadlock.
With `CARTETHYIA_TEST_DATABASE_URL` unset the run was supposed to be green with
stated skips, and it was not: four cases in `remote-debug-access`, one error per
file in `static-token-detection`, `runtime-settings-legacy` and
`share-link-refusal`.

Three distinct causes, one shape. `getDb()` throws when no connection string is
configured, and a skipped suite still evaluates its body to collect the cases —
so a handle built at collection time fails the file instead of skipping it.
`remote-debug-access` was worse: it was a plain `describe` over `createWorld`,
which needs the database, so it ran and failed four assertions rather than
skipping.

Each site now resolves its handle where the work happens: inside `beforeAll` for
the two suites that keep it for the whole file, and per call for
`share-link-refusal`, whose helpers are the only consumers. `remote-debug-access`
uses the `dbDescribe` primitive that exists for exactly this. `requireDatabase()`
is gone from `share-link-refusal`: with `dbDescribe` it was the second gate on
the same condition.

No behaviour changes when a database is configured: 1992 backend + 375 dashboard
pass, same as before.
The workflow was `workflow_dispatch` only and ran two typechecks, so nothing
ever executed `test/` — even though `AGENTS.md` names `bun run test` as a gate.
Two defects sat in `dev` because of that: a test that opened an absolute path on
the author's desktop, and a `homeDir` that Bun caches on first use, which made
the injector tests write into the operator's real home directory.

Adds a `test` job with disposable PostgreSQL and Redis services carrying the
same image, credentials and host port as `docker-compose.test.yml`, so the URL
that works in CI also works for `bun run test-db:up` locally. The job exports
both service URLs; `scripts/ci-run-tests.ts` loads `.env.test` without
overriding exported variables, which is what lets CI keep the file's fixed
encryption key while pointing at the service containers.

Triggers become `push` to `dev`/`SeeYouLater`, `pull_request`, and the existing
manual dispatch, with `contents: read` and a ref-scoped concurrency group.

`bun-version` is dropped from all three jobs: `oven-sh/setup-bun` reads the
committed `.bun-version`, the same pin `packageManager` and `engines` carry,
where `latest` let CI drift onto a Bun the lockfile was never resolved against.

Verified locally against the same service shape the workflow declares
(PostgreSQL on 55432, Redis db 15): 1992 backend + 375 dashboard tests pass with
no skips, and a control run with a bogus port fails, proving an exported
`CARTETHYIA_TEST_DATABASE_URL` overrides `.env.test` as the job relies on.
@joshberen joshberen closed this Oct 14, 2026
@joshberen
joshberen deleted the ci/run-test-suite branch October 14, 2026 05:41
risunCode added a commit that referenced this pull request Oct 15, 2026
- #10 env-overridable UA os/arch/cl (was pinned darwin/arm64 everywhere)
- #4 interleaved-thinking beta passthrough on Claude routes
- #6 parametersJsonSchema -> parameters tool-schema normalization
- #3 last-good-endpoint stickiness, so a dead endpoint is not
  re-probed every turn

31 tests pass, typecheck clean.
risunCode added a commit that referenced this pull request Oct 15, 2026
Items #6 (Codex 429 budget), #7 (fallbacks chain), #10 (image clamp,
both paths) are now complete. Slow-lane #1 is partial with the section
still missing named explicitly rather than left vague.
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.

2 participants