1.0 hardening: fix @transaction early-return composition - #7

Merged
quinnj merged 5 commits into
mainfrom
release-1.0-hardening
Aug 7, 2026
Merged

1.0 hardening: fix @transaction early-return composition#7
quinnj merged 5 commits into
mainfrom
release-1.0-hardening

Conversation

@quinnj

@quinnjquinnj commented Aug 7, 2026

Copy link
Copy Markdown
Member

Continuation of the 1.0 readiness work from #5, on a fresh branch against merged main: independent adversarial review rounds against a live PostgreSQL, fixing what they surface, until a round comes back clean.

The problem: @transaction early-return was unsound under composition

Round 13 (the first review of #5's merged tail) found that the early-return support rewrote return x into an untagged thrown marker, so the dynamically nearest @transaction expansion always intercepted it. Verified live: nested @transaction + return silently rolled back all levels and left the connection stuck in a transaction; a user try/catch swallowed the marker and returned its own fallback value; return inside task macros threw the marker instead of producing the task's value; break/continue left the transaction open.

The evolution of the fix (rounds 14–16 + external review)

Commits 1–4 fixed this incrementally: per-expansion tokens, guards injected into user catches, and a growing skip list of closure/task-forming constructs (short-form defs, @spawnat, @fetch/@fetchfrom, comprehensions). Each round's reviewer found the next hole in the allowlist.

External review (codex) then proved the endpoint of that trajectory: no finite allowlist can cover third-party task macros (reproduced with a minimal @local_task), and — the key insight — the expansion's finally already gives plain return the intended semantics with no rewriting at all.

Final design (commit 5, net −140 lines)

The marker struct, AST walker, try-guard injection, and both skip lists are deleted. The macro is now just try/catch/finally:

  • Every non-exceptional exit commits — normal completion, return, break, continue. A return unwinds through every enclosing expansion's finally, each committing its level exactly once, innermost first. Only a thrown exception rolls back.
  • Plain Julia semantics everywhere: user catches cannot intercept a plain return; closures, do-blocks, comprehensions, and any task-forming macro (standard or third-party) keep their ordinary meaning untouched.
  • The finally handles its own commit failure (the second P1 from external review): it rolls back the current level before propagating — commit at savepoint depth leaves the depth unchanged on failure, so each enclosing level unwinds its own. Previously a RELEASE SAVEPOINT failure during a break out of an aborted nested level escaped cleanup and left the outer transaction open with its work pending.

Verification

  • Behavioral regressions for every failure mode found along the way: nested return (incl. cross-connection and 3-level), both user-catch shapes, Threads.@spawn, a third-party @local_task macro, Distributed.@spawnat/@fetch/@fetchfrom (run locally on worker 1), short-form helpers escaping the block, break/continue, recursion re-entering the same expansion, plain-vs-wrapped flattened-iterator equivalence, and the nested-break-with-aborted-savepoint commit-failure case (asserts the server error surfaces and nothing stays open client- or server-side).
  • Mutation-verified: restoring the original marker behavior fails 6+ tests; removing the finally rollback fails 5.
  • Suite: 1433/1433 locally (Docker integration + TLS fixture); full 18-check CI matrix green on every commit.
  • All three external review threads answered on their respective conversations.

🤖 Generated with Claude Code

The return-rewrite threw an untagged TransactionReturn marker, so the
dynamically nearest @transaction expansion always intercepted it:
- a return inside a NESTED @transaction committed only the inner savepoint,
and the inner expansion's own plain `return` then skipped every enclosing
commit — all levels' work was silently rolled back and the connection was
left inside the outer transaction
- a user try/catch inside the body swallowed the marker and returned the
catch's value instead of the intended return value, silently
- a return inside Threads.@Spawn / @async in the body was rewritten too, so
the task threw the marker instead of producing its value
Each expansion now tags its markers with a compile-time token. A catch that
receives a foreign marker commits its own level and keeps unwinding to the
owning expansion, so an early return commits every enclosing level and
returns exactly once. User catch blocks get a guard injected that rethrows
the marker (a private type no handler can mean to catch). Task-forming
macros are excluded from the rewrite, matching the existing exclusion of
closures. break/continue — which bypass both the commit and any catch — now
commit via a finally, making every non-exceptional exit consistent: only a
thrown exception rolls back. Documented in the docstring.
Regression tests cover nested return, both catch shapes, @Spawn, break,
continue, and recursive re-entry of the same expansion; removing the fix
fails six of them plus downstream testsets poisoned by the stuck-open
transaction.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment threadsrc/Postgres.jl Outdated
h(x) = ... parses as :(=) with a call-shaped left-hand side, not as
:function, so the rewrite's closure exclusion missed it: a return inside a
local short-form helper defined in the @transaction body was rewritten into
a transaction-return marker. Calling such a helper silently early-returned
the ENCLOSING function with the helper's internal value (committing on the
way out), and a helper that escaped the block threw a raw TransactionReturn
at its caller with no expansion active to catch it.
All short-form shapes are skipped (plain, ::T return-type, where-clauses,
qualified names), while ordinary assignments whose right-hand side contains
a return are still rewritten. Also adds @spawnat to the task-macro skip
list — same bug class as @spawn/@async, verified to wrap the marker in a
RemoteException instead of producing the task's value.
Live test: a short-form helper with an internal early return, used inside
the block and after it escapes. Unit pins for every definition shape, the
task macros, and the ordinary-assignment counter-cases. Removing the skip
fails six of them.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment threadsrc/Postgres.jl Outdated
`return` anywhere inside a comprehension or generator — body or iterator
expression — is a lowering error in plain Julia. The rewrite turned it
into a legal `throw` of the transaction-return marker, silently accepting
code that would stop compiling the moment the @transaction wrapper is
removed, and giving it early-return semantics it never legitimately had.
Comprehension, typed-comprehension, generator, and flatten heads are now
left untouched so the construct errors exactly as it does everywhere else.
Nothing valid is lost: a legal comprehension cannot contain a bare
`return`, and nested closures inside one were already excluded. Unit pins
cover all four syntactic shapes plus the counter-case that a `return`
inside an ordinary `for` loop is still rewritten.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment threadsrc/Postgres.jl Outdated
quinnjand others added 2 commits August 7, 2026 13:44
Distributed.@fetch and @fetchfrom wrap their body in a remotely-executed
thunk whose return is the fetched value, exactly like @spawnat — but they
were missing from _TASK_MACROS, so a return inside one was rewritten into
a transaction-return marker. Verified live: the block then throws a
RemoteException wrapping the marker and rolls back, where plain Julia
returns the value.
Also corrects the comprehension-skip rationale in comments: a return in a
comprehension/generator BODY is a lowering error (which the rewrite must
not legalize), while the iterator-expression shapes lowering does accept
behave correctly un-rewritten — they exit the block non-exceptionally and
commit through the expansion's finally, as verified live. And documents at
the token comparison that unconditional returning would be observationally
equivalent today only because every enclosing expansion's finally also
commits; the token check stays as the semantic guarantee.
Independent adversarial verification of the three @transaction commits
(61 live scenarios, plain-Julia baselines, 6 mutations against the full
suite) found no other behavioral gaps.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…nally
Review of the rewrite approach (PR #7 threads) proved its task-macro
allowlist structurally insufficient: any third-party macro that wraps its
body in a task or closure — reproduced with a minimal @local_task — had its
internal returns rewritten into transaction-return markers, throwing
TaskFailedException(TransactionReturn) instead of producing the task's
value. No finite list of standard macros covers user-defined ones.
The expansion's finally already gives plain `return` the intended semantics
with no rewriting at all: a return unwinds through every enclosing
expansion's finally, each committing its level exactly once, innermost
first. User catches cannot intercept a plain return, closures and task
macros keep their ordinary meaning untouched, and the flattened-iterator
form that plain lowering accepts behaves identically wrapped or not. The
marker struct, the AST walker, the try-guard injection, and both skip lists
are deleted.
The finally also now handles its own commit failure: it rolls back the
current level before propagating (commit at savepoint depth leaves depth
unchanged on failure), so every enclosing level — macro expansion or plain
catch — unwinds its own. Previously a RELEASE SAVEPOINT failure during a
break out of a nested level (savepoint aborted by a swallowed server error)
escaped past the enclosing macro's ability to clean up, leaving the outer
transaction open with its work pending.
Behavioral regressions replace the deleted unit AST pins: a third-party
@local_task macro, Distributed @spawnat/@fetch/@fetchfrom run locally on
worker 1, plain-vs-wrapped flattened-iterator equivalence, and the nested
break with an aborted savepoint (asserts the server error surfaces and
nothing stays open client- or server-side; removing the finally rollback
fails five assertions).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@quinnj

Copy link
Copy Markdown
MemberAuthor

READY/CLEAN on exact head 467596923c86b4f483d107174b8a28d4edd7f476.

Independent PostgreSQL 16 retests passed: standard and third-party task macros preserve plain return semantics and commit; a failed finally savepoint commit now unwinds every client/server transaction level and rolls back pending rows; the flattened-iterator case matches plain Julia and commits. All 15 exact-head CI jobs and both Codecov checks are green. The PR is MERGEABLE/CLEAN with no unresolved review threads.

@quinnj
quinnj merged commit 77b9296 into mainAug 7, 2026
17 checks passed
quinnj added a commit that referenced this pull request Aug 10, 2026
Release the 1.0 hardening fix from #7 (@transaction early-return
composition) as a patch on top of the registered 1.0.0.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for freeto 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

@quinnj
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

1.0 hardening: fix @transaction early-return composition - #7

Merged
quinnj merged 5 commits into
mainfrom
release-1.0-hardening
Aug 7, 2026
Merged

1.0 hardening: fix @transaction early-return composition#7
quinnj merged 5 commits into
mainfrom
release-1.0-hardening

Conversation

@quinnj

@quinnjquinnj commented Aug 7, 2026

Copy link
Copy Markdown
Member

Continuation of the 1.0 readiness work from #5, on a fresh branch against merged main: independent adversarial review rounds against a live PostgreSQL, fixing what they surface, until a round comes back clean.

The problem: @transaction early-return was unsound under composition

Round 13 (the first review of #5's merged tail) found that the early-return support rewrote return x into an untagged thrown marker, so the dynamically nearest @transaction expansion always intercepted it. Verified live: nested @transaction + return silently rolled back all levels and left the connection stuck in a transaction; a user try/catch swallowed the marker and returned its own fallback value; return inside task macros threw the marker instead of producing the task's value; break/continue left the transaction open.

The evolution of the fix (rounds 14–16 + external review)

Commits 1–4 fixed this incrementally: per-expansion tokens, guards injected into user catches, and a growing skip list of closure/task-forming constructs (short-form defs, @spawnat, @fetch/@fetchfrom, comprehensions). Each round's reviewer found the next hole in the allowlist.

External review (codex) then proved the endpoint of that trajectory: no finite allowlist can cover third-party task macros (reproduced with a minimal @local_task), and — the key insight — the expansion's finally already gives plain return the intended semantics with no rewriting at all.

Final design (commit 5, net −140 lines)

The marker struct, AST walker, try-guard injection, and both skip lists are deleted. The macro is now just try/catch/finally:

  • Every non-exceptional exit commits — normal completion, return, break, continue. A return unwinds through every enclosing expansion's finally, each committing its level exactly once, innermost first. Only a thrown exception rolls back.
  • Plain Julia semantics everywhere: user catches cannot intercept a plain return; closures, do-blocks, comprehensions, and any task-forming macro (standard or third-party) keep their ordinary meaning untouched.
  • The finally handles its own commit failure (the second P1 from external review): it rolls back the current level before propagating — commit at savepoint depth leaves the depth unchanged on failure, so each enclosing level unwinds its own. Previously a RELEASE SAVEPOINT failure during a break out of an aborted nested level escaped cleanup and left the outer transaction open with its work pending.

Verification

  • Behavioral regressions for every failure mode found along the way: nested return (incl. cross-connection and 3-level), both user-catch shapes, Threads.@spawn, a third-party @local_task macro, Distributed.@spawnat/@fetch/@fetchfrom (run locally on worker 1), short-form helpers escaping the block, break/continue, recursion re-entering the same expansion, plain-vs-wrapped flattened-iterator equivalence, and the nested-break-with-aborted-savepoint commit-failure case (asserts the server error surfaces and nothing stays open client- or server-side).
  • Mutation-verified: restoring the original marker behavior fails 6+ tests; removing the finally rollback fails 5.
  • Suite: 1433/1433 locally (Docker integration + TLS fixture); full 18-check CI matrix green on every commit.
  • All three external review threads answered on their respective conversations.

🤖 Generated with Claude Code

The return-rewrite threw an untagged TransactionReturn marker, so the
dynamically nearest @transaction expansion always intercepted it:
- a return inside a NESTED @transaction committed only the inner savepoint,
and the inner expansion's own plain `return` then skipped every enclosing
commit — all levels' work was silently rolled back and the connection was
left inside the outer transaction
- a user try/catch inside the body swallowed the marker and returned the
catch's value instead of the intended return value, silently
- a return inside Threads.@Spawn / @async in the body was rewritten too, so
the task threw the marker instead of producing its value
Each expansion now tags its markers with a compile-time token. A catch that
receives a foreign marker commits its own level and keeps unwinding to the
owning expansion, so an early return commits every enclosing level and
returns exactly once. User catch blocks get a guard injected that rethrows
the marker (a private type no handler can mean to catch). Task-forming
macros are excluded from the rewrite, matching the existing exclusion of
closures. break/continue — which bypass both the commit and any catch — now
commit via a finally, making every non-exceptional exit consistent: only a
thrown exception rolls back. Documented in the docstring.
Regression tests cover nested return, both catch shapes, @Spawn, break,
continue, and recursive re-entry of the same expansion; removing the fix
fails six of them plus downstream testsets poisoned by the stuck-open
transaction.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment threadsrc/Postgres.jl Outdated
h(x) = ... parses as :(=) with a call-shaped left-hand side, not as
:function, so the rewrite's closure exclusion missed it: a return inside a
local short-form helper defined in the @transaction body was rewritten into
a transaction-return marker. Calling such a helper silently early-returned
the ENCLOSING function with the helper's internal value (committing on the
way out), and a helper that escaped the block threw a raw TransactionReturn
at its caller with no expansion active to catch it.
All short-form shapes are skipped (plain, ::T return-type, where-clauses,
qualified names), while ordinary assignments whose right-hand side contains
a return are still rewritten. Also adds @spawnat to the task-macro skip
list — same bug class as @spawn/@async, verified to wrap the marker in a
RemoteException instead of producing the task's value.
Live test: a short-form helper with an internal early return, used inside
the block and after it escapes. Unit pins for every definition shape, the
task macros, and the ordinary-assignment counter-cases. Removing the skip
fails six of them.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment threadsrc/Postgres.jl Outdated
`return` anywhere inside a comprehension or generator — body or iterator
expression — is a lowering error in plain Julia. The rewrite turned it
into a legal `throw` of the transaction-return marker, silently accepting
code that would stop compiling the moment the @transaction wrapper is
removed, and giving it early-return semantics it never legitimately had.
Comprehension, typed-comprehension, generator, and flatten heads are now
left untouched so the construct errors exactly as it does everywhere else.
Nothing valid is lost: a legal comprehension cannot contain a bare
`return`, and nested closures inside one were already excluded. Unit pins
cover all four syntactic shapes plus the counter-case that a `return`
inside an ordinary `for` loop is still rewritten.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment threadsrc/Postgres.jl Outdated
quinnjand others added 2 commits August 7, 2026 13:44
Distributed.@fetch and @fetchfrom wrap their body in a remotely-executed
thunk whose return is the fetched value, exactly like @spawnat — but they
were missing from _TASK_MACROS, so a return inside one was rewritten into
a transaction-return marker. Verified live: the block then throws a
RemoteException wrapping the marker and rolls back, where plain Julia
returns the value.
Also corrects the comprehension-skip rationale in comments: a return in a
comprehension/generator BODY is a lowering error (which the rewrite must
not legalize), while the iterator-expression shapes lowering does accept
behave correctly un-rewritten — they exit the block non-exceptionally and
commit through the expansion's finally, as verified live. And documents at
the token comparison that unconditional returning would be observationally
equivalent today only because every enclosing expansion's finally also
commits; the token check stays as the semantic guarantee.
Independent adversarial verification of the three @transaction commits
(61 live scenarios, plain-Julia baselines, 6 mutations against the full
suite) found no other behavioral gaps.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…nally
Review of the rewrite approach (PR #7 threads) proved its task-macro
allowlist structurally insufficient: any third-party macro that wraps its
body in a task or closure — reproduced with a minimal @local_task — had its
internal returns rewritten into transaction-return markers, throwing
TaskFailedException(TransactionReturn) instead of producing the task's
value. No finite list of standard macros covers user-defined ones.
The expansion's finally already gives plain `return` the intended semantics
with no rewriting at all: a return unwinds through every enclosing
expansion's finally, each committing its level exactly once, innermost
first. User catches cannot intercept a plain return, closures and task
macros keep their ordinary meaning untouched, and the flattened-iterator
form that plain lowering accepts behaves identically wrapped or not. The
marker struct, the AST walker, the try-guard injection, and both skip lists
are deleted.
The finally also now handles its own commit failure: it rolls back the
current level before propagating (commit at savepoint depth leaves depth
unchanged on failure), so every enclosing level — macro expansion or plain
catch — unwinds its own. Previously a RELEASE SAVEPOINT failure during a
break out of a nested level (savepoint aborted by a swallowed server error)
escaped past the enclosing macro's ability to clean up, leaving the outer
transaction open with its work pending.
Behavioral regressions replace the deleted unit AST pins: a third-party
@local_task macro, Distributed @spawnat/@fetch/@fetchfrom run locally on
worker 1, plain-vs-wrapped flattened-iterator equivalence, and the nested
break with an aborted savepoint (asserts the server error surfaces and
nothing stays open client- or server-side; removing the finally rollback
fails five assertions).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@quinnj

Copy link
Copy Markdown
MemberAuthor

READY/CLEAN on exact head 467596923c86b4f483d107174b8a28d4edd7f476.

Independent PostgreSQL 16 retests passed: standard and third-party task macros preserve plain return semantics and commit; a failed finally savepoint commit now unwinds every client/server transaction level and rolls back pending rows; the flattened-iterator case matches plain Julia and commits. All 15 exact-head CI jobs and both Codecov checks are green. The PR is MERGEABLE/CLEAN with no unresolved review threads.

@quinnj
quinnj merged commit 77b9296 into mainAug 7, 2026
17 checks passed
quinnj added a commit that referenced this pull request Aug 10, 2026
Release the 1.0 hardening fix from #7 (@transaction early-return
composition) as a patch on top of the registered 1.0.0.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for freeto 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

@quinnj
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

1.0 hardening: fix @transaction early-return composition - #7

Merged
quinnj merged 5 commits into
mainfrom
release-1.0-hardening
Aug 7, 2026
Merged

1.0 hardening: fix @transaction early-return composition#7
quinnj merged 5 commits into
mainfrom
release-1.0-hardening

Conversation

@quinnj

@quinnjquinnj commented Aug 7, 2026

Copy link
Copy Markdown
Member

Continuation of the 1.0 readiness work from #5, on a fresh branch against merged main: independent adversarial review rounds against a live PostgreSQL, fixing what they surface, until a round comes back clean.

The problem: @transaction early-return was unsound under composition

Round 13 (the first review of #5's merged tail) found that the early-return support rewrote return x into an untagged thrown marker, so the dynamically nearest @transaction expansion always intercepted it. Verified live: nested @transaction + return silently rolled back all levels and left the connection stuck in a transaction; a user try/catch swallowed the marker and returned its own fallback value; return inside task macros threw the marker instead of producing the task's value; break/continue left the transaction open.

The evolution of the fix (rounds 14–16 + external review)

Commits 1–4 fixed this incrementally: per-expansion tokens, guards injected into user catches, and a growing skip list of closure/task-forming constructs (short-form defs, @spawnat, @fetch/@fetchfrom, comprehensions). Each round's reviewer found the next hole in the allowlist.

External review (codex) then proved the endpoint of that trajectory: no finite allowlist can cover third-party task macros (reproduced with a minimal @local_task), and — the key insight — the expansion's finally already gives plain return the intended semantics with no rewriting at all.

Final design (commit 5, net −140 lines)

The marker struct, AST walker, try-guard injection, and both skip lists are deleted. The macro is now just try/catch/finally:

  • Every non-exceptional exit commits — normal completion, return, break, continue. A return unwinds through every enclosing expansion's finally, each committing its level exactly once, innermost first. Only a thrown exception rolls back.
  • Plain Julia semantics everywhere: user catches cannot intercept a plain return; closures, do-blocks, comprehensions, and any task-forming macro (standard or third-party) keep their ordinary meaning untouched.
  • The finally handles its own commit failure (the second P1 from external review): it rolls back the current level before propagating — commit at savepoint depth leaves the depth unchanged on failure, so each enclosing level unwinds its own. Previously a RELEASE SAVEPOINT failure during a break out of an aborted nested level escaped cleanup and left the outer transaction open with its work pending.

Verification

  • Behavioral regressions for every failure mode found along the way: nested return (incl. cross-connection and 3-level), both user-catch shapes, Threads.@spawn, a third-party @local_task macro, Distributed.@spawnat/@fetch/@fetchfrom (run locally on worker 1), short-form helpers escaping the block, break/continue, recursion re-entering the same expansion, plain-vs-wrapped flattened-iterator equivalence, and the nested-break-with-aborted-savepoint commit-failure case (asserts the server error surfaces and nothing stays open client- or server-side).
  • Mutation-verified: restoring the original marker behavior fails 6+ tests; removing the finally rollback fails 5.
  • Suite: 1433/1433 locally (Docker integration + TLS fixture); full 18-check CI matrix green on every commit.
  • All three external review threads answered on their respective conversations.

🤖 Generated with Claude Code

The return-rewrite threw an untagged TransactionReturn marker, so the
dynamically nearest @transaction expansion always intercepted it:
- a return inside a NESTED @transaction committed only the inner savepoint,
and the inner expansion's own plain `return` then skipped every enclosing
commit — all levels' work was silently rolled back and the connection was
left inside the outer transaction
- a user try/catch inside the body swallowed the marker and returned the
catch's value instead of the intended return value, silently
- a return inside Threads.@Spawn / @async in the body was rewritten too, so
the task threw the marker instead of producing its value
Each expansion now tags its markers with a compile-time token. A catch that
receives a foreign marker commits its own level and keeps unwinding to the
owning expansion, so an early return commits every enclosing level and
returns exactly once. User catch blocks get a guard injected that rethrows
the marker (a private type no handler can mean to catch). Task-forming
macros are excluded from the rewrite, matching the existing exclusion of
closures. break/continue — which bypass both the commit and any catch — now
commit via a finally, making every non-exceptional exit consistent: only a
thrown exception rolls back. Documented in the docstring.
Regression tests cover nested return, both catch shapes, @Spawn, break,
continue, and recursive re-entry of the same expansion; removing the fix
fails six of them plus downstream testsets poisoned by the stuck-open
transaction.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment threadsrc/Postgres.jl Outdated
h(x) = ... parses as :(=) with a call-shaped left-hand side, not as
:function, so the rewrite's closure exclusion missed it: a return inside a
local short-form helper defined in the @transaction body was rewritten into
a transaction-return marker. Calling such a helper silently early-returned
the ENCLOSING function with the helper's internal value (committing on the
way out), and a helper that escaped the block threw a raw TransactionReturn
at its caller with no expansion active to catch it.
All short-form shapes are skipped (plain, ::T return-type, where-clauses,
qualified names), while ordinary assignments whose right-hand side contains
a return are still rewritten. Also adds @spawnat to the task-macro skip
list — same bug class as @spawn/@async, verified to wrap the marker in a
RemoteException instead of producing the task's value.
Live test: a short-form helper with an internal early return, used inside
the block and after it escapes. Unit pins for every definition shape, the
task macros, and the ordinary-assignment counter-cases. Removing the skip
fails six of them.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment threadsrc/Postgres.jl Outdated
`return` anywhere inside a comprehension or generator — body or iterator
expression — is a lowering error in plain Julia. The rewrite turned it
into a legal `throw` of the transaction-return marker, silently accepting
code that would stop compiling the moment the @transaction wrapper is
removed, and giving it early-return semantics it never legitimately had.
Comprehension, typed-comprehension, generator, and flatten heads are now
left untouched so the construct errors exactly as it does everywhere else.
Nothing valid is lost: a legal comprehension cannot contain a bare
`return`, and nested closures inside one were already excluded. Unit pins
cover all four syntactic shapes plus the counter-case that a `return`
inside an ordinary `for` loop is still rewritten.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment threadsrc/Postgres.jl Outdated
quinnjand others added 2 commits August 7, 2026 13:44
Distributed.@fetch and @fetchfrom wrap their body in a remotely-executed
thunk whose return is the fetched value, exactly like @spawnat — but they
were missing from _TASK_MACROS, so a return inside one was rewritten into
a transaction-return marker. Verified live: the block then throws a
RemoteException wrapping the marker and rolls back, where plain Julia
returns the value.
Also corrects the comprehension-skip rationale in comments: a return in a
comprehension/generator BODY is a lowering error (which the rewrite must
not legalize), while the iterator-expression shapes lowering does accept
behave correctly un-rewritten — they exit the block non-exceptionally and
commit through the expansion's finally, as verified live. And documents at
the token comparison that unconditional returning would be observationally
equivalent today only because every enclosing expansion's finally also
commits; the token check stays as the semantic guarantee.
Independent adversarial verification of the three @transaction commits
(61 live scenarios, plain-Julia baselines, 6 mutations against the full
suite) found no other behavioral gaps.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…nally
Review of the rewrite approach (PR #7 threads) proved its task-macro
allowlist structurally insufficient: any third-party macro that wraps its
body in a task or closure — reproduced with a minimal @local_task — had its
internal returns rewritten into transaction-return markers, throwing
TaskFailedException(TransactionReturn) instead of producing the task's
value. No finite list of standard macros covers user-defined ones.
The expansion's finally already gives plain `return` the intended semantics
with no rewriting at all: a return unwinds through every enclosing
expansion's finally, each committing its level exactly once, innermost
first. User catches cannot intercept a plain return, closures and task
macros keep their ordinary meaning untouched, and the flattened-iterator
form that plain lowering accepts behaves identically wrapped or not. The
marker struct, the AST walker, the try-guard injection, and both skip lists
are deleted.
The finally also now handles its own commit failure: it rolls back the
current level before propagating (commit at savepoint depth leaves depth
unchanged on failure), so every enclosing level — macro expansion or plain
catch — unwinds its own. Previously a RELEASE SAVEPOINT failure during a
break out of a nested level (savepoint aborted by a swallowed server error)
escaped past the enclosing macro's ability to clean up, leaving the outer
transaction open with its work pending.
Behavioral regressions replace the deleted unit AST pins: a third-party
@local_task macro, Distributed @spawnat/@fetch/@fetchfrom run locally on
worker 1, plain-vs-wrapped flattened-iterator equivalence, and the nested
break with an aborted savepoint (asserts the server error surfaces and
nothing stays open client- or server-side; removing the finally rollback
fails five assertions).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@quinnj

Copy link
Copy Markdown
MemberAuthor

READY/CLEAN on exact head 467596923c86b4f483d107174b8a28d4edd7f476.

Independent PostgreSQL 16 retests passed: standard and third-party task macros preserve plain return semantics and commit; a failed finally savepoint commit now unwinds every client/server transaction level and rolls back pending rows; the flattened-iterator case matches plain Julia and commits. All 15 exact-head CI jobs and both Codecov checks are green. The PR is MERGEABLE/CLEAN with no unresolved review threads.

@quinnj
quinnj merged commit 77b9296 into mainAug 7, 2026
17 checks passed
quinnj added a commit that referenced this pull request Aug 10, 2026
Release the 1.0 hardening fix from #7 (@transaction early-return
composition) as a patch on top of the registered 1.0.0.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for freeto 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

@quinnj
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

1.0 hardening: fix @transaction early-return composition - #7

Merged
quinnj merged 5 commits into
mainfrom
release-1.0-hardening
Aug 7, 2026
Merged

1.0 hardening: fix @transaction early-return composition#7
quinnj merged 5 commits into
mainfrom
release-1.0-hardening

Conversation

@quinnj

@quinnjquinnj commented Aug 7, 2026

Copy link
Copy Markdown
Member

Continuation of the 1.0 readiness work from #5, on a fresh branch against merged main: independent adversarial review rounds against a live PostgreSQL, fixing what they surface, until a round comes back clean.

The problem: @transaction early-return was unsound under composition

Round 13 (the first review of #5's merged tail) found that the early-return support rewrote return x into an untagged thrown marker, so the dynamically nearest @transaction expansion always intercepted it. Verified live: nested @transaction + return silently rolled back all levels and left the connection stuck in a transaction; a user try/catch swallowed the marker and returned its own fallback value; return inside task macros threw the marker instead of producing the task's value; break/continue left the transaction open.

The evolution of the fix (rounds 14–16 + external review)

Commits 1–4 fixed this incrementally: per-expansion tokens, guards injected into user catches, and a growing skip list of closure/task-forming constructs (short-form defs, @spawnat, @fetch/@fetchfrom, comprehensions). Each round's reviewer found the next hole in the allowlist.

External review (codex) then proved the endpoint of that trajectory: no finite allowlist can cover third-party task macros (reproduced with a minimal @local_task), and — the key insight — the expansion's finally already gives plain return the intended semantics with no rewriting at all.

Final design (commit 5, net −140 lines)

The marker struct, AST walker, try-guard injection, and both skip lists are deleted. The macro is now just try/catch/finally:

  • Every non-exceptional exit commits — normal completion, return, break, continue. A return unwinds through every enclosing expansion's finally, each committing its level exactly once, innermost first. Only a thrown exception rolls back.
  • Plain Julia semantics everywhere: user catches cannot intercept a plain return; closures, do-blocks, comprehensions, and any task-forming macro (standard or third-party) keep their ordinary meaning untouched.
  • The finally handles its own commit failure (the second P1 from external review): it rolls back the current level before propagating — commit at savepoint depth leaves the depth unchanged on failure, so each enclosing level unwinds its own. Previously a RELEASE SAVEPOINT failure during a break out of an aborted nested level escaped cleanup and left the outer transaction open with its work pending.

Verification

  • Behavioral regressions for every failure mode found along the way: nested return (incl. cross-connection and 3-level), both user-catch shapes, Threads.@spawn, a third-party @local_task macro, Distributed.@spawnat/@fetch/@fetchfrom (run locally on worker 1), short-form helpers escaping the block, break/continue, recursion re-entering the same expansion, plain-vs-wrapped flattened-iterator equivalence, and the nested-break-with-aborted-savepoint commit-failure case (asserts the server error surfaces and nothing stays open client- or server-side).
  • Mutation-verified: restoring the original marker behavior fails 6+ tests; removing the finally rollback fails 5.
  • Suite: 1433/1433 locally (Docker integration + TLS fixture); full 18-check CI matrix green on every commit.
  • All three external review threads answered on their respective conversations.

🤖 Generated with Claude Code

The return-rewrite threw an untagged TransactionReturn marker, so the
dynamically nearest @transaction expansion always intercepted it:
- a return inside a NESTED @transaction committed only the inner savepoint,
and the inner expansion's own plain `return` then skipped every enclosing
commit — all levels' work was silently rolled back and the connection was
left inside the outer transaction
- a user try/catch inside the body swallowed the marker and returned the
catch's value instead of the intended return value, silently
- a return inside Threads.@Spawn / @async in the body was rewritten too, so
the task threw the marker instead of producing its value
Each expansion now tags its markers with a compile-time token. A catch that
receives a foreign marker commits its own level and keeps unwinding to the
owning expansion, so an early return commits every enclosing level and
returns exactly once. User catch blocks get a guard injected that rethrows
the marker (a private type no handler can mean to catch). Task-forming
macros are excluded from the rewrite, matching the existing exclusion of
closures. break/continue — which bypass both the commit and any catch — now
commit via a finally, making every non-exceptional exit consistent: only a
thrown exception rolls back. Documented in the docstring.
Regression tests cover nested return, both catch shapes, @Spawn, break,
continue, and recursive re-entry of the same expansion; removing the fix
fails six of them plus downstream testsets poisoned by the stuck-open
transaction.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment threadsrc/Postgres.jl Outdated
h(x) = ... parses as :(=) with a call-shaped left-hand side, not as
:function, so the rewrite's closure exclusion missed it: a return inside a
local short-form helper defined in the @transaction body was rewritten into
a transaction-return marker. Calling such a helper silently early-returned
the ENCLOSING function with the helper's internal value (committing on the
way out), and a helper that escaped the block threw a raw TransactionReturn
at its caller with no expansion active to catch it.
All short-form shapes are skipped (plain, ::T return-type, where-clauses,
qualified names), while ordinary assignments whose right-hand side contains
a return are still rewritten. Also adds @spawnat to the task-macro skip
list — same bug class as @spawn/@async, verified to wrap the marker in a
RemoteException instead of producing the task's value.
Live test: a short-form helper with an internal early return, used inside
the block and after it escapes. Unit pins for every definition shape, the
task macros, and the ordinary-assignment counter-cases. Removing the skip
fails six of them.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment threadsrc/Postgres.jl Outdated
`return` anywhere inside a comprehension or generator — body or iterator
expression — is a lowering error in plain Julia. The rewrite turned it
into a legal `throw` of the transaction-return marker, silently accepting
code that would stop compiling the moment the @transaction wrapper is
removed, and giving it early-return semantics it never legitimately had.
Comprehension, typed-comprehension, generator, and flatten heads are now
left untouched so the construct errors exactly as it does everywhere else.
Nothing valid is lost: a legal comprehension cannot contain a bare
`return`, and nested closures inside one were already excluded. Unit pins
cover all four syntactic shapes plus the counter-case that a `return`
inside an ordinary `for` loop is still rewritten.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment threadsrc/Postgres.jl Outdated
quinnjand others added 2 commits August 7, 2026 13:44
Distributed.@fetch and @fetchfrom wrap their body in a remotely-executed
thunk whose return is the fetched value, exactly like @spawnat — but they
were missing from _TASK_MACROS, so a return inside one was rewritten into
a transaction-return marker. Verified live: the block then throws a
RemoteException wrapping the marker and rolls back, where plain Julia
returns the value.
Also corrects the comprehension-skip rationale in comments: a return in a
comprehension/generator BODY is a lowering error (which the rewrite must
not legalize), while the iterator-expression shapes lowering does accept
behave correctly un-rewritten — they exit the block non-exceptionally and
commit through the expansion's finally, as verified live. And documents at
the token comparison that unconditional returning would be observationally
equivalent today only because every enclosing expansion's finally also
commits; the token check stays as the semantic guarantee.
Independent adversarial verification of the three @transaction commits
(61 live scenarios, plain-Julia baselines, 6 mutations against the full
suite) found no other behavioral gaps.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…nally
Review of the rewrite approach (PR #7 threads) proved its task-macro
allowlist structurally insufficient: any third-party macro that wraps its
body in a task or closure — reproduced with a minimal @local_task — had its
internal returns rewritten into transaction-return markers, throwing
TaskFailedException(TransactionReturn) instead of producing the task's
value. No finite list of standard macros covers user-defined ones.
The expansion's finally already gives plain `return` the intended semantics
with no rewriting at all: a return unwinds through every enclosing
expansion's finally, each committing its level exactly once, innermost
first. User catches cannot intercept a plain return, closures and task
macros keep their ordinary meaning untouched, and the flattened-iterator
form that plain lowering accepts behaves identically wrapped or not. The
marker struct, the AST walker, the try-guard injection, and both skip lists
are deleted.
The finally also now handles its own commit failure: it rolls back the
current level before propagating (commit at savepoint depth leaves depth
unchanged on failure), so every enclosing level — macro expansion or plain
catch — unwinds its own. Previously a RELEASE SAVEPOINT failure during a
break out of a nested level (savepoint aborted by a swallowed server error)
escaped past the enclosing macro's ability to clean up, leaving the outer
transaction open with its work pending.
Behavioral regressions replace the deleted unit AST pins: a third-party
@local_task macro, Distributed @spawnat/@fetch/@fetchfrom run locally on
worker 1, plain-vs-wrapped flattened-iterator equivalence, and the nested
break with an aborted savepoint (asserts the server error surfaces and
nothing stays open client- or server-side; removing the finally rollback
fails five assertions).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@quinnj

Copy link
Copy Markdown
MemberAuthor

READY/CLEAN on exact head 467596923c86b4f483d107174b8a28d4edd7f476.

Independent PostgreSQL 16 retests passed: standard and third-party task macros preserve plain return semantics and commit; a failed finally savepoint commit now unwinds every client/server transaction level and rolls back pending rows; the flattened-iterator case matches plain Julia and commits. All 15 exact-head CI jobs and both Codecov checks are green. The PR is MERGEABLE/CLEAN with no unresolved review threads.

@quinnj
quinnj merged commit 77b9296 into mainAug 7, 2026
17 checks passed
quinnj added a commit that referenced this pull request Aug 10, 2026
Release the 1.0 hardening fix from #7 (@transaction early-return
composition) as a patch on top of the registered 1.0.0.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for freeto 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

@quinnj
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

1.0 hardening: fix @transaction early-return composition - #7

Merged
quinnj merged 5 commits into
mainfrom
release-1.0-hardening
Aug 7, 2026
Merged

1.0 hardening: fix @transaction early-return composition#7
quinnj merged 5 commits into
mainfrom
release-1.0-hardening

Conversation

@quinnj

@quinnjquinnj commented Aug 7, 2026

Copy link
Copy Markdown
Member

Continuation of the 1.0 readiness work from #5, on a fresh branch against merged main: independent adversarial review rounds against a live PostgreSQL, fixing what they surface, until a round comes back clean.

The problem: @transaction early-return was unsound under composition

Round 13 (the first review of #5's merged tail) found that the early-return support rewrote return x into an untagged thrown marker, so the dynamically nearest @transaction expansion always intercepted it. Verified live: nested @transaction + return silently rolled back all levels and left the connection stuck in a transaction; a user try/catch swallowed the marker and returned its own fallback value; return inside task macros threw the marker instead of producing the task's value; break/continue left the transaction open.

The evolution of the fix (rounds 14–16 + external review)

Commits 1–4 fixed this incrementally: per-expansion tokens, guards injected into user catches, and a growing skip list of closure/task-forming constructs (short-form defs, @spawnat, @fetch/@fetchfrom, comprehensions). Each round's reviewer found the next hole in the allowlist.

External review (codex) then proved the endpoint of that trajectory: no finite allowlist can cover third-party task macros (reproduced with a minimal @local_task), and — the key insight — the expansion's finally already gives plain return the intended semantics with no rewriting at all.

Final design (commit 5, net −140 lines)

The marker struct, AST walker, try-guard injection, and both skip lists are deleted. The macro is now just try/catch/finally:

  • Every non-exceptional exit commits — normal completion, return, break, continue. A return unwinds through every enclosing expansion's finally, each committing its level exactly once, innermost first. Only a thrown exception rolls back.
  • Plain Julia semantics everywhere: user catches cannot intercept a plain return; closures, do-blocks, comprehensions, and any task-forming macro (standard or third-party) keep their ordinary meaning untouched.
  • The finally handles its own commit failure (the second P1 from external review): it rolls back the current level before propagating — commit at savepoint depth leaves the depth unchanged on failure, so each enclosing level unwinds its own. Previously a RELEASE SAVEPOINT failure during a break out of an aborted nested level escaped cleanup and left the outer transaction open with its work pending.

Verification

  • Behavioral regressions for every failure mode found along the way: nested return (incl. cross-connection and 3-level), both user-catch shapes, Threads.@spawn, a third-party @local_task macro, Distributed.@spawnat/@fetch/@fetchfrom (run locally on worker 1), short-form helpers escaping the block, break/continue, recursion re-entering the same expansion, plain-vs-wrapped flattened-iterator equivalence, and the nested-break-with-aborted-savepoint commit-failure case (asserts the server error surfaces and nothing stays open client- or server-side).
  • Mutation-verified: restoring the original marker behavior fails 6+ tests; removing the finally rollback fails 5.
  • Suite: 1433/1433 locally (Docker integration + TLS fixture); full 18-check CI matrix green on every commit.
  • All three external review threads answered on their respective conversations.

🤖 Generated with Claude Code

The return-rewrite threw an untagged TransactionReturn marker, so the
dynamically nearest @transaction expansion always intercepted it:
- a return inside a NESTED @transaction committed only the inner savepoint,
and the inner expansion's own plain `return` then skipped every enclosing
commit — all levels' work was silently rolled back and the connection was
left inside the outer transaction
- a user try/catch inside the body swallowed the marker and returned the
catch's value instead of the intended return value, silently
- a return inside Threads.@Spawn / @async in the body was rewritten too, so
the task threw the marker instead of producing its value
Each expansion now tags its markers with a compile-time token. A catch that
receives a foreign marker commits its own level and keeps unwinding to the
owning expansion, so an early return commits every enclosing level and
returns exactly once. User catch blocks get a guard injected that rethrows
the marker (a private type no handler can mean to catch). Task-forming
macros are excluded from the rewrite, matching the existing exclusion of
closures. break/continue — which bypass both the commit and any catch — now
commit via a finally, making every non-exceptional exit consistent: only a
thrown exception rolls back. Documented in the docstring.
Regression tests cover nested return, both catch shapes, @Spawn, break,
continue, and recursive re-entry of the same expansion; removing the fix
fails six of them plus downstream testsets poisoned by the stuck-open
transaction.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment threadsrc/Postgres.jl Outdated
h(x) = ... parses as :(=) with a call-shaped left-hand side, not as
:function, so the rewrite's closure exclusion missed it: a return inside a
local short-form helper defined in the @transaction body was rewritten into
a transaction-return marker. Calling such a helper silently early-returned
the ENCLOSING function with the helper's internal value (committing on the
way out), and a helper that escaped the block threw a raw TransactionReturn
at its caller with no expansion active to catch it.
All short-form shapes are skipped (plain, ::T return-type, where-clauses,
qualified names), while ordinary assignments whose right-hand side contains
a return are still rewritten. Also adds @spawnat to the task-macro skip
list — same bug class as @spawn/@async, verified to wrap the marker in a
RemoteException instead of producing the task's value.
Live test: a short-form helper with an internal early return, used inside
the block and after it escapes. Unit pins for every definition shape, the
task macros, and the ordinary-assignment counter-cases. Removing the skip
fails six of them.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment threadsrc/Postgres.jl Outdated
`return` anywhere inside a comprehension or generator — body or iterator
expression — is a lowering error in plain Julia. The rewrite turned it
into a legal `throw` of the transaction-return marker, silently accepting
code that would stop compiling the moment the @transaction wrapper is
removed, and giving it early-return semantics it never legitimately had.
Comprehension, typed-comprehension, generator, and flatten heads are now
left untouched so the construct errors exactly as it does everywhere else.
Nothing valid is lost: a legal comprehension cannot contain a bare
`return`, and nested closures inside one were already excluded. Unit pins
cover all four syntactic shapes plus the counter-case that a `return`
inside an ordinary `for` loop is still rewritten.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment threadsrc/Postgres.jl Outdated
quinnjand others added 2 commits August 7, 2026 13:44
Distributed.@fetch and @fetchfrom wrap their body in a remotely-executed
thunk whose return is the fetched value, exactly like @spawnat — but they
were missing from _TASK_MACROS, so a return inside one was rewritten into
a transaction-return marker. Verified live: the block then throws a
RemoteException wrapping the marker and rolls back, where plain Julia
returns the value.
Also corrects the comprehension-skip rationale in comments: a return in a
comprehension/generator BODY is a lowering error (which the rewrite must
not legalize), while the iterator-expression shapes lowering does accept
behave correctly un-rewritten — they exit the block non-exceptionally and
commit through the expansion's finally, as verified live. And documents at
the token comparison that unconditional returning would be observationally
equivalent today only because every enclosing expansion's finally also
commits; the token check stays as the semantic guarantee.
Independent adversarial verification of the three @transaction commits
(61 live scenarios, plain-Julia baselines, 6 mutations against the full
suite) found no other behavioral gaps.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…nally
Review of the rewrite approach (PR #7 threads) proved its task-macro
allowlist structurally insufficient: any third-party macro that wraps its
body in a task or closure — reproduced with a minimal @local_task — had its
internal returns rewritten into transaction-return markers, throwing
TaskFailedException(TransactionReturn) instead of producing the task's
value. No finite list of standard macros covers user-defined ones.
The expansion's finally already gives plain `return` the intended semantics
with no rewriting at all: a return unwinds through every enclosing
expansion's finally, each committing its level exactly once, innermost
first. User catches cannot intercept a plain return, closures and task
macros keep their ordinary meaning untouched, and the flattened-iterator
form that plain lowering accepts behaves identically wrapped or not. The
marker struct, the AST walker, the try-guard injection, and both skip lists
are deleted.
The finally also now handles its own commit failure: it rolls back the
current level before propagating (commit at savepoint depth leaves depth
unchanged on failure), so every enclosing level — macro expansion or plain
catch — unwinds its own. Previously a RELEASE SAVEPOINT failure during a
break out of a nested level (savepoint aborted by a swallowed server error)
escaped past the enclosing macro's ability to clean up, leaving the outer
transaction open with its work pending.
Behavioral regressions replace the deleted unit AST pins: a third-party
@local_task macro, Distributed @spawnat/@fetch/@fetchfrom run locally on
worker 1, plain-vs-wrapped flattened-iterator equivalence, and the nested
break with an aborted savepoint (asserts the server error surfaces and
nothing stays open client- or server-side; removing the finally rollback
fails five assertions).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@quinnj

Copy link
Copy Markdown
MemberAuthor

READY/CLEAN on exact head 467596923c86b4f483d107174b8a28d4edd7f476.

Independent PostgreSQL 16 retests passed: standard and third-party task macros preserve plain return semantics and commit; a failed finally savepoint commit now unwinds every client/server transaction level and rolls back pending rows; the flattened-iterator case matches plain Julia and commits. All 15 exact-head CI jobs and both Codecov checks are green. The PR is MERGEABLE/CLEAN with no unresolved review threads.

@quinnj
quinnj merged commit 77b9296 into mainAug 7, 2026
17 checks passed
quinnj added a commit that referenced this pull request Aug 10, 2026
Release the 1.0 hardening fix from #7 (@transaction early-return
composition) as a patch on top of the registered 1.0.0.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for freeto 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

@quinnj
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

1.0 hardening: fix @transaction early-return composition - #7

Merged
quinnj merged 5 commits into
mainfrom
release-1.0-hardening
Aug 7, 2026
Merged

1.0 hardening: fix @transaction early-return composition#7
quinnj merged 5 commits into
mainfrom
release-1.0-hardening

Conversation

@quinnj

@quinnjquinnj commented Aug 7, 2026

Copy link
Copy Markdown
Member

Continuation of the 1.0 readiness work from #5, on a fresh branch against merged main: independent adversarial review rounds against a live PostgreSQL, fixing what they surface, until a round comes back clean.

The problem: @transaction early-return was unsound under composition

Round 13 (the first review of #5's merged tail) found that the early-return support rewrote return x into an untagged thrown marker, so the dynamically nearest @transaction expansion always intercepted it. Verified live: nested @transaction + return silently rolled back all levels and left the connection stuck in a transaction; a user try/catch swallowed the marker and returned its own fallback value; return inside task macros threw the marker instead of producing the task's value; break/continue left the transaction open.

The evolution of the fix (rounds 14–16 + external review)

Commits 1–4 fixed this incrementally: per-expansion tokens, guards injected into user catches, and a growing skip list of closure/task-forming constructs (short-form defs, @spawnat, @fetch/@fetchfrom, comprehensions). Each round's reviewer found the next hole in the allowlist.

External review (codex) then proved the endpoint of that trajectory: no finite allowlist can cover third-party task macros (reproduced with a minimal @local_task), and — the key insight — the expansion's finally already gives plain return the intended semantics with no rewriting at all.

Final design (commit 5, net −140 lines)

The marker struct, AST walker, try-guard injection, and both skip lists are deleted. The macro is now just try/catch/finally:

  • Every non-exceptional exit commits — normal completion, return, break, continue. A return unwinds through every enclosing expansion's finally, each committing its level exactly once, innermost first. Only a thrown exception rolls back.
  • Plain Julia semantics everywhere: user catches cannot intercept a plain return; closures, do-blocks, comprehensions, and any task-forming macro (standard or third-party) keep their ordinary meaning untouched.
  • The finally handles its own commit failure (the second P1 from external review): it rolls back the current level before propagating — commit at savepoint depth leaves the depth unchanged on failure, so each enclosing level unwinds its own. Previously a RELEASE SAVEPOINT failure during a break out of an aborted nested level escaped cleanup and left the outer transaction open with its work pending.

Verification

  • Behavioral regressions for every failure mode found along the way: nested return (incl. cross-connection and 3-level), both user-catch shapes, Threads.@spawn, a third-party @local_task macro, Distributed.@spawnat/@fetch/@fetchfrom (run locally on worker 1), short-form helpers escaping the block, break/continue, recursion re-entering the same expansion, plain-vs-wrapped flattened-iterator equivalence, and the nested-break-with-aborted-savepoint commit-failure case (asserts the server error surfaces and nothing stays open client- or server-side).
  • Mutation-verified: restoring the original marker behavior fails 6+ tests; removing the finally rollback fails 5.
  • Suite: 1433/1433 locally (Docker integration + TLS fixture); full 18-check CI matrix green on every commit.
  • All three external review threads answered on their respective conversations.

🤖 Generated with Claude Code

The return-rewrite threw an untagged TransactionReturn marker, so the
dynamically nearest @transaction expansion always intercepted it:
- a return inside a NESTED @transaction committed only the inner savepoint,
and the inner expansion's own plain `return` then skipped every enclosing
commit — all levels' work was silently rolled back and the connection was
left inside the outer transaction
- a user try/catch inside the body swallowed the marker and returned the
catch's value instead of the intended return value, silently
- a return inside Threads.@Spawn / @async in the body was rewritten too, so
the task threw the marker instead of producing its value
Each expansion now tags its markers with a compile-time token. A catch that
receives a foreign marker commits its own level and keeps unwinding to the
owning expansion, so an early return commits every enclosing level and
returns exactly once. User catch blocks get a guard injected that rethrows
the marker (a private type no handler can mean to catch). Task-forming
macros are excluded from the rewrite, matching the existing exclusion of
closures. break/continue — which bypass both the commit and any catch — now
commit via a finally, making every non-exceptional exit consistent: only a
thrown exception rolls back. Documented in the docstring.
Regression tests cover nested return, both catch shapes, @Spawn, break,
continue, and recursive re-entry of the same expansion; removing the fix
fails six of them plus downstream testsets poisoned by the stuck-open
transaction.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment threadsrc/Postgres.jl Outdated
h(x) = ... parses as :(=) with a call-shaped left-hand side, not as
:function, so the rewrite's closure exclusion missed it: a return inside a
local short-form helper defined in the @transaction body was rewritten into
a transaction-return marker. Calling such a helper silently early-returned
the ENCLOSING function with the helper's internal value (committing on the
way out), and a helper that escaped the block threw a raw TransactionReturn
at its caller with no expansion active to catch it.
All short-form shapes are skipped (plain, ::T return-type, where-clauses,
qualified names), while ordinary assignments whose right-hand side contains
a return are still rewritten. Also adds @spawnat to the task-macro skip
list — same bug class as @spawn/@async, verified to wrap the marker in a
RemoteException instead of producing the task's value.
Live test: a short-form helper with an internal early return, used inside
the block and after it escapes. Unit pins for every definition shape, the
task macros, and the ordinary-assignment counter-cases. Removing the skip
fails six of them.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment threadsrc/Postgres.jl Outdated
`return` anywhere inside a comprehension or generator — body or iterator
expression — is a lowering error in plain Julia. The rewrite turned it
into a legal `throw` of the transaction-return marker, silently accepting
code that would stop compiling the moment the @transaction wrapper is
removed, and giving it early-return semantics it never legitimately had.
Comprehension, typed-comprehension, generator, and flatten heads are now
left untouched so the construct errors exactly as it does everywhere else.
Nothing valid is lost: a legal comprehension cannot contain a bare
`return`, and nested closures inside one were already excluded. Unit pins
cover all four syntactic shapes plus the counter-case that a `return`
inside an ordinary `for` loop is still rewritten.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment threadsrc/Postgres.jl Outdated
quinnjand others added 2 commits August 7, 2026 13:44
Distributed.@fetch and @fetchfrom wrap their body in a remotely-executed
thunk whose return is the fetched value, exactly like @spawnat — but they
were missing from _TASK_MACROS, so a return inside one was rewritten into
a transaction-return marker. Verified live: the block then throws a
RemoteException wrapping the marker and rolls back, where plain Julia
returns the value.
Also corrects the comprehension-skip rationale in comments: a return in a
comprehension/generator BODY is a lowering error (which the rewrite must
not legalize), while the iterator-expression shapes lowering does accept
behave correctly un-rewritten — they exit the block non-exceptionally and
commit through the expansion's finally, as verified live. And documents at
the token comparison that unconditional returning would be observationally
equivalent today only because every enclosing expansion's finally also
commits; the token check stays as the semantic guarantee.
Independent adversarial verification of the three @transaction commits
(61 live scenarios, plain-Julia baselines, 6 mutations against the full
suite) found no other behavioral gaps.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…nally
Review of the rewrite approach (PR #7 threads) proved its task-macro
allowlist structurally insufficient: any third-party macro that wraps its
body in a task or closure — reproduced with a minimal @local_task — had its
internal returns rewritten into transaction-return markers, throwing
TaskFailedException(TransactionReturn) instead of producing the task's
value. No finite list of standard macros covers user-defined ones.
The expansion's finally already gives plain `return` the intended semantics
with no rewriting at all: a return unwinds through every enclosing
expansion's finally, each committing its level exactly once, innermost
first. User catches cannot intercept a plain return, closures and task
macros keep their ordinary meaning untouched, and the flattened-iterator
form that plain lowering accepts behaves identically wrapped or not. The
marker struct, the AST walker, the try-guard injection, and both skip lists
are deleted.
The finally also now handles its own commit failure: it rolls back the
current level before propagating (commit at savepoint depth leaves depth
unchanged on failure), so every enclosing level — macro expansion or plain
catch — unwinds its own. Previously a RELEASE SAVEPOINT failure during a
break out of a nested level (savepoint aborted by a swallowed server error)
escaped past the enclosing macro's ability to clean up, leaving the outer
transaction open with its work pending.
Behavioral regressions replace the deleted unit AST pins: a third-party
@local_task macro, Distributed @spawnat/@fetch/@fetchfrom run locally on
worker 1, plain-vs-wrapped flattened-iterator equivalence, and the nested
break with an aborted savepoint (asserts the server error surfaces and
nothing stays open client- or server-side; removing the finally rollback
fails five assertions).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@quinnj

Copy link
Copy Markdown
MemberAuthor

READY/CLEAN on exact head 467596923c86b4f483d107174b8a28d4edd7f476.

Independent PostgreSQL 16 retests passed: standard and third-party task macros preserve plain return semantics and commit; a failed finally savepoint commit now unwinds every client/server transaction level and rolls back pending rows; the flattened-iterator case matches plain Julia and commits. All 15 exact-head CI jobs and both Codecov checks are green. The PR is MERGEABLE/CLEAN with no unresolved review threads.

@quinnj
quinnj merged commit 77b9296 into mainAug 7, 2026
17 checks passed
quinnj added a commit that referenced this pull request Aug 10, 2026
Release the 1.0 hardening fix from #7 (@transaction early-return
composition) as a patch on top of the registered 1.0.0.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for freeto 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

@quinnj
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

1.0 hardening: fix @transaction early-return composition - #7

Merged
quinnj merged 5 commits into
mainfrom
release-1.0-hardening
Aug 7, 2026
Merged

1.0 hardening: fix @transaction early-return composition#7
quinnj merged 5 commits into
mainfrom
release-1.0-hardening

Conversation

@quinnj

@quinnjquinnj commented Aug 7, 2026

Copy link
Copy Markdown
Member

Continuation of the 1.0 readiness work from #5, on a fresh branch against merged main: independent adversarial review rounds against a live PostgreSQL, fixing what they surface, until a round comes back clean.

The problem: @transaction early-return was unsound under composition

Round 13 (the first review of #5's merged tail) found that the early-return support rewrote return x into an untagged thrown marker, so the dynamically nearest @transaction expansion always intercepted it. Verified live: nested @transaction + return silently rolled back all levels and left the connection stuck in a transaction; a user try/catch swallowed the marker and returned its own fallback value; return inside task macros threw the marker instead of producing the task's value; break/continue left the transaction open.

The evolution of the fix (rounds 14–16 + external review)

Commits 1–4 fixed this incrementally: per-expansion tokens, guards injected into user catches, and a growing skip list of closure/task-forming constructs (short-form defs, @spawnat, @fetch/@fetchfrom, comprehensions). Each round's reviewer found the next hole in the allowlist.

External review (codex) then proved the endpoint of that trajectory: no finite allowlist can cover third-party task macros (reproduced with a minimal @local_task), and — the key insight — the expansion's finally already gives plain return the intended semantics with no rewriting at all.

Final design (commit 5, net −140 lines)

The marker struct, AST walker, try-guard injection, and both skip lists are deleted. The macro is now just try/catch/finally:

  • Every non-exceptional exit commits — normal completion, return, break, continue. A return unwinds through every enclosing expansion's finally, each committing its level exactly once, innermost first. Only a thrown exception rolls back.
  • Plain Julia semantics everywhere: user catches cannot intercept a plain return; closures, do-blocks, comprehensions, and any task-forming macro (standard or third-party) keep their ordinary meaning untouched.
  • The finally handles its own commit failure (the second P1 from external review): it rolls back the current level before propagating — commit at savepoint depth leaves the depth unchanged on failure, so each enclosing level unwinds its own. Previously a RELEASE SAVEPOINT failure during a break out of an aborted nested level escaped cleanup and left the outer transaction open with its work pending.

Verification

  • Behavioral regressions for every failure mode found along the way: nested return (incl. cross-connection and 3-level), both user-catch shapes, Threads.@spawn, a third-party @local_task macro, Distributed.@spawnat/@fetch/@fetchfrom (run locally on worker 1), short-form helpers escaping the block, break/continue, recursion re-entering the same expansion, plain-vs-wrapped flattened-iterator equivalence, and the nested-break-with-aborted-savepoint commit-failure case (asserts the server error surfaces and nothing stays open client- or server-side).
  • Mutation-verified: restoring the original marker behavior fails 6+ tests; removing the finally rollback fails 5.
  • Suite: 1433/1433 locally (Docker integration + TLS fixture); full 18-check CI matrix green on every commit.
  • All three external review threads answered on their respective conversations.

🤖 Generated with Claude Code

The return-rewrite threw an untagged TransactionReturn marker, so the
dynamically nearest @transaction expansion always intercepted it:
- a return inside a NESTED @transaction committed only the inner savepoint,
and the inner expansion's own plain `return` then skipped every enclosing
commit — all levels' work was silently rolled back and the connection was
left inside the outer transaction
- a user try/catch inside the body swallowed the marker and returned the
catch's value instead of the intended return value, silently
- a return inside Threads.@Spawn / @async in the body was rewritten too, so
the task threw the marker instead of producing its value
Each expansion now tags its markers with a compile-time token. A catch that
receives a foreign marker commits its own level and keeps unwinding to the
owning expansion, so an early return commits every enclosing level and
returns exactly once. User catch blocks get a guard injected that rethrows
the marker (a private type no handler can mean to catch). Task-forming
macros are excluded from the rewrite, matching the existing exclusion of
closures. break/continue — which bypass both the commit and any catch — now
commit via a finally, making every non-exceptional exit consistent: only a
thrown exception rolls back. Documented in the docstring.
Regression tests cover nested return, both catch shapes, @Spawn, break,
continue, and recursive re-entry of the same expansion; removing the fix
fails six of them plus downstream testsets poisoned by the stuck-open
transaction.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment threadsrc/Postgres.jl Outdated
h(x) = ... parses as :(=) with a call-shaped left-hand side, not as
:function, so the rewrite's closure exclusion missed it: a return inside a
local short-form helper defined in the @transaction body was rewritten into
a transaction-return marker. Calling such a helper silently early-returned
the ENCLOSING function with the helper's internal value (committing on the
way out), and a helper that escaped the block threw a raw TransactionReturn
at its caller with no expansion active to catch it.
All short-form shapes are skipped (plain, ::T return-type, where-clauses,
qualified names), while ordinary assignments whose right-hand side contains
a return are still rewritten. Also adds @spawnat to the task-macro skip
list — same bug class as @spawn/@async, verified to wrap the marker in a
RemoteException instead of producing the task's value.
Live test: a short-form helper with an internal early return, used inside
the block and after it escapes. Unit pins for every definition shape, the
task macros, and the ordinary-assignment counter-cases. Removing the skip
fails six of them.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment threadsrc/Postgres.jl Outdated
`return` anywhere inside a comprehension or generator — body or iterator
expression — is a lowering error in plain Julia. The rewrite turned it
into a legal `throw` of the transaction-return marker, silently accepting
code that would stop compiling the moment the @transaction wrapper is
removed, and giving it early-return semantics it never legitimately had.
Comprehension, typed-comprehension, generator, and flatten heads are now
left untouched so the construct errors exactly as it does everywhere else.
Nothing valid is lost: a legal comprehension cannot contain a bare
`return`, and nested closures inside one were already excluded. Unit pins
cover all four syntactic shapes plus the counter-case that a `return`
inside an ordinary `for` loop is still rewritten.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment threadsrc/Postgres.jl Outdated
quinnjand others added 2 commits August 7, 2026 13:44
Distributed.@fetch and @fetchfrom wrap their body in a remotely-executed
thunk whose return is the fetched value, exactly like @spawnat — but they
were missing from _TASK_MACROS, so a return inside one was rewritten into
a transaction-return marker. Verified live: the block then throws a
RemoteException wrapping the marker and rolls back, where plain Julia
returns the value.
Also corrects the comprehension-skip rationale in comments: a return in a
comprehension/generator BODY is a lowering error (which the rewrite must
not legalize), while the iterator-expression shapes lowering does accept
behave correctly un-rewritten — they exit the block non-exceptionally and
commit through the expansion's finally, as verified live. And documents at
the token comparison that unconditional returning would be observationally
equivalent today only because every enclosing expansion's finally also
commits; the token check stays as the semantic guarantee.
Independent adversarial verification of the three @transaction commits
(61 live scenarios, plain-Julia baselines, 6 mutations against the full
suite) found no other behavioral gaps.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…nally
Review of the rewrite approach (PR #7 threads) proved its task-macro
allowlist structurally insufficient: any third-party macro that wraps its
body in a task or closure — reproduced with a minimal @local_task — had its
internal returns rewritten into transaction-return markers, throwing
TaskFailedException(TransactionReturn) instead of producing the task's
value. No finite list of standard macros covers user-defined ones.
The expansion's finally already gives plain `return` the intended semantics
with no rewriting at all: a return unwinds through every enclosing
expansion's finally, each committing its level exactly once, innermost
first. User catches cannot intercept a plain return, closures and task
macros keep their ordinary meaning untouched, and the flattened-iterator
form that plain lowering accepts behaves identically wrapped or not. The
marker struct, the AST walker, the try-guard injection, and both skip lists
are deleted.
The finally also now handles its own commit failure: it rolls back the
current level before propagating (commit at savepoint depth leaves depth
unchanged on failure), so every enclosing level — macro expansion or plain
catch — unwinds its own. Previously a RELEASE SAVEPOINT failure during a
break out of a nested level (savepoint aborted by a swallowed server error)
escaped past the enclosing macro's ability to clean up, leaving the outer
transaction open with its work pending.
Behavioral regressions replace the deleted unit AST pins: a third-party
@local_task macro, Distributed @spawnat/@fetch/@fetchfrom run locally on
worker 1, plain-vs-wrapped flattened-iterator equivalence, and the nested
break with an aborted savepoint (asserts the server error surfaces and
nothing stays open client- or server-side; removing the finally rollback
fails five assertions).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@quinnj

Copy link
Copy Markdown
MemberAuthor

READY/CLEAN on exact head 467596923c86b4f483d107174b8a28d4edd7f476.

Independent PostgreSQL 16 retests passed: standard and third-party task macros preserve plain return semantics and commit; a failed finally savepoint commit now unwinds every client/server transaction level and rolls back pending rows; the flattened-iterator case matches plain Julia and commits. All 15 exact-head CI jobs and both Codecov checks are green. The PR is MERGEABLE/CLEAN with no unresolved review threads.

@quinnj
quinnj merged commit 77b9296 into mainAug 7, 2026
17 checks passed
quinnj added a commit that referenced this pull request Aug 10, 2026
Release the 1.0 hardening fix from #7 (@transaction early-return
composition) as a patch on top of the registered 1.0.0.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for freeto 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

@quinnj
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

1.0 hardening: fix @transaction early-return composition - #7

Merged
quinnj merged 5 commits into
mainfrom
release-1.0-hardening
Aug 7, 2026
Merged

1.0 hardening: fix @transaction early-return composition#7
quinnj merged 5 commits into
mainfrom
release-1.0-hardening

Conversation

@quinnj

@quinnjquinnj commented Aug 7, 2026

Copy link
Copy Markdown
Member

Continuation of the 1.0 readiness work from #5, on a fresh branch against merged main: independent adversarial review rounds against a live PostgreSQL, fixing what they surface, until a round comes back clean.

The problem: @transaction early-return was unsound under composition

Round 13 (the first review of #5's merged tail) found that the early-return support rewrote return x into an untagged thrown marker, so the dynamically nearest @transaction expansion always intercepted it. Verified live: nested @transaction + return silently rolled back all levels and left the connection stuck in a transaction; a user try/catch swallowed the marker and returned its own fallback value; return inside task macros threw the marker instead of producing the task's value; break/continue left the transaction open.

The evolution of the fix (rounds 14–16 + external review)

Commits 1–4 fixed this incrementally: per-expansion tokens, guards injected into user catches, and a growing skip list of closure/task-forming constructs (short-form defs, @spawnat, @fetch/@fetchfrom, comprehensions). Each round's reviewer found the next hole in the allowlist.

External review (codex) then proved the endpoint of that trajectory: no finite allowlist can cover third-party task macros (reproduced with a minimal @local_task), and — the key insight — the expansion's finally already gives plain return the intended semantics with no rewriting at all.

Final design (commit 5, net −140 lines)

The marker struct, AST walker, try-guard injection, and both skip lists are deleted. The macro is now just try/catch/finally:

  • Every non-exceptional exit commits — normal completion, return, break, continue. A return unwinds through every enclosing expansion's finally, each committing its level exactly once, innermost first. Only a thrown exception rolls back.
  • Plain Julia semantics everywhere: user catches cannot intercept a plain return; closures, do-blocks, comprehensions, and any task-forming macro (standard or third-party) keep their ordinary meaning untouched.
  • The finally handles its own commit failure (the second P1 from external review): it rolls back the current level before propagating — commit at savepoint depth leaves the depth unchanged on failure, so each enclosing level unwinds its own. Previously a RELEASE SAVEPOINT failure during a break out of an aborted nested level escaped cleanup and left the outer transaction open with its work pending.

Verification

  • Behavioral regressions for every failure mode found along the way: nested return (incl. cross-connection and 3-level), both user-catch shapes, Threads.@spawn, a third-party @local_task macro, Distributed.@spawnat/@fetch/@fetchfrom (run locally on worker 1), short-form helpers escaping the block, break/continue, recursion re-entering the same expansion, plain-vs-wrapped flattened-iterator equivalence, and the nested-break-with-aborted-savepoint commit-failure case (asserts the server error surfaces and nothing stays open client- or server-side).
  • Mutation-verified: restoring the original marker behavior fails 6+ tests; removing the finally rollback fails 5.
  • Suite: 1433/1433 locally (Docker integration + TLS fixture); full 18-check CI matrix green on every commit.
  • All three external review threads answered on their respective conversations.

🤖 Generated with Claude Code

The return-rewrite threw an untagged TransactionReturn marker, so the
dynamically nearest @transaction expansion always intercepted it:
- a return inside a NESTED @transaction committed only the inner savepoint,
and the inner expansion's own plain `return` then skipped every enclosing
commit — all levels' work was silently rolled back and the connection was
left inside the outer transaction
- a user try/catch inside the body swallowed the marker and returned the
catch's value instead of the intended return value, silently
- a return inside Threads.@Spawn / @async in the body was rewritten too, so
the task threw the marker instead of producing its value
Each expansion now tags its markers with a compile-time token. A catch that
receives a foreign marker commits its own level and keeps unwinding to the
owning expansion, so an early return commits every enclosing level and
returns exactly once. User catch blocks get a guard injected that rethrows
the marker (a private type no handler can mean to catch). Task-forming
macros are excluded from the rewrite, matching the existing exclusion of
closures. break/continue — which bypass both the commit and any catch — now
commit via a finally, making every non-exceptional exit consistent: only a
thrown exception rolls back. Documented in the docstring.
Regression tests cover nested return, both catch shapes, @Spawn, break,
continue, and recursive re-entry of the same expansion; removing the fix
fails six of them plus downstream testsets poisoned by the stuck-open
transaction.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment threadsrc/Postgres.jl Outdated
h(x) = ... parses as :(=) with a call-shaped left-hand side, not as
:function, so the rewrite's closure exclusion missed it: a return inside a
local short-form helper defined in the @transaction body was rewritten into
a transaction-return marker. Calling such a helper silently early-returned
the ENCLOSING function with the helper's internal value (committing on the
way out), and a helper that escaped the block threw a raw TransactionReturn
at its caller with no expansion active to catch it.
All short-form shapes are skipped (plain, ::T return-type, where-clauses,
qualified names), while ordinary assignments whose right-hand side contains
a return are still rewritten. Also adds @spawnat to the task-macro skip
list — same bug class as @spawn/@async, verified to wrap the marker in a
RemoteException instead of producing the task's value.
Live test: a short-form helper with an internal early return, used inside
the block and after it escapes. Unit pins for every definition shape, the
task macros, and the ordinary-assignment counter-cases. Removing the skip
fails six of them.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment threadsrc/Postgres.jl Outdated
`return` anywhere inside a comprehension or generator — body or iterator
expression — is a lowering error in plain Julia. The rewrite turned it
into a legal `throw` of the transaction-return marker, silently accepting
code that would stop compiling the moment the @transaction wrapper is
removed, and giving it early-return semantics it never legitimately had.
Comprehension, typed-comprehension, generator, and flatten heads are now
left untouched so the construct errors exactly as it does everywhere else.
Nothing valid is lost: a legal comprehension cannot contain a bare
`return`, and nested closures inside one were already excluded. Unit pins
cover all four syntactic shapes plus the counter-case that a `return`
inside an ordinary `for` loop is still rewritten.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment threadsrc/Postgres.jl Outdated
quinnjand others added 2 commits August 7, 2026 13:44
Distributed.@fetch and @fetchfrom wrap their body in a remotely-executed
thunk whose return is the fetched value, exactly like @spawnat — but they
were missing from _TASK_MACROS, so a return inside one was rewritten into
a transaction-return marker. Verified live: the block then throws a
RemoteException wrapping the marker and rolls back, where plain Julia
returns the value.
Also corrects the comprehension-skip rationale in comments: a return in a
comprehension/generator BODY is a lowering error (which the rewrite must
not legalize), while the iterator-expression shapes lowering does accept
behave correctly un-rewritten — they exit the block non-exceptionally and
commit through the expansion's finally, as verified live. And documents at
the token comparison that unconditional returning would be observationally
equivalent today only because every enclosing expansion's finally also
commits; the token check stays as the semantic guarantee.
Independent adversarial verification of the three @transaction commits
(61 live scenarios, plain-Julia baselines, 6 mutations against the full
suite) found no other behavioral gaps.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…nally
Review of the rewrite approach (PR #7 threads) proved its task-macro
allowlist structurally insufficient: any third-party macro that wraps its
body in a task or closure — reproduced with a minimal @local_task — had its
internal returns rewritten into transaction-return markers, throwing
TaskFailedException(TransactionReturn) instead of producing the task's
value. No finite list of standard macros covers user-defined ones.
The expansion's finally already gives plain `return` the intended semantics
with no rewriting at all: a return unwinds through every enclosing
expansion's finally, each committing its level exactly once, innermost
first. User catches cannot intercept a plain return, closures and task
macros keep their ordinary meaning untouched, and the flattened-iterator
form that plain lowering accepts behaves identically wrapped or not. The
marker struct, the AST walker, the try-guard injection, and both skip lists
are deleted.
The finally also now handles its own commit failure: it rolls back the
current level before propagating (commit at savepoint depth leaves depth
unchanged on failure), so every enclosing level — macro expansion or plain
catch — unwinds its own. Previously a RELEASE SAVEPOINT failure during a
break out of a nested level (savepoint aborted by a swallowed server error)
escaped past the enclosing macro's ability to clean up, leaving the outer
transaction open with its work pending.
Behavioral regressions replace the deleted unit AST pins: a third-party
@local_task macro, Distributed @spawnat/@fetch/@fetchfrom run locally on
worker 1, plain-vs-wrapped flattened-iterator equivalence, and the nested
break with an aborted savepoint (asserts the server error surfaces and
nothing stays open client- or server-side; removing the finally rollback
fails five assertions).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@quinnj

Copy link
Copy Markdown
MemberAuthor

READY/CLEAN on exact head 467596923c86b4f483d107174b8a28d4edd7f476.

Independent PostgreSQL 16 retests passed: standard and third-party task macros preserve plain return semantics and commit; a failed finally savepoint commit now unwinds every client/server transaction level and rolls back pending rows; the flattened-iterator case matches plain Julia and commits. All 15 exact-head CI jobs and both Codecov checks are green. The PR is MERGEABLE/CLEAN with no unresolved review threads.

@quinnj
quinnj merged commit 77b9296 into mainAug 7, 2026
17 checks passed
quinnj added a commit that referenced this pull request Aug 10, 2026
Release the 1.0 hardening fix from #7 (@transaction early-return
composition) as a patch on top of the registered 1.0.0.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for freeto 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

@quinnj