feat: Phase 1 attachment foundation – chat.Document, attachment.Decide, CapabilityTable#2607
Open
simonferquel-clanker wants to merge 2 commits intodocker:mainfrom
Open
Conversation
…e, CapabilityTable - Add MessagePartTypeDocument constant and Document/DocumentSource types to pkg/chat - Extend MessagePart with Document field - New pkg/attachment package with Strategy enum, Capability, CapabilityTable, Decide function, TXTEnvelope helper, FetchURL, and Advisor interface - Table-driven tests covering all Decide branches (6 strategy outcomes + MIME miss) - httptest-based tests for FetchURL (success, 404, 500, cancelled ctx, binary) Closes docker#2595 Assisted-By: docker-agent
- B1: TXTEnvelope attribute renamed from type= to mime-type= (format string, godoc example, and test assertion updated) - S2: InlineData check reverted to nil comparison (spec behaviour); new test case locks in that non-nil empty slice → StrategyB64 - S3: Decide godoc now documents URL > InlineData > InlineText precedence, and notes intentional non-empty reason strings for FetchAsB64/FetchAsTXT fallbacks (S1 decision) - S4: double doc.Source.URL check collapsed into single switch block - N1/N2: TODO comments added to fetch.go for per-runtime timeout configurability and SSRF mitigations in Phase 2 Assisted-By: docker-agent
Contributor
|
We can't land this before part 2, proceed with it in the same or |
|
❌ PR Review Failed — The review agent encountered an error and could not complete the review. View logs. |
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.
Summary
Foundation PR for the generalized attachment system (Phase 1, PR 1 of 3).
No provider changes, no embedder changes — pure infrastructure.
Closes #2595
Changes
pkg/chatMessagePartTypeDocumentconstant (added to the existingMessagePartTypeconst block inchat.go)document.go:DocumentSourcestruct (three exclusive variants:InlineText,InlineData,URL) andDocumentstruct (Name,MimeType,Size,Source)MessagePartextended withDocument *Documentfieldpkg/attachment(new package)attachment.go:Strategyenum (Drop/TXT/B64/URL/FetchAsB64/FetchAsTXT),Capability,CapabilityTable, pureDecide(doc, table)function,TXTEnvelopehelperfetch.go:FetchURL(ctx, url)with 10 s timeout, non-2xx erroradvisor.go:Advisorinterface (SupportedMIMETypes() []string) for provider-side file-picker integrationTests
decide_test.go: 11 table-driven cases covering all 6 strategy outcomes, MIME-miss, and every source variantfetch_test.go: 5 parallel httptest-based cases (success, 404, 500, cancelled context, binary content)What this PR does NOT do
convertDocumentimplementation