docs: final design assessment — post-implementation consistency pass#105
Merged
Merged
Conversation
Whole-system read of the spec and every package after the #45–#69 implementation batch, hunting for holes the issue-by-issue review missed and inconsistencies introduced by landing decisions one issue at a time. Headline findings: the _attachment@1 create path bypasses #51's reference-conveyance gate (hash-confirmation oracle), permissionEqual() predates #58's role field so role-only ACL changes silently no-op, canAccessFile()/ensureOwnerEntity() violate the #50 exhaustive-pagination rule internally, snapshot+mutate non-atomicity can permanently brick a record's mutations, and the conformance fixtures use ids the #55 validation rules require servers to reject. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XS3tQxohWFqeRgDxmyBiV2
This was referenced Jul 20, 2026
7 tasks
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.
Adds
docs/design-assessment-2026-07.md— a whole-system read of the spec and every package after the #45–#69 implementation batch, hunting for holes the issue-by-issue review missed and inconsistencies introduced by landing decisions one issue at a time. Nothing is filed as an issue yet; the doc is the deliverable to react to, per the "discussion first, filing second" convention.Nothing but docs changes here — no code touched.
Verdict
The design philosophy survived implementation intact. The invariant layering (#67/#68), error-taxonomy round-trip (#53), two-relations split (#54/#68), and attachment determinism/backstop pairing (#65/#66) are all faithful. The 25 findings cluster at the seams where two separately-implemented decisions meet.
The four I'd treat as urgent
_attachment@1creation bypasses ScopedStack.create() passes caller options through unchecked — attachment-association escalation, parent/app spoofing #51's reference gate. Creating a metadata record is itself access-conveying (satisfiescanAccessFile()'s "uploaded it themselves" clause), but nothing gates it — so an uploader can mint a record for a guessed SHA-256 and download those bytes. The exact hash-confirmation oracle ScopedStack.create() passes caller options through unchecked — attachment-association escalation, parent/app spoofing #51 was written to prevent; the mimeType-conflict error even leaks the established type. Needs a design decision (I sketch an upload-receipt approach) before anyone codes it.permissionEqual()never learned about Group roles are decorative: admin should mean "can manage the group" #58'srole. Tightening a group ACL by addingrole: 'admin'is judged a no-op and silently dropped while reporting success. A failed tightening of access; one line.canAccessFile()andensureOwnerEntity()both treat one bounded query page as complete → false permission denials, and duplicate owner cards past ~50 records.Stack-level call.Plus D1 [bug] — the conformance fixtures use ids (
rec-1, fileIdabc123) that #55's validation rules require a conformant server to reject, so the enforcement suite and the rules it enforces can't both pass; and the newest contracts (If-Match/412, schema_drift,_configprotection, 401-vs-403) have no fixtures at all.Structure
entityIdinvariant leaks (Polish batch: owner entityId stamping, unused timezone, lenient date validation, silent null content filter #69/_grant records store the grantee in entityId, the authorship field — move it into GrantContent #57)Closes with a fix order grouped by shared root cause. Deliberately skips everything already tracked open (#3/#15/#16/#72/#90/#93).
🤖 Generated with Claude Code
https://claude.ai/code/session_01XS3tQxohWFqeRgDxmyBiV2
Generated by Claude Code