-
Notifications
You must be signed in to change notification settings - Fork 1.5k
refactor(conversations,attachments): fix the conversations/threads naming trap and widen attachments (WS5) #7005
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
Closed
Changes from all commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
c0a6864
refactor(conversations): fix the conversations/threads naming trap (WS5)
BenKurrek aed1429
refactor(attachments): widen ironclaw_attachments to own its ports an…
BenKurrek a9a32dc
docs(target-arch): record the WS5 naming-trap and attachments-widenin…
BenKurrek 39a5870
docs(conversations): state the threads boundary in the crate doc
BenKurrek bbdf1e8
fix(attachments,conversations,product): close the review gaps on the …
BenKurrek 7fd3998
test(ws5): close the changed-coverage gate on the naming-trap slice
BenKurrek 4cd166f
fix(conversations): keep the durable grammar so the rename survives a…
BenKurrek 5c12b1f
docs(conversations): spell the binding-identity guardrail topic_id
BenKurrek b169571
docs(target-arch): correct the WS5 durable-grammar row after review
BenKurrek 5b85634
test(product): cover the attachment reader's product-surface error ta…
BenKurrek 5f1867a
test(product): replace a vacuous non-leak assertion with the sanitize…
BenKurrek File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
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
390 changes: 390 additions & 0 deletions
390
crates/ironclaw_architecture/tests/reborn_conversations_threads_attachments.rs
Large diffs are not rendered by default.
Oops, something went wrong.
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
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
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
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
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
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
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Medium — Reconcile guidance with the widened attachments boundary.
The diff makes ironclaw_attachments own and export the ports, default lander, and advertised capabilities, while existing crate-map, product-contracts, and WebUI guidance still describes the previous ownership/dependency boundary. Those instructions conflict with the refactor's new boundary.
Fix: Update the crate map, product-contracts module documentation, and WebUI AGENTS/CLAUDE dependency guidance to describe the new attachment owner and explicitly allow the justified direct WebUI edge.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two of the three were stale and are fixed; the third was already updated in this PR.
4cd166f727.Fixed — the crate map.
crates/AGENTS.md:87describedironclaw_attachmentsas "the single inbound-attachment landing routine" and nothing else, which is the pre-widening boundary. It now names what the crate actually owns after this PR: the ports (InboundAttachmentLander,InboundAttachmentReader,AttachmentCleanupReport), the defaultProjectScopedAttachmentLander, and the size ceilings + advertisedAttachmentCapabilitiesthat WebUI and the OpenAI-compatible adapter read. The "does not own" column now carries the reader carve-out, since that is the part of the row a reader is most likely to get wrong.Fixed — the WebUI dependency guidance.
crates/ironclaw_webui/AGENTS.md:68-75enumerated the allowed workspace edges and closed with "any other workspace-crate edge requires anironclaw_architectureboundary-test update plus explicit PR rationale".ironclaw_attachmentswas not in that list, so the guidance contradicted the manifest. It is now listed with the justification scoped to what the edge is actually for — the advertised ceilings only, so the transport reads the same constants the landing routine enforces instead of keeping a second copy.Already correct — the product-contracts module documentation.
crates/ironclaw_product_contracts/src/inbound_requests.rs:15-17already points at the new owner:I re-read the crate's other attachment-touching modules (
inbound.rs,product_wire.rs,workspace_views.rs,surface.rs) and found no ownership claim that survived the move, so no edit there. If you were pointing at a specific line I have not found, say which and I will take it.Note this is a different ask from the earlier CodeRabbit thread on
Cargo.toml:39, which I refuted: that one named acrates/ironclaw_webui/tests/reborn_dependency_boundaries.rsthat does not exist and treated a blocklist as an allowlist. Yours is about the guidance being inconsistent with the new boundary, and it was.