fix(api): UpdateNote reserves updated_at/supersedes/supersede_reason from the caller's fragment #137

2026-09-23T08:16:33Z

mathias commented

2026-09-23 08:16:33 +00:00

Closes #125, a non-blocking observation from #111's review decided now rather than left implicit.

UpdateNote's merge loop applied every field from the caller's leading frontmatter fragment onto the restamped note, including updated_at/supersedes/supersede_reason. updated_at/supersedes were already safe — both get unconditionally re-set right after the loop regardless — but supersede_reason is only re-set when opts.Reason is non-empty, so a caller could set it via its own fragment and have it survive even with the Reason option left empty, bypassing the one intended entry point for that field.

Decision

These three fields are this function's own output, not caller-settable, full stop — skip all three in the merge loop rather than lean on the other two happening to be safe via call order. Makes the invariant explicit at the one place it could be silently broken by a future reordering.

Verification

task check exit 0. Flip-tested: reverting the skip to just l.key == "" lets the caller's supersede_reason survive with Reason empty — the new test catches it. Full module suite green.

Co-Authored-By: Claude Sonnet 5 noreply@anthropic.com

Created via git-mcp on behalf of @mathias

Closes #125, a non-blocking observation from #111's review decided now rather than left implicit. `UpdateNote`'s merge loop applied every field from the caller's leading frontmatter fragment onto the restamped note, including `updated_at`/`supersedes`/`supersede_reason`. `updated_at`/`supersedes` were already safe — both get unconditionally re-set right after the loop regardless — but `supersede_reason` is only re-set when `opts.Reason` is non-empty, so a caller could set it via its own fragment and have it survive even with the `Reason` option left empty, bypassing the one intended entry point for that field. ## Decision These three fields are this function's own output, not caller-settable, full stop — skip all three in the merge loop rather than lean on the other two happening to be safe via call order. Makes the invariant explicit at the one place it could be silently broken by a future reordering. ## Verification `task check` exit 0. Flip-tested: reverting the skip to just `l.key == ""` lets the caller's `supersede_reason` survive with `Reason` empty — the new test catches it. Full module suite green. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --- _Created via git-mcp on behalf of @mathias_

mathias added 1 commit 2026-09-23 08:16:33 +00:00

fix(api): UpdateNote reserves updated_at/supersedes/supersede_reason from the caller's fragment

CI / Lint / Test / Vet (pull_request) Successful in 32s

Details

CI / Mirror to GitHub (pull_request) Skipped

Details

fb69f4bf3d

hyperguild#125, found in #111's review as a non-blocking observation,
decided now rather than left implicit: UpdateNote's merge loop applied
every field from the caller's leading frontmatter fragment onto the
restamped note, including updated_at/supersedes/supersede_reason.
updated_at and supersedes were already safe -- both get unconditionally
re-set right after the loop regardless of what the loop did with them
-- but supersede_reason is only re-set when opts.Reason is non-empty, so
a caller could set it via its own fragment and have it survive even
with the Reason option left empty, bypassing the one intended entry
point for that field.

Decision: these three fields are this function's own output, not
caller-settable, full stop -- skip all three in the merge loop rather
than lean on the other two happening to be safe via call order. Makes
the invariant explicit at the one place it could be silently broken by
a future reordering, instead of implicit in which fm.set calls happen
to run after the loop.

Flip-tested: reverting the skip to just `l.key == ""` lets the caller's
supersede_reason survive with Reason empty -- the new test catches it.
Full module suite green.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

mathias merged commit a5f502eb9c into main

2026-09-23 08:18:30 +00:00

mathias referenced this pull request

2026-09-23 08:18:51 +00:00

UpdateNote: supersede_reason settable via caller's own frontmatter fragment, bypassing the Reason option #125

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: mathias/hyperguild#137