Skip to content

Reword the write-queue 503 message and keep API-test exports out of the repo root - #3016

Merged
kriszyp merged 2 commits into
mainfrom
fix/queue-message-and-export-path-hygiene
Oct 5, 2026
Merged

kriszyp merged 2 commits into
mainfrom
fix/queue-message-and-export-path-hygiene

Conversation

@kriszyp

@kriszyp kriszyp commented Oct 5, 2026 •

Copy link
Copy Markdown
Member

⊙ Problem

The write-queue 503 reads "Outstanding write transactions have too long of queue", which is ungrammatical in a client-visible error. Separately, the terminology and northwind API tests call export_local with path: './', so their export files land in the repo root, and only .gitignore hides them.

❓ Your call: The wording change is client-visible. A repo-wide grep finds no matcher for the old text in this worktree, in harper-pro outside its core submodule copies, or in documentation. Keeping the old text is the alternative; I made the change as specified. Reverting is a one-line change.

💡 Solution

  • The 503 now reads "Outstanding write transactions have too long a queue, please try again later" (DatabaseTransaction.ts:1306). Status, error class and commit accounting are unchanged. No docs companion: the message text does not appear in documentation. The quoted comment at DatabaseTransaction.ts:2014 follows the new wording.
  • terminology.test.mjs imports mkdtempSync, rmSync and existsSync. The export goes to a per-test mkdtemp directory under os.tmpdir() (L550). The test asserts test_export_terminology_test.json exists there (L564), then removes the directory in finally (L566).
  • northwind.test.mjs imports the same helpers. All seven export_local calls use one suite-scoped directory (declared L2036, created in before() L2042, removed in after() L2129). The task named one call (L11880); two more (Jobs Test Export To Local using SQL / NoSQL as su, e.g. L12436) succeed and wrote test_export.json into the root, so they are included.

Merge note: Add integrated logger-status system with .status() API, health checks and hierarchical view and Add type-strip mode: run Harper directly from .ts sources also rewrite the quoted comment at DatabaseTransaction.ts:2014. Whichever merges second resolves that one line.

⚖️ Alternatives

  • One export directory per test instead of per suite: rejected. Seven call sites in one suite share one directory with one rule (path: exportDir). The permission-denial cases write nothing, and they use the same directory so no call site keeps ./.

✅ Verification

Run in this worktree on the PR head:

  • npm run build: clean. prettier --check and oxlint --quiet on the three changed files: clean.
  • npm run test:integration -- integrationTests/apiTests/terminology.test.mjs: 48 pass, 0 fail. The repo root has no export files before or after.
  • npm run test:integration -- integrationTests/apiTests/northwind.test.mjs: 575 tests, 562 pass, 0 fail, 13 skipped (the existing S3 and network-CSV cases). The repo root has no export files.
  • test:unit:main (daemon GIT_* variables unset): 6436 pass, 202 pending, 3 failing under load from other agents' suites on this host. Re-running the two affected files alone (rootConfigPublication, prepareApplicationSerialization) gives 42 pass.
  • test:unit:resources: 3951 pass, 0 fail, 54 pending.
  • test:integration:all, run on 0e20e38: 2296 pass, 0 fail, 18 skipped, 6 cancelled. The six cancelled tests are the Ollama suite, which lacks its default models on this host, so the run exits 1 here. The follow-up commit changes only the two API test files, which were re-run above.
  • Pre-push review: round 1 (full) returned COMMENTS. Its surviving items were fixed in the follow-up commit. The one claim that absolute export_local paths fail was refuted by the passing runs above. Round 2 (delta over the follow-up commit) returned LGTM.

🤖 Generated by Claude Sonnet 5 (Claude Code); posted via @kriszyp.

🤖 Generated with Claude Code

Related PRs: #372 overlaps (logger-status system; also edits the quoted comment), #562 overlaps (type-strip mode; also edits the quoted comment), #1835 independent (blob-unlink queue), #2155 independent (RocksDB expiry sweeps), #2918 independent (record lock ordering), #2958 independent (long-transaction abort log)
Complexity: easy

Review-Coverage: authored=claude; ran=gemini,cursor-grok,codex; adjudicated=domain; declined=cursor-composer,cursor-kimi,cursor-muse; rounds=2; full=1 @ 4ffae6c

Review-Attention: read ~7m (critical: DatabaseTransaction.ts) @ 4ffae6c

kriszyp and others added 2 commits October 5, 2026 08:49
…he repo root

The 503 said "too long of queue"; it now says "too long a queue". The
terminology and northwind api tests exported to './', which is the repo
root, and only .gitignore hid the files. They now export into a per-run
mkdtemp directory under os.tmpdir(), removed in teardown.

Dispatch-Task: harper-queue-message-and-export-path-hygiene
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013zNpYJvbqe9NqkxRpZgoSA
…ctory

Teardown now runs in try/finally and skips the rm when the export
directory was never created, so a failed setup reports its own error.
The terminology export test asserts the output file exists in the
directory it requested.

Dispatch-Task: harper-queue-message-and-export-path-hygiene
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013zNpYJvbqe9NqkxRpZgoSA

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request improves the integration tests by using temporary directories for local export tests instead of writing to the project root directory. Specifically, in northwind.test.mjs and terminology.test.mjs, temporary directories are created and cleaned up during the test lifecycle, and the export paths are updated accordingly. Additionally, a minor grammatical typo ("too long of queue" to "too long a queue") is corrected in DatabaseTransaction.ts. There are no review comments, and we have no feedback to provide.

@kriszyp
kriszyp marked this pull request as ready for review October 5, 2026 19:36
@kriszyp
kriszyp merged commit 0bc46e7 into main Oct 5, 2026
50 of 51 checks passed
@kriszyp
kriszyp deleted the fix/queue-message-and-export-path-hygiene branch October 5, 2026 19:36
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