feat(plan-eng-review): flag optimistic-UI on a server-gated action as a code-quality check#1874
Open
timlinnet wants to merge 1 commit into
Open
feat(plan-eng-review): flag optimistic-UI on a server-gated action as a code-quality check#1874timlinnet wants to merge 1 commit into
timlinnet wants to merge 1 commit into
Conversation
… a code-quality check Adds one Code-quality-review bullet: for an action whose real effect is gated or actuated server-side (auth/permission/validation), check that the client awaits and surfaces the real result rather than optimistically assuming success. Optimistic status + a swallowed rejection renders as a false success — a green check while the server quietly rejected and nothing happened. The catalog already has general "silent failure / swallowed error" framing in the adversarial /ship pass; this names the specific, common client↔server variant so an eng-review reviewer checks the effect landed, not the toast. Source edited in the section template; generated section doc regenerated via `bun run gen:skill-docs`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Merging to
After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here |
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
2 participants
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 — One bullet added to eng-review's Code quality review checklist: for an action whose real effect is gated/actuated server-side (auth, permission, validation), check that the client awaits and surfaces the real result instead of optimistically assuming success.
Why — Hit this on a live bug: an approval action was wired to the user who clicked it, but only an admin could grant it. The UI optimistically marked it done and showed a success toast while the server returned 403 — green check, nothing granted, no admin ever notified. eng-review slid past it because it fails closed (dodges the security lens) and looks like success (dodges the failure-mode lens). The catalog already has general "silent failure / swallowed error" framing in the adversarial
/shippass, but not this specific, common client↔server variant.Change — One bullet. Edited the source template (
plan-eng-review/sections/review-sections.md.tmpl) and regenerated the committed section doc viabun run gen:skill-docsso.mdand.tmplstay in sync. No version bump — left to your release flow. Template/section/validation tests pass locally:template-context-parity,section-manifest-consistency,skill-validation,skill-size-budget,parity-sectioned(389 pass).