fix: deny always blocks; rule link id, resume labels, moderation copy#13
Merged
Conversation
four fixes from one debugging session, confirmed against prod (read-only) on scratch #83. - resume: the verdict is the maintainer's decision, not graph traversal. deny always blocks — a deny edge drawn to a non-block action (discord), or to nowhere, no longer resumes to pass. side-effect edges still conduct; a block action floors in when the graph produced none. approve resumes to the graph's verdict. extracted resolveResumeOutcome (pure, unit-tested). - rules: the "edit in workflow" link built the definition wire id, not the editor's row id (two separate generateId() calls at create), so it 404'd. map wire->row via listEnabledWorkflowRows. - run view: resumed steps carry a :resume suffix the snapshot label map lacks — strip it so the moderation node and its actions label correctly; label the synthetic deny-floor step as block. - moderation queue: toast names the outcome, not the mechanism. regression: resume-verdict.test.ts covers deny -> non-block action -> block. moderation.integration.test.ts needs docker; ci runs it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
what
four fixes from one debugging session, confirmed against prod (read-only) on scratch #83.
deny always blocks (the security one)
a maintainer's deny produced a passing check. the edited workflow's send-to-moderation node routed its deny edge to a
discordaction with no block anywhere, so deny resumed topass. the T4 deny-floor only checked that a deny edge existed, not that it blocked. now the verdict is the maintainer's decision, not the graph's traversal: deny always blocks, side-effect edges still conduct (discord/webhook fire), and a block action floors in when the graph produced none. approve resumes to the graph's verdict. logic lives in a pureresolveResumeOutcome.rule "edit in workflow" link
the link built the workflow definition's wire id, not the editor route's row id — two separate
generateId()calls at create, so the link 404'd at a workflow that "doesn't exist". map wire->row vialistEnabledWorkflowRows. the badge was always accurate; only the link id was wrong.resume step labels
resumed steps carry a
:resumesuffix the snapshot label map doesn't, so the moderation node and its downstream actions rendered as bareaction. strip the suffix before lookup; label the synthetic deny-floor step asblock.moderation toast copy
names the outcome ("denied — this change is blocked" / "approved — this change can proceed"), not the mechanism.
tests
resume-verdict.test.ts(new): deny -> non-block action (discord) -> still blocks. the exact prod shape. green.moderation.integration.test.ts: floor path end-to-end; updated the deny-floor assertion string. needs docker; ci runs it.scope note
scoped to 7 files. this windows checkout has unrelated local changes (bun.lock rewritten by bun 1.3.2, nitro/dev shims, a pg-boss error handler, migrate.ts path fix) left uncommitted on purpose — not part of this work, and committing the lockfile would fork it.