Test the tripwire refusal's rollback and unwrapped-error guarantee by borisno2 · Pull Request #1546 · OpenSaasAU/stack

borisno2 · 2026-09-13T11:43:02Z

Summary

Adds a test on packages/core/src/context/write-transaction.test.ts proving that a tripwire refusal (UnmarkedQueryError, raised by originTripwire in beforeCompile) mid-write rolls back the write's implicit transaction and reaches the caller unwrapped.
The refusal is triggered by having an afterOperation hook issue a raw statement directly against context.ormHandle without entering an origin scope (no withOrigin wrap) — hooks run outside the engine's origin scope by construction (ADR-0059), so an unmarked statement issued from one is exactly what the tripwire is designed to refuse.
Asserts Object.getPrototypeOf(failure) === UnmarkedQueryError.prototype (prototype identity, not message matching, per the issue's acceptance criteria) and that the error is not a DatabaseError (i.e., not re-classified/wrapped by normalizeDatabaseError).
Asserts both the Job and Audit tables are left empty, proving the whole write (not just the refused statement) rolled back.

Why this test design

Since context.db writes now open a real implicit transaction (#1205), and the tripwire fires on any statement compiled with no origin in scope (ADR-0059), the cleanest way to reproduce "the second of several statements is refused by the tripwire" without fabricating middleware internals is to have a hook perform a second, real statement through the raw ORM handle with no origin entered — the same technique the existing "a hook's own write through ormHandle rolls back with the write" test uses, but omitting the withOrigin wrap that test relies on to avoid the refusal.

I verified the test is falsifiable against both regressions it guards:

Reverting runInTransaction to skip opening a transaction (simulating the pre-Every write on prisma-8 runs without a transaction: the rollback probe tests the Prisma 7 $transaction name #1205 defect) makes the rollback assertion fail (rows survive).
Making normalizeDatabaseError unconditionally wrap errors makes the prototype-identity assertion fail (the error is no longer UnmarkedQueryError).

Both were confirmed locally, then reverted before committing.

Test plan

New test added: a tripwire refusal mid-write rolls back and reaches the caller unwrapped
pnpm --filter @opensaas/stack-core test — 1758 passed, 7 skipped (no regressions)
pnpm lint passes
pnpm format / pnpm manypkg fix — no changes needed beyond the new files
Changeset added (@opensaas/stack-core, patch)

Closes #1209

🤖 Generated with Claude Code

https://claude.ai/code/session_01LFqWbBhZwZyqBUzLmvbcgs

Generated by Claude Code

…antee Closes #1209. A refusal mid-write (UnmarkedQueryError, raised by originTripwire in beforeCompile) now has a test proving it rolls back the write's implicit transaction and reaches the caller unwrapped, checked by prototype identity rather than message matching. Verified falsifiable against both the pre-#1205 no-transaction regression and a hypothetical error-wrapping regression in normalizeDatabaseError. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LFqWbBhZwZyqBUzLmvbcgs

chatgpt-codex-connector · 2026-09-13T11:43:05Z

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

changeset-bot · 2026-09-13T11:43:05Z

🦋 Changeset detected

Latest commit: 910fb36

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 9 packages

Name	Type
@opensaas/stack-core	Patch
@opensaas/stack-auth	Patch
@opensaas/stack-cli	Patch
@opensaas/stack-rag	Patch
@opensaas/stack-storage	Patch
@opensaas/stack-tiptap	Patch
@opensaas/stack-ui	Patch
@opensaas/stack-storage-s3	Patch
@opensaas/stack-storage-vercel	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

vercel · 2026-09-13T11:43:05Z

Deployment failed for project stack-docs with the following error:

Resource is limited - try again in 24 hours (more than 100, code: "api-deployments-free-per-day").

Learn More: https://vercel.com/open-saas?upgradeToPro=build-rate-limit

borisno2 · 2026-09-13T11:51:10Z

Code review — `/review` (medium effort)

Reviewed the diff (packages/core/src/context/write-transaction.test.ts + the changeset) against origin/main, including tracing withOrigin → originTripwire → runWriteInTransaction → runWithTransactionBoundary → normalizeDatabaseError/classifyDriverError, and empirically verifying the test's falsifiability by mutating the source it exercises (each mutation reverted before this comment):

Neutered refuseUnmarkedQuery to a no-op → the new test fails with TypeError: Cannot convert undefined or null to object at the Object.getPrototypeOf(failure) line (see finding 1 below) — confirms the raw context.ormHandle.Audit.create() call really is refused by the tripwire, and only by it (the identical call succeeds one test above when wrapped in withOrigin('engine', …), which already serves as the implicit control).
Made runInTransaction skip opening a real transaction → the Job rows assertion fails with a persisted ship row — confirms the rollback assertion is load-bearing, not vacuously true.
Made the transaction-boundary rethrow a fresh plain Error with the same message instead of the original → expect(Object.getPrototypeOf(failure)).toBe(UnmarkedQueryError.prototype) fails while a message-matching assertion would not have — confirms the prototype-identity check is doing real work per User story 12's refusal rollback and unwrapped-error guarantee has no test #1209's acceptance criterion ("provable by prototype identity, not message matching").

Conclusion on the core question: the test exercises exactly what it claims. Hooks run outside the engine's origin scope by construction (origin.ts's withOrigin doc), so the raw create.call(collection, { note }) issued with no withOrigin wrap compiles with originStore.getStore() === undefined and is refused by originTripwire.beforeCompile — not for some unrelated reason (bad field, access denial, etc.). The rollback assertion is real: runWriteInTransaction's throw propagates unmodified out of runInTransaction (Prisma's $transaction callback), through runWithTransactionBoundary's throw txError (no reclassification happens outside the withOrigin('engine', …) catch, and UnmarkedQueryError doesn't match classifyDriverError's driver-error shape anyway), to the caller.

Findings

Minor — opaque failure mode if the regression under test ever recurs (write-transaction.test.ts:262). If the write ever silently stops being refused (tripwire regresses), .then(() => undefined, ...) resolves failure to undefined, and Object.getPrototypeOf(undefined) throws before the expect runs — confirmed above. CI still fails, but with a confusing crash instead of "expected UnmarkedQueryError, got success." A expect(failure).toBeInstanceOf(Error) (or similar) guard before the prototype check would give a clearer signal.
Nit — duplication (write-transaction.test.ts:224-235 vs. 169-180). writeWithNoOrigin is a near-verbatim copy of writeThroughHandle in the test immediately above, differing only in the withOrigin('engine', …) wrap. Not a bug, just maintenance surface — a shared rawAuditCreate(collection, note) helper that each test wraps differently would remove it.

No correctness bugs found, and no coverage gap for the specific guarantee #1209 asks for (rollback + unwrapped, proven by prototype identity). Test suite run locally: all 7 tests in the file pass.

Generated by Claude Code

- Assert failure is an Error before checking its prototype, so a tripwire regression fails cleanly instead of crashing on Object.getPrototypeOf(undefined). - Extract writeAuditRow(context, note, { marked }) so the marked and unmarked raw-write tests share one helper instead of two near-identical copies. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LFqWbBhZwZyqBUzLmvbcgs

borisno2 · 2026-09-13T11:53:35Z

Thanks for the review — both findings addressed in 910fb36:

Added expect(failure).toBeInstanceOf(Error) before the prototype-identity check, so a tripwire regression fails the assertion cleanly instead of crashing on Object.getPrototypeOf(undefined).
Extracted a shared writeAuditRow(context, note, { marked }) helper so the marked (existing) and unmarked (new) raw-write tests no longer carry two near-identical copies of the same helper.

pnpm --filter @opensaas/stack-core test, pnpm lint, and pnpm format all still pass clean.

Generated by Claude Code

borisno2 · 2026-09-13T11:55:30Z

The Vercel status check is failing with "Deployment rate limited — retry in 24 hours" (api-deployments-free-per-day). This is the docs-site preview deployment hitting Vercel's free-tier daily deployment cap — this PR touches only packages/core/src/context/write-transaction.test.ts and a changeset, nothing under docs/, so the failure isn't caused by this change. It's an account-level quota exhausted by the day's overall deployment volume across the repo, not a per-PR/per-commit condition, so re-running it now would not help (it will only succeed once the 24-hour window resets) — there's no fix to port here. Leaving this PR watched; will keep an eye on it independent of this check.

Generated by Claude Code

github-actions · 2026-09-13T12:13:56Z