docs: define channel image ingestion strategy (#266)#327
Conversation
Codify the canonical 5-step ingestion pipeline (parse → emit → fetch → validate → stage) for multimodal image input across channels. - 9 requirements covering MVP channels, config gating, staging, cleanup - 9 Given/When/Then scenarios including rejection and edge cases - Channel-specific contracts for Telegram, WhatsApp, Discord, Slack - 5 ADRs: shared validation, magic-byte sniffing, RAII cleanup, fail-closed default, single dispatch point - Follow-up issue definitions for Discord, Slack, reaper, multi-image
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds Discord image-attachment ingestion: parses Discord attachments into image parts, downloads and streams-validated bytes with magic-byte MIME checks and size limits, computes SHA-256, writes staged temp files, and wires Discord into the centralized staging dispatch and multimodal allowlist. Changes
Sequence Diagram(s)sequenceDiagram
participant Gateway as Discord Gateway
participant Channel as DiscordChannel
participant CDN as Discord CDN
participant Media as media::Validator
participant FS as Temp File System
participant Dispatcher as stage_channel_images()
Gateway->>Channel: MESSAGE_CREATE with attachments
Channel->>Channel: parse_image_attachments() -> ContentPart::Image
Channel->>CDN: GET attachment URL (stream)
CDN-->>Channel: stream bytes (chunks)
Channel->>Media: validate mime & size (per-chunk + total)
Media-->>Channel: validation result
Channel->>FS: write temp file, compute SHA-256
FS-->>Channel: StagedImage(path, metadata)
Channel->>Dispatcher: return StagedImage
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
✅ Contributor ReportUser: @yacosta738
Contributor Report evaluates based on public GitHub activity. Analysis period: 2025-03-26 to 2026-03-26 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@openspec/changes/2026-03-26-channel-image-ingestion-strategy/design.md`:
- Line 12: The fenced code blocks in design.md lack language specifiers which
hinders syntax highlighting and tooling; update the three blocks referenced (the
architecture diagram block starting at the diagram that begins with
"┌────────────────", the file-naming example block showing
"{temp_dir}/corvus-{channel_abbrev}-img-{sha256_prefix}.{ext}", and the sequence
diagram block that starts "User Telegram API
TelegramChannel media.rs Provider") by changing their opening
backtick fences from ``` to ```text (or another appropriate language like
```bash/```text for diagrams) so each fenced block includes a language
specifier.
In
`@openspec/changes/2026-03-26-channel-image-ingestion-strategy/specs/channel-image-ingestion/spec.md`:
- Line 175: The phrase "PNG image" is redundant; update the text that currently
reads 'When a WhatsApp user sends a PNG image with no caption' to use 'PNG' (or
'PNG file') instead — e.g., change it to 'When a WhatsApp user sends a PNG with
no caption' — to remove the redundant wording while preserving meaning.
- Line 91: Update the wording to American English by changing the sentence that
mentions "The `multimodal.max_image_bytes` config field MAY override
`MAX_IMAGE_BYTES` in future." to read "The `multimodal.max_image_bytes` config
field MAY override `MAX_IMAGE_BYTES` in the future." — locate the sentence
referencing `multimodal.max_image_bytes` and `MAX_IMAGE_BYTES` and insert "the"
before "future".
In
`@openspec/changes/2026-03-26-channel-image-ingestion-strategy/verify-report.md`:
- Around line 61-74: The scenario audit table is missing Scenario 9; add a new
table row for "9: Image rejected — vision route misconfigured" with the same
Given/When/Then/And status format (mark as ✅ Correct if it follows the format),
update the summary count at the end from "All 8 scenarios" to "All 9 scenarios",
and remove or update the SUGGESTION that asks to add Scenario 9 (the existing
SUGGESTION referencing Scenario 9 should be deleted or marked resolved); locate
references by the scenario title "Image rejected — vision route misconfigured"
and the SUGGESTION block mentioning Scenario 9 to make the edits.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 078c7cc6-24d1-4e89-80e8-0879a36b820f
📒 Files selected for processing (6)
openspec/changes/2026-03-26-channel-image-ingestion-strategy/design.mdopenspec/changes/2026-03-26-channel-image-ingestion-strategy/exploration.mdopenspec/changes/2026-03-26-channel-image-ingestion-strategy/proposal.mdopenspec/changes/2026-03-26-channel-image-ingestion-strategy/specs/channel-image-ingestion/spec.mdopenspec/changes/2026-03-26-channel-image-ingestion-strategy/tasks.mdopenspec/changes/2026-03-26-channel-image-ingestion-strategy/verify-report.md
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: pr-checks
- GitHub Check: sonar
- GitHub Check: pr-checks
- GitHub Check: Cloudflare Pages
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{md,mdx}
⚙️ CodeRabbit configuration file
**/*.{md,mdx}: Verify technical accuracy and that docs stay aligned with code changes.
For user-facing docs, check EN/ES parity or explicitly note pending translation gaps.
Files:
openspec/changes/2026-03-26-channel-image-ingestion-strategy/tasks.mdopenspec/changes/2026-03-26-channel-image-ingestion-strategy/exploration.mdopenspec/changes/2026-03-26-channel-image-ingestion-strategy/verify-report.mdopenspec/changes/2026-03-26-channel-image-ingestion-strategy/proposal.mdopenspec/changes/2026-03-26-channel-image-ingestion-strategy/design.mdopenspec/changes/2026-03-26-channel-image-ingestion-strategy/specs/channel-image-ingestion/spec.md
**/*
⚙️ CodeRabbit configuration file
**/*: Security first, performance second.
Validate input boundaries, auth/authz implications, and secret management.
Look for behavioral regressions, missing tests, and contract breaks across modules.
Files:
openspec/changes/2026-03-26-channel-image-ingestion-strategy/tasks.mdopenspec/changes/2026-03-26-channel-image-ingestion-strategy/exploration.mdopenspec/changes/2026-03-26-channel-image-ingestion-strategy/verify-report.mdopenspec/changes/2026-03-26-channel-image-ingestion-strategy/proposal.mdopenspec/changes/2026-03-26-channel-image-ingestion-strategy/design.mdopenspec/changes/2026-03-26-channel-image-ingestion-strategy/specs/channel-image-ingestion/spec.md
🪛 LanguageTool
openspec/changes/2026-03-26-channel-image-ingestion-strategy/specs/channel-image-ingestion/spec.md
[locale-violation] ~91-~91: The phrase ‘in future’ is British English. Did you mean: “in the future”?
Context: ...ig field MAY override MAX_IMAGE_BYTES in future. ### REQ-5: Config Gating Image inges...
(IN_FUTURE)
[style] ~175-~175: This phrase is redundant (‘G’ stands for ‘graphic’). Use simply “PNG”.
Context: ...app"]` When a WhatsApp user sends a PNG image with no caption Then the channel em...
(ACRONYM_TAUTOLOGY)
🪛 markdownlint-cli2 (0.22.0)
openspec/changes/2026-03-26-channel-image-ingestion-strategy/design.md
[warning] 12-12: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 115-115: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 134-134: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (13)
openspec/changes/2026-03-26-channel-image-ingestion-strategy/specs/channel-image-ingestion/spec.md (6)
1-25: LGTM! Clear definitions and scope.The overview and definitions section provides a solid foundation for the specification. The 5-step pipeline definition (parse → emit → fetch → validate → stage) is clear and aligns with the design document.
94-109: LGTM! Config gating requirements align with implementation.The config validation requirements match the existing
validate_multimodal_config()implementation perfectly (as shown in context snippets). The fail-closed semantics and startup validation rules are correctly specified.
128-134: LGTM! RAII cleanup semantics correctly specified.The
StagedImageGuardRAII cleanup requirements match the existing implementation. The best-effort cleanup on all exit paths (success, error, timeout, early return) is correctly documented.
236-276: LGTM! Channel contracts are accurate and complete.The channel-specific contracts correctly document:
- Telegram and WhatsApp contracts match their existing implementations (verified against context snippets)
- Discord and Slack are properly marked as Wave 2 with contract-only status
- Auth mechanisms (bot token in URL for Telegram, Bearer token for WhatsApp, etc.) are correctly specified
- Metadata fields align with each channel's API capabilities
189-210:⚠️ Potential issue | 🟡 MinorEnsure all rejection scenarios explicitly assert ImageIngressEvent emission.
REQ-8 and REQ-9 require that all rejection cases emit an
ImageIngressEvent. However, Scenarios 4 (oversized), 5 (unsupported format), and 6 (multimodal disabled) do not explicitly include "And anImageIngressEventwith outcomeRejectedand reason[X]is emitted" assertions, unlike Scenarios 1, 3, 8, and 9.For completeness and to match the requirement, add explicit ImageIngressEvent assertions to these scenarios.
📝 Suggested additions
Add to Scenario 4 (after line 194):
**And** the user receives "That image is too large to process" +**And** an `ImageIngressEvent` with outcome `Rejected` and reason `Oversize` is emittedAdd to Scenario 5 (after line 202):
**And** the user receives "That image format is not supported" +**And** an `ImageIngressEvent` with outcome `Rejected` and reason `MimeRejected` is emittedAdd to Scenario 6 (after line 210):
**And** the user receives "Image input is currently disabled" +**And** an `ImageIngressEvent` with outcome `Rejected` and reason `Disabled` is emitted> Likely an incorrect or invalid review comment.
115-121: No action needed. The spec documentation is correct: providers do base64-encode images for InlineBytes transport. BothOpenAiCompatibleProvider(compatible.rs:531) andGeminiProvider(gemini.rs:390) read fromtemp_pathand explicitly callbase64_encode()before passing to providers. The enum documentation "Raw bytes inlined in the provider request" refers to inline format (vs. URL references), not encoding state.openspec/changes/2026-03-26-channel-image-ingestion-strategy/exploration.md (1)
1-124: LGTM! Thorough exploration that grounds the specification.This exploration document provides excellent context by:
- Documenting the current implementation state for Telegram and WhatsApp
- Identifying which channels are in scope vs. deferred
- Analyzing staging/retention behavior and risks
- Closing key questions that inform the spec requirements
The identified risks (orphaned temp files, multi-image support, GIF rejection) align with the design's future considerations and verify-report findings.
openspec/changes/2026-03-26-channel-image-ingestion-strategy/tasks.md (1)
1-57: LGTM! Clear task breakdown supporting implementation.The task structure is well-organized:
- Phase 1 (spec-only) correctly marked complete
- Phase 2 follow-up issues are discrete and actionable
- Each channel implementation (Discord, Slack) includes parsing, staging, config updates, and tests
- Acceptance criteria mapping provides traceability back to requirements
This will make it straightforward to create follow-up implementation issues.
openspec/changes/2026-03-26-channel-image-ingestion-strategy/proposal.md (1)
1-73: LGTM! Concise proposal with clear scope boundaries.The proposal effectively:
- Defines intent to codify existing Telegram/WhatsApp patterns
- Sets clear in-scope vs. out-of-scope boundaries (including explicit exclusion of implementation, GIF, multi-image, video/audio)
- Identifies affected modules accurately
- Notes this is spec-only with simple rollback
- Lists reasonable risks with appropriate mitigations
openspec/changes/2026-03-26-channel-image-ingestion-strategy/verify-report.md (1)
136-153: LGTM! Valuable verification with actionable warnings.The verification report correctly identifies:
- Missing ImageIngressEvent assertions in scenarios 4 and 5 (WARNING - valid, I flagged this in spec review)
- Deduplication scope ambiguity (SUGGESTION - reasonable)
- Placeholder issue numbers in tasks (SUGGESTION - acceptable for now)
The PASS WITH WARNINGS verdict is appropriate for a spec-only change.
openspec/changes/2026-03-26-channel-image-ingestion-strategy/design.md (3)
7-62: LGTM! Clear architecture with excellent separation of concerns.The pipeline architecture diagram effectively shows:
- Per-channel parsing layer producing
ContentPart::Image- Centralized config gating
- Single dispatch point to per-channel staging implementations
- Fail-closed default for unimplemented channels
- Provider handoff with RAII cleanup
This design supports the spec requirements cleanly.
64-112: LGTM! Well-justified ADRs addressing key design decisions.All 5 ADRs have clear rationale:
- ADR-1: Channel-specific fetch with shared validation (security-critical)
- ADR-2: Magic-byte sniffing over declared MIME (prevents MIME confusion attacks)
- ADR-3: RAII cleanup (reliable on all exit paths)
- ADR-4: Fail-closed default (security-first)
- ADR-5: Single dispatch point (auditability and consistency)
These align perfectly with the spec requirements and security-first approach.
162-172: LGTM! Future considerations align with exploration risks.The future considerations section appropriately flags:
- Multi-image support (matches tasks Phase 2)
- GIF support (noted as rejected in spec)
- Startup reaper (matches exploration risks)
- URL-based transport optimization for Discord
- Deduplication potential via SHA-256 naming
These provide good guidance for follow-up work.
| ### Scenario 2: WhatsApp image without caption | ||
|
|
||
| **Given** multimodal is enabled with `allowed_channels: ["whatsapp"]` | ||
| **When** a WhatsApp user sends a PNG image with no caption |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Minor: Remove redundant "PNG image" phrasing.
"PNG" already stands for "Portable Network Graphics" (where "graphic" means image), so "PNG image" is technically redundant. However, this is extremely minor and can be ignored if the team prefers the more explicit phrasing for clarity.
🧰 Tools
🪛 LanguageTool
[style] ~175-~175: This phrase is redundant (‘G’ stands for ‘graphic’). Use simply “PNG”.
Context: ...app"]` When a WhatsApp user sends a PNG image with no caption Then the channel em...
(ACRONYM_TAUTOLOGY)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@openspec/changes/2026-03-26-channel-image-ingestion-strategy/specs/channel-image-ingestion/spec.md`
at line 175, The phrase "PNG image" is redundant; update the text that currently
reads 'When a WhatsApp user sends a PNG image with no caption' to use 'PNG' (or
'PNG file') instead — e.g., change it to 'When a WhatsApp user sends a PNG with
no caption' — to remove the redundant wording while preserving meaning.
| ### Scenario Format Audit | ||
|
|
||
| | Scenario | Given/When/Then | Status | | ||
| |----------|-----------------|--------| | ||
| | 1: Telegram photo accepted | Given/And/When/Then/And(x5) | ✅ Correct | | ||
| | 2: WhatsApp image without caption | Given/When/Then/And(x3) | ✅ Correct | | ||
| | 3: Image rejected — channel not allowed | Given/When/Then/And(x2) | ✅ Correct | | ||
| | 4: Image rejected — oversized | Given/When/Then/And | ✅ Correct | | ||
| | 5: Image rejected — unsupported format | Given/When/Then/And | ✅ Correct | | ||
| | 6: Image rejected — multimodal disabled | Given/When/Then/And | ✅ Correct | | ||
| | 7: Fail-closed for unimplemented channel | Given/When/Then/And | ✅ Correct | | ||
| | 8: Temp file cleanup on timeout | Given/When/Then/And | ✅ Correct | | ||
|
|
||
| All 8 scenarios follow Given/When/Then format per openspec config rules. |
There was a problem hiding this comment.
Update scenario count to match spec.
The scenario format audit lists 8 scenarios, but the spec (lines 159-235) actually contains 9 scenarios. Scenario 9 ("Image rejected — vision route misconfigured") is present in the spec but missing from this table.
Additionally, the SUGGESTION on line 146-148 asks for a Scenario 9 covering vision route rejection, but this scenario already exists in the spec as Scenario 9 (lines 227-234).
📝 Proposed fix
Add Scenario 9 to the table after line 72:
| 7: Fail-closed for unimplemented channel | Given/When/Then/And | ✅ Correct |
| 8: Temp file cleanup on timeout | Given/When/Then/And | ✅ Correct |
+| 9: Image rejected — vision route misconfigured | Given/When/Then/And(x2) | ✅ Correct |And update the summary on line 74:
-All 8 scenarios follow Given/When/Then format per openspec config rules.
+All 9 scenarios follow Given/When/Then format per openspec config rules.Remove or update the SUGGESTION on lines 146-148 since Scenario 9 already exists.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@openspec/changes/2026-03-26-channel-image-ingestion-strategy/verify-report.md`
around lines 61 - 74, The scenario audit table is missing Scenario 9; add a new
table row for "9: Image rejected — vision route misconfigured" with the same
Given/When/Then/And status format (mark as ✅ Correct if it follows the format),
update the summary count at the end from "All 8 scenarios" to "All 9 scenarios",
and remove or update the SUGGESTION that asks to add Scenario 9 (the existing
SUGGESTION referencing Scenario 9 should be deleted or marked resolved); locate
references by the scenario title "Image rejected — vision route misconfigured"
and the SUGGESTION block mentioning Scenario 9 to make the edits.
Implement Discord channel image ingestion following the canonical 5-step pipeline defined in the channel image ingestion strategy. Code changes: - Parse Discord message attachments into ContentPart::Image - Implement DiscordChannel::fetch_and_stage_image() (CDN download) - Add "discord" dispatch arm in stage_channel_images() - Add "discord" to MVP_VALID_MULTIMODAL_CHANNELS in config validation - Deduplicate inline parsing by calling parse_image_attachments() - 11 new tests for image attachment parsing Spec refinements: - Add observability assertions to Scenarios 4, 5, 8 - Add Scenario 9 (vision route misconfiguration) - Add REQ-10 (dedup explicitly out of scope for MVP) - Update verify report to PASS - Add follow-up issue definitions for Slack, reaper, multi-image
- Add observability section (REQ-9) to design doc - Update tasks with Linear issue refs (DALLAY-192 through DALLAY-195) - Sync delta spec to openspec/specs/channel-image-ingestion/ - Archive change to openspec/changes/archive/ - Full SDD cycle: explore → propose → spec → design → tasks → verify → archive
Deploying corvus with
|
| Latest commit: |
b6e9edf
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://3fa55fe9.corvus-42x.pages.dev |
| Branch Preview URL: | https://feat-channel-image-ingestion.corvus-42x.pages.dev |
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
clients/agent-runtime/src/config/schema.rs (1)
6033-6049: 🧹 Nitpick | 🔵 TrivialAdd a runtime-validation success case for
"discord".The new allowlist value is only exercised by parsing.
validate_multimodal_config()is still never asserted to accept"discord", so a regression inMVP_VALID_MULTIMODAL_CHANNELSwould slip past these tests.➕ Suggested test
+ #[test] + fn multimodal_validation_accepts_discord_when_enabled() { + let config = Config { + multimodal: MultimodalConfig { + enabled: true, + allowed_channels: vec!["discord".into()], + vision_model_hint: Some("vision".into()), + max_image_bytes: None, + }, + model_routes: vec![make_vision_route()], + ..Config::default() + }; + + assert!(config.validate_multimodal_config().is_ok()); + }As per coding guidelines,
**/*: look for behavioral regressions, missing tests, and contract breaks across modules.Also applies to: 6164-6178
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/agent-runtime/src/config/schema.rs` around lines 6033 - 6049, Add a runtime-validation assertion that the parsed multimodal config accepts "discord": after parsing the TOML into parsed (the multimodal struct) call and assert validate_multimodal_config(&parsed.multimodal).unwrap_or(…) succeeds (or returns Ok) to exercise the validate_multimodal_config function and its MVP_VALID_MULTIMODAL_CHANNELS allowlist; ensure MVP_VALID_MULTIMODAL_CHANNELS contains "discord" if validation checks that constant. This will pair the parsing assertions (parsed.multimodal.allowed_channels contains "discord") with a runtime validation check to catch regressions in validate_multimodal_config/MVP_VALID_MULTIMODAL_CHANNELS.
♻️ Duplicate comments (2)
openspec/changes/archive/2026-03-26-channel-image-ingestion-strategy/specs/channel-image-ingestion/spec.md (1)
91-91:⚠️ Potential issue | 🟡 MinorUse American English phrasing for consistency.
Line 91 should read “in the future” instead of “in future”.
As per coding guidelines
**/*.{md,mdx}: "Verify technical accuracy and that docs stay aligned with code changes."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openspec/changes/archive/2026-03-26-channel-image-ingestion-strategy/specs/channel-image-ingestion/spec.md` at line 91, Update the phrasing for consistency with American English by changing the sentence that references the multimodal.max_image_bytes config field (the line mentioning "The `multimodal.max_image_bytes` config field MAY override `MAX_IMAGE_BYTES` in future.") to read "in the future" instead of "in future", ensuring the clause around `multimodal.max_image_bytes` and `MAX_IMAGE_BYTES` remains otherwise unchanged.openspec/changes/archive/2026-03-26-channel-image-ingestion-strategy/design.md (1)
12-62:⚠️ Potential issue | 🟡 MinorAdd language identifiers to fenced code blocks.
The diagram/example fences (Line 12, Line 149, Line 168) still omit language tags (
```textrecommended), which keeps triggering markdownlint MD040.As per coding guidelines
**/*: "Look for behavioral regressions, missing tests, and contract breaks across modules."Also applies to: 149-157, 168-194
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openspec/changes/archive/2026-03-26-channel-image-ingestion-strategy/design.md` around lines 12 - 62, The fenced diagrams in design.md are missing language identifiers and trigger markdownlint MD040; update each triple-backtick fence (the large channel diagram and the two smaller fences referenced around the diagram) to use a language tag such as ```text (i.e., replace ``` with ```text) so markdownlint recognizes them as code blocks; this affects the diagram block containing Channel Layer/Config Gating/Fetch & Stage/Provider Dispatch and the two other fences noted in the comment.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@clients/agent-runtime/src/channels/discord.rs`:
- Around line 473-479: The mention_only check is being bypassed for
attachment-only messages because the guard uses && !content.is_empty(); remove
the content non-empty condition so the block always enforces mention-only when
self.mention_only is true (i.e., change the condition to if self.mention_only &&
!contains_bot_mention(content, &bot_user_id) { continue; }), and add a
regression test exercising an image/attachment-only Discord message (empty
content, attachment present) to assert that an unmentioned upload is ignored
when mention_only=true; reference the mention_only flag, contains_bot_mention
function, and bot_user_id variable to locate the change and the handler code in
discord.rs.
- Around line 100-114: The staging path currently uses only the hash prefix
(temp_path derived from sha256[..16]) so concurrent identical images collide;
generate a per-stage unique nonce (e.g., let nonce =
uuid::Uuid::new_v4().to_string() or similar) and include it in the filename
(e.g., format!("corvus-dc-img-{}-{}.{}", &sha256[..16], nonce, ext)) before
calling tokio::fs::write, and use that same temp_path variable for any
subsequent cleanup to ensure each turn has its own file.
In
`@openspec/changes/archive/2026-03-26-channel-image-ingestion-strategy/design.md`:
- Around line 37-42: Update the architecture doc to reflect that Discord image
staging is implemented: change the line in the stage_channel_images() dispatch
list that currently reads `"discord" → DiscordChannel::fetch_and_stage_image()
[Wave2]` to indicate current/implemented status (e.g., remove `[Wave2]` or
replace with `[implemented]` / `current-state`), ensuring the reference to
DiscordChannel::fetch_and_stage_image() and stage_channel_images() remains
unchanged so the doc aligns with the PR's implementation.
In
`@openspec/changes/archive/2026-03-26-channel-image-ingestion-strategy/specs/channel-image-ingestion/spec.md`:
- Around line 32-37: Update the spec to mark Discord as implemented for MVP:
change the Discord row in the channel status table (the "Channel | Status |
Priority" table showing Discord as "Planned | Wave 2") to "Implemented | MVP",
and also update the later section that describes Discord as
"planned/contract-only" (the paragraph around the Discord implementation
details) to reflect that Discord ingestion is implemented and behaves as MVP;
ensure wording and any examples or notes align with implemented behavior.
- Around line 115-121: The fenced code block showing the runtime handoff
(starting with "ChannelMessage.parts: [ContentPart::Image ...]" and including
stage_channel_images(), Vec<StagedImage> and Provider::chat()) is missing a
fence language and triggers markdownlint MD040; update that fence to include an
explicit language token (e.g., add "text" after the opening ```), re-save the
spec.md so the block becomes ```text ... ```, and ensure related references
(ChannelMessage.parts, ContentPart::Image, stage_channel_images(),
Vec<StagedImage>, Provider::chat(), and StagedImage fields like sha256,
mime_type, temp_path, transport_form, channel_origin) remain unchanged.
- Around line 218-221: The scenario uses allowed_channels: ["matrix"] which
violates REQ-5 (allowed_channels must be valid MVP channel names) making the
test unreachable; update the scenario to use a valid-but-unimplemented MVP
channel name (e.g., replace "matrix" with an actual MVP channel identifier used
elsewhere in the spec) or alternatively revise REQ-5 to allow "matrix" so the
scenario becomes valid, and keep the expected behavior referencing
stage_channel_images() and the rejection message unchanged.
In
`@openspec/changes/archive/2026-03-26-channel-image-ingestion-strategy/tasks.md`:
- Around line 29-56: The task checklist items under the "Issue: Slack image
ingestion", "Issue: Startup temp file reaper", and "Issue: Multi-image per turn"
sections are incorrectly marked as completed; update the checklist entries
(e.g., items 2.6–2.11, 2.12–2.15, 2.16–2.19) to use unchecked boxes "[ ]"
instead of "[x]" or alternatively rename the section header to clarify that
"[x]" denotes "issue created" (not "implementation complete"); ensure the
changes are applied to the corresponding markdown headings ("Issue: Slack image
ingestion", "Issue: Startup temp file reaper", "Issue: Multi-image per turn") so
the documentation accurately reflects that these are tracked follow-up tasks
rather than shipped features.
In
`@openspec/changes/archive/2026-03-26-channel-image-ingestion-strategy/verify-report.md`:
- Around line 5-6: Update the verify-report.md artifact to reflect actual PR
changes: remove the "spec-only / no code modified" claim and either (a) scope
the report explicitly to archived spec artifacts only, or (b) document the
runtime validation that was run for the Rust changes in clients/agent-runtime
(specifically mention channels::discord.rs and config::schema.rs changes);
include the updated task IDs DALLAY-192 through DALLAY-195 instead of `#next`
placeholders; and for any modified clients/agent-runtime/**/*.rs files, add or
update explicit lines stating the status of cargo fmt, cargo clippy, and cargo
test (or explain why any were skipped). Ensure markdown files **/*.{md,mdx}
mentioned in the PR are updated to remain aligned with these code changes.
In `@openspec/specs/channel-image-ingestion/spec.md`:
- Around line 107-110: REQ-5 and Scenario 7 conflict: when `enabled=true` REQ-5
requires all `allowed_channels` to be valid MVP channel names but Scenario 7
uses `["matrix"]`; either make the scenario use an MVP channel or loosen REQ-5.
Fix by updating Scenario 7’s `allowed_channels` from `["matrix"]` to a valid MVP
channel (e.g., `"slack"`), or alternatively update the REQ-5 text to permit
non-MVP channels (adjust the validation sentence that mentions
`allowed_channels` and `valid MVP channel names`) and update any startup
validation logic that enforces REQ-5 accordingly; ensure `vision_model_hint` and
`enabled=true` rules remain satisfied.
- Around line 32-37: Update the spec to reflect that Discord is implemented at
runtime: change the Channel status table row for "Discord" from "Planned" to
"Implemented" (and Priority from "Wave 2" to "MVP" if applicable), and update
the narrative in the sections around the old contract-only description (the
paragraph covering the Discord channel and the block referencing the "discord"
dispatch) to indicate that stage_channel_images now routes a live "discord"
path; reference the runtime symbol stage_channel_images and the existence of a
"discord" dispatch when editing the sections previously claiming Discord was
planned/contract-only.
- Around line 115-121: The fenced handoff block (the code fence showing
ChannelMessage.parts → stage_channel_images → Vec<StagedImage> → Provider
behavior) is untyped and triggers markdownlint MD040; fix it by adding a
language tag (e.g., change the opening ``` to ```text) so the fence becomes a
typed block (```text) and the rest of the content remains unchanged.
---
Outside diff comments:
In `@clients/agent-runtime/src/config/schema.rs`:
- Around line 6033-6049: Add a runtime-validation assertion that the parsed
multimodal config accepts "discord": after parsing the TOML into parsed (the
multimodal struct) call and assert
validate_multimodal_config(&parsed.multimodal).unwrap_or(…) succeeds (or returns
Ok) to exercise the validate_multimodal_config function and its
MVP_VALID_MULTIMODAL_CHANNELS allowlist; ensure MVP_VALID_MULTIMODAL_CHANNELS
contains "discord" if validation checks that constant. This will pair the
parsing assertions (parsed.multimodal.allowed_channels contains "discord") with
a runtime validation check to catch regressions in
validate_multimodal_config/MVP_VALID_MULTIMODAL_CHANNELS.
---
Duplicate comments:
In
`@openspec/changes/archive/2026-03-26-channel-image-ingestion-strategy/design.md`:
- Around line 12-62: The fenced diagrams in design.md are missing language
identifiers and trigger markdownlint MD040; update each triple-backtick fence
(the large channel diagram and the two smaller fences referenced around the
diagram) to use a language tag such as ```text (i.e., replace ``` with ```text)
so markdownlint recognizes them as code blocks; this affects the diagram block
containing Channel Layer/Config Gating/Fetch & Stage/Provider Dispatch and the
two other fences noted in the comment.
In
`@openspec/changes/archive/2026-03-26-channel-image-ingestion-strategy/specs/channel-image-ingestion/spec.md`:
- Line 91: Update the phrasing for consistency with American English by changing
the sentence that references the multimodal.max_image_bytes config field (the
line mentioning "The `multimodal.max_image_bytes` config field MAY override
`MAX_IMAGE_BYTES` in future.") to read "in the future" instead of "in future",
ensuring the clause around `multimodal.max_image_bytes` and `MAX_IMAGE_BYTES`
remains otherwise unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 54392901-b695-4216-bc8a-d510d7cb179d
📒 Files selected for processing (11)
clients/agent-runtime/src/channels/discord.rsclients/agent-runtime/src/channels/mod.rsclients/agent-runtime/src/config/schema.rsopenspec/changes/archive/2026-03-26-channel-image-ingestion-strategy/design.mdopenspec/changes/archive/2026-03-26-channel-image-ingestion-strategy/exploration.mdopenspec/changes/archive/2026-03-26-channel-image-ingestion-strategy/follow-up-issues.mdopenspec/changes/archive/2026-03-26-channel-image-ingestion-strategy/proposal.mdopenspec/changes/archive/2026-03-26-channel-image-ingestion-strategy/specs/channel-image-ingestion/spec.mdopenspec/changes/archive/2026-03-26-channel-image-ingestion-strategy/tasks.mdopenspec/changes/archive/2026-03-26-channel-image-ingestion-strategy/verify-report.mdopenspec/specs/channel-image-ingestion/spec.md
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: pr-checks
- GitHub Check: pr-checks
- GitHub Check: sonar
- GitHub Check: Cloudflare Pages
🧰 Additional context used
📓 Path-based instructions (7)
clients/agent-runtime/src/channels/**/*.rs
📄 CodeRabbit inference engine (clients/agent-runtime/AGENTS.md)
Implement
Channeltrait insrc/channels/with consistentsend,listen, andhealth_checksemantics and cover auth/allowlist/health behavior with tests
Files:
clients/agent-runtime/src/channels/mod.rsclients/agent-runtime/src/channels/discord.rs
clients/agent-runtime/src/**/*.rs
📄 CodeRabbit inference engine (clients/agent-runtime/AGENTS.md)
clients/agent-runtime/src/**/*.rs: Never log secrets, tokens, raw credentials, or sensitive payloads in any logging statements
Avoid unnecessary allocations, clones, and blocking operations to maintain performance and efficiency
Files:
clients/agent-runtime/src/channels/mod.rsclients/agent-runtime/src/config/schema.rsclients/agent-runtime/src/channels/discord.rs
clients/agent-runtime/**/*.rs
📄 CodeRabbit inference engine (clients/agent-runtime/AGENTS.md)
Run
cargo fmt --all -- --check,cargo clippy --all-targets -- -D warnings, andcargo testfor code validation, or document which checks were skipped and why
Files:
clients/agent-runtime/src/channels/mod.rsclients/agent-runtime/src/config/schema.rsclients/agent-runtime/src/channels/discord.rs
**/*.rs
⚙️ CodeRabbit configuration file
**/*.rs: Focus on Rust idioms, memory safety, and ownership/borrowing correctness.
Flag unnecessary clones, unchecked panics in production paths, and weak error context.
Prioritize unsafe blocks, FFI boundaries, concurrency races, and secret handling.
Files:
clients/agent-runtime/src/channels/mod.rsclients/agent-runtime/src/config/schema.rsclients/agent-runtime/src/channels/discord.rs
**/*
⚙️ CodeRabbit configuration file
**/*: Security first, performance second.
Validate input boundaries, auth/authz implications, and secret management.
Look for behavioral regressions, missing tests, and contract breaks across modules.
Files:
clients/agent-runtime/src/channels/mod.rsopenspec/changes/archive/2026-03-26-channel-image-ingestion-strategy/tasks.mdopenspec/specs/channel-image-ingestion/spec.mdopenspec/changes/archive/2026-03-26-channel-image-ingestion-strategy/proposal.mdopenspec/changes/archive/2026-03-26-channel-image-ingestion-strategy/verify-report.mdclients/agent-runtime/src/config/schema.rsopenspec/changes/archive/2026-03-26-channel-image-ingestion-strategy/exploration.mdopenspec/changes/archive/2026-03-26-channel-image-ingestion-strategy/follow-up-issues.mdclients/agent-runtime/src/channels/discord.rsopenspec/changes/archive/2026-03-26-channel-image-ingestion-strategy/design.mdopenspec/changes/archive/2026-03-26-channel-image-ingestion-strategy/specs/channel-image-ingestion/spec.md
**/*.{md,mdx}
⚙️ CodeRabbit configuration file
**/*.{md,mdx}: Verify technical accuracy and that docs stay aligned with code changes.
For user-facing docs, check EN/ES parity or explicitly note pending translation gaps.
Files:
openspec/changes/archive/2026-03-26-channel-image-ingestion-strategy/tasks.mdopenspec/specs/channel-image-ingestion/spec.mdopenspec/changes/archive/2026-03-26-channel-image-ingestion-strategy/proposal.mdopenspec/changes/archive/2026-03-26-channel-image-ingestion-strategy/verify-report.mdopenspec/changes/archive/2026-03-26-channel-image-ingestion-strategy/exploration.mdopenspec/changes/archive/2026-03-26-channel-image-ingestion-strategy/follow-up-issues.mdopenspec/changes/archive/2026-03-26-channel-image-ingestion-strategy/design.mdopenspec/changes/archive/2026-03-26-channel-image-ingestion-strategy/specs/channel-image-ingestion/spec.md
clients/agent-runtime/src/{security,gateway,tools,config}/**/*.rs
📄 CodeRabbit inference engine (clients/agent-runtime/AGENTS.md)
Do not silently weaken security policy or access constraints; keep default behavior secure-by-default with deny-by-default where applicable
Files:
clients/agent-runtime/src/config/schema.rs
🧠 Learnings (1)
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/channels/**/*.rs : Implement `Channel` trait in `src/channels/` with consistent `send`, `listen`, and `health_check` semantics and cover auth/allowlist/health behavior with tests
Applied to files:
clients/agent-runtime/src/channels/mod.rsclients/agent-runtime/src/config/schema.rsclients/agent-runtime/src/channels/discord.rs
🪛 LanguageTool
openspec/specs/channel-image-ingestion/spec.md
[locale-violation] ~91-~91: The phrase ‘in future’ is British English. Did you mean: “in the future”?
Context: ...ig field MAY override MAX_IMAGE_BYTES in future. ### REQ-5: Config Gating Image inges...
(IN_FUTURE)
[style] ~179-~179: This phrase is redundant (‘G’ stands for ‘graphic’). Use simply “PNG”.
Context: ...app"]` When a WhatsApp user sends a PNG image with no caption Then the channel em...
(ACRONYM_TAUTOLOGY)
openspec/changes/archive/2026-03-26-channel-image-ingestion-strategy/specs/channel-image-ingestion/spec.md
[locale-violation] ~91-~91: The phrase ‘in future’ is British English. Did you mean: “in the future”?
Context: ...ig field MAY override MAX_IMAGE_BYTES in future. ### REQ-5: Config Gating Image inges...
(IN_FUTURE)
[style] ~179-~179: This phrase is redundant (‘G’ stands for ‘graphic’). Use simply “PNG”.
Context: ...app"]` When a WhatsApp user sends a PNG image with no caption Then the channel em...
(ACRONYM_TAUTOLOGY)
🪛 markdownlint-cli2 (0.22.0)
openspec/specs/channel-image-ingestion/spec.md
[warning] 115-115: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
openspec/changes/archive/2026-03-26-channel-image-ingestion-strategy/design.md
[warning] 12-12: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 149-149: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 168-168: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
openspec/changes/archive/2026-03-26-channel-image-ingestion-strategy/specs/channel-image-ingestion/spec.md
[warning] 115-115: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (1)
clients/agent-runtime/src/channels/mod.rs (1)
1043-1048: Discord staging dispatch is consistent with existing channel routing.This new match arm follows the same fetch-and-stage contract as Telegram/WhatsApp and keeps the fail-closed default path intact.
- Fix mention_only bypass for attachment-only Discord messages - Add UUID nonce to staging path to prevent concurrent collision - Add regression tests for mention_only + image-only messages - Add discord multimodal validation test in schema.rs - Update Discord status from Planned/Wave2 to Implemented/MVP in specs - Fix Scenario 7: use valid MVP channel (slack) instead of matrix - Add language tags to fenced code blocks (MD040) - Uncheck Slack/reaper/multi-image tasks (not yet implemented) - Update verify-report with actual build/test status and Linear refs - Fix 'in future' → 'in the future' wording
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (5)
openspec/specs/channel-image-ingestion/spec.md (2)
109-110:⚠️ Potential issue | 🟠 MajorScenario 7 conflicts with REQ-5 channel validation.
Line 109 requires MVP channel names only, but Line 218 uses Slack (non-MVP in Line 37). This scenario is not reachable under the current requirement set.
As per coding guidelines
**/*.{md,mdx}: "Verify technical accuracy and that docs stay aligned with code changes."Also applies to: 218-221
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openspec/specs/channel-image-ingestion/spec.md` around lines 109 - 110, Scenario 7 conflicts with REQ-5 because allowed_channels is restricted to MVP channel names but the scenario uses Slack (a non-MVP channel); update the scenario or the requirement so they align: either change Scenario 7 (and the entries at lines referenced 218-221) to use a valid MVP channel name instead of "Slack", or modify REQ-5/allowed_channels to explicitly allow Slack and document the exception; ensure references to "allowed_channels", "REQ-5", and "Scenario 7" are updated consistently.
10-13:⚠️ Potential issue | 🟠 MajorDiscord status remains inconsistent across this spec.
Line 10-Line 13 and Line 261-Line 270 still frame Discord as future/contract-only while Line 36 lists it as Implemented MVP. Please unify these sections to the same rollout state.
As per coding guidelines
**/*.{md,mdx}: "Verify technical accuracy and that docs stay aligned with code changes."Also applies to: 261-270
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openspec/specs/channel-image-ingestion/spec.md` around lines 10 - 13, The spec contains conflicting rollout status for Discord: update all mentions so they match (choose either "Implemented MVP" or "Contract-only") to keep documentation consistent; specifically change the Discord status in the opening summary block (currently lines 10–13), the "Implemented MVP" list (around line 36) and the later status block (around lines 261–270) so they all state the same rollout state and wording, and ensure any related changelog/notes or bullets referencing Discord use the identical term and tense.openspec/changes/archive/2026-03-26-channel-image-ingestion-strategy/specs/channel-image-ingestion/spec.md (2)
109-110:⚠️ Potential issue | 🟠 MajorScenario 7 is unreachable under REQ-5 as written.
Line 109 requires
allowed_channelsto be valid MVP channel names, but Line 218 uses["slack"]while Slack is not MVP in Line 37. Either relax REQ-5 + runtime validation, or rewrite/remove Scenario 7 to match current enforceable behavior.As per coding guidelines
**/*.{md,mdx}: "Verify technical accuracy and that docs stay aligned with code changes."Also applies to: 218-221
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openspec/changes/archive/2026-03-26-channel-image-ingestion-strategy/specs/channel-image-ingestion/spec.md` around lines 109 - 110, REQ-5 currently mandates that allowed_channels entries be valid MVP channel names, but Scenario 7 uses ["slack"] which is not an MVP channel; update the spec so it is internally consistent by either (a) relaxing REQ-5 to allow non-MVP channel names with runtime validation and document the new behavior in REQ-5 and the runtime validation rules, or (b) change Scenario 7’s allowed_channels to use an actual MVP channel name (e.g., replace ["slack"] with a valid MVP channel like ["email"] or another MVP entry) or remove Scenario 7 entirely; locate references to allowed_channels, REQ-5 and "Scenario 7" in the spec and update the text so the requirement and the scenario are aligned.
10-13:⚠️ Potential issue | 🟠 MajorDiscord status is still contradictory across the spec.
Line 10-Line 13 and Line 261-Line 270 still describe Discord as future/contract-only, but Line 36 marks Discord as Implemented for MVP. Please align these sections to one status.
Suggested doc patch
-This specification defines the canonical strategy for how Corvus messaging channels ingest user-sent -images, validate them, stage them to disk, and hand them off to the runtime's provider pipeline. It -codifies the patterns already implemented for Telegram and WhatsApp, and defines contracts for -future channel implementations. +This specification defines the canonical strategy for how Corvus messaging channels ingest user-sent +images, validate them, stage them to disk, and hand them off to the runtime's provider pipeline. It +codifies the patterns already implemented for Telegram, WhatsApp, and Discord, and defines contracts +for future channel implementations. @@ -### Discord (Wave 2 — contract only) +### Discord (Implemented — MVP)As per coding guidelines
**/*.{md,mdx}: "Verify technical accuracy and that docs stay aligned with code changes."Also applies to: 261-270
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openspec/changes/archive/2026-03-26-channel-image-ingestion-strategy/specs/channel-image-ingestion/spec.md` around lines 10 - 13, The spec contains contradictory Discord status entries: update all occurrences of "Discord" in this document so they use a single status (pick either "Implemented for MVP" or "future/contract-only") and match the chosen wording and tense. Specifically, change the Discord mention in the opening summary paragraph (the block around the first few lines), the status entry at the section near line 36 where it currently says "Implemented for MVP", and the later Discord subsection (around lines 261–270) so they all use identical status text and any follow-up wording (e.g., "implemented", "will implement", "contract") is consistent.openspec/changes/archive/2026-03-26-channel-image-ingestion-strategy/verify-report.md (1)
36-38:⚠️ Potential issue | 🟠 MajorVerification report still contains stale “spec-only” and placeholder statements.
Line 36-Line 38 conflicts with Line 6 and Line 26-Line 31, and Line 99-Line 100 / Line 149-Line 151 are outdated against the current artifacts. Please refresh these sections so the report has one consistent truth.
As per coding guidelines
**/*.{md,mdx}: "Verify technical accuracy and that docs stay aligned with code changes."Also applies to: 99-100, 149-151
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openspec/changes/archive/2026-03-26-channel-image-ingestion-strategy/verify-report.md` around lines 36 - 38, The verification report contains stale "spec-only" and placeholder statements (e.g., the paragraph starting "Not applicable — this is a spec-only change. No implementation to validate against scenarios.") that conflict with other sections (lines referenced in the review: 6, 26-31, 99-100, 149-151); update the verify-report.md so all assertions are consistent: remove or replace the "spec-only" placeholder, reconcile the Behavioral validation statement with the content at lines 6 and 26-31, refresh the outdated sections around lines 99-100 and 149-151 to reflect current artifacts, and ensure the file follows the docs guidance for **/*.{md,mdx} (technical accuracy and alignment with code changes).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@clients/agent-runtime/src/channels/discord.rs`:
- Around line 79-86: The stream error closure currently discards the diagnostic
by using |_|; change the map_err closure in the loop that processes
stream.next().await to capture the original error (e.g., |err|), call the future
sanitize_error(err) helper (or sanitize_error(&err) when implemented) to produce
a sanitized string/message, log that sanitized message with tracing::warn!
alongside the existing context, and then map to
media::ImageRejectionReason::FetchFailed so behavior is unchanged; update the
closure surrounding chunk_result.map_err(...) in the loop that handles
chunk_result to perform these steps.
- Around line 61-64: The current map_err closure in the Discord image download
(the get/send call inside the method that assigns dl_resp) logs the raw Reqwest
error which can include Discord CDN query tokens; update that error handling to
sanitize the error before logging (reuse or mirror the Telegram implementation's
sanitize_error() helper), e.g. call sanitize_error(&e) and log the sanitized
message instead of the raw error, then still return
media::ImageRejectionReason::FetchFailed so the rest of the flow is unchanged.
In
`@openspec/changes/archive/2026-03-26-channel-image-ingestion-strategy/design.md`:
- Around line 115-117: The text incorrectly states that all ImageIngressEvent
emissions occur at stage_channel_images(); update the wording to say that
ImageIngressEvent emissions occur both at pre-dispatch gating points (e.g.,
channel-not-allowed/disabled rejections emitted from channels::mod before
staging) and at the single dispatch point (stage_channel_images()) for
post-staging outcomes (validation success, provider errors, etc.), and reference
the Observer trait as receiving events from both pre-dispatch gates and the
staging/provider path so the doc accurately reflects where each rejection or
success is emitted.
In
`@openspec/changes/archive/2026-03-26-channel-image-ingestion-strategy/tasks.md`:
- Around line 15-19: Update the section header "Phase 2: Follow-Up
Implementation Issues (to be created)" to reflect that these issues are already
created/tracked (e.g., "Phase 2: Follow-Up Implementation Issues
(created/tracked)" or remove the parenthetical), and ensure the text under the
"Issue: Discord image ingestion ([DALLAY-192])" entry accurately indicates the
issue exists in Linear; verify the change follows the project's docs guideline
for markdown files so the doc and linked Linear items remain technically
accurate and aligned with the codebase.
- Around line 62-63: Update the acceptance mapping for REQ-1 so it reflects that
Discord is implemented in the MVP: edit the table cell labeled "Initial channel
list is defined" (the REQ-1 mapping) to replace "Discord, Slack Wave 2" with a
current mapping such as "Discord included in MVP; Slack Wave 2" or "Telegram,
WhatsApp, Discord (MVP); Slack Wave 2" to keep the "REQ-1" mapping consistent
with the completed Discord implementation.
---
Duplicate comments:
In
`@openspec/changes/archive/2026-03-26-channel-image-ingestion-strategy/specs/channel-image-ingestion/spec.md`:
- Around line 109-110: REQ-5 currently mandates that allowed_channels entries be
valid MVP channel names, but Scenario 7 uses ["slack"] which is not an MVP
channel; update the spec so it is internally consistent by either (a) relaxing
REQ-5 to allow non-MVP channel names with runtime validation and document the
new behavior in REQ-5 and the runtime validation rules, or (b) change Scenario
7’s allowed_channels to use an actual MVP channel name (e.g., replace ["slack"]
with a valid MVP channel like ["email"] or another MVP entry) or remove Scenario
7 entirely; locate references to allowed_channels, REQ-5 and "Scenario 7" in the
spec and update the text so the requirement and the scenario are aligned.
- Around line 10-13: The spec contains contradictory Discord status entries:
update all occurrences of "Discord" in this document so they use a single status
(pick either "Implemented for MVP" or "future/contract-only") and match the
chosen wording and tense. Specifically, change the Discord mention in the
opening summary paragraph (the block around the first few lines), the status
entry at the section near line 36 where it currently says "Implemented for MVP",
and the later Discord subsection (around lines 261–270) so they all use
identical status text and any follow-up wording (e.g., "implemented", "will
implement", "contract") is consistent.
In
`@openspec/changes/archive/2026-03-26-channel-image-ingestion-strategy/verify-report.md`:
- Around line 36-38: The verification report contains stale "spec-only" and
placeholder statements (e.g., the paragraph starting "Not applicable — this is a
spec-only change. No implementation to validate against scenarios.") that
conflict with other sections (lines referenced in the review: 6, 26-31, 99-100,
149-151); update the verify-report.md so all assertions are consistent: remove
or replace the "spec-only" placeholder, reconcile the Behavioral validation
statement with the content at lines 6 and 26-31, refresh the outdated sections
around lines 99-100 and 149-151 to reflect current artifacts, and ensure the
file follows the docs guidance for **/*.{md,mdx} (technical accuracy and
alignment with code changes).
In `@openspec/specs/channel-image-ingestion/spec.md`:
- Around line 109-110: Scenario 7 conflicts with REQ-5 because allowed_channels
is restricted to MVP channel names but the scenario uses Slack (a non-MVP
channel); update the scenario or the requirement so they align: either change
Scenario 7 (and the entries at lines referenced 218-221) to use a valid MVP
channel name instead of "Slack", or modify REQ-5/allowed_channels to explicitly
allow Slack and document the exception; ensure references to "allowed_channels",
"REQ-5", and "Scenario 7" are updated consistently.
- Around line 10-13: The spec contains conflicting rollout status for Discord:
update all mentions so they match (choose either "Implemented MVP" or
"Contract-only") to keep documentation consistent; specifically change the
Discord status in the opening summary block (currently lines 10–13), the
"Implemented MVP" list (around line 36) and the later status block (around lines
261–270) so they all state the same rollout state and wording, and ensure any
related changelog/notes or bullets referencing Discord use the identical term
and tense.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 325e2753-6d41-48a8-a62d-1b0bcb1d3e75
📒 Files selected for processing (7)
clients/agent-runtime/src/channels/discord.rsclients/agent-runtime/src/config/schema.rsopenspec/changes/archive/2026-03-26-channel-image-ingestion-strategy/design.mdopenspec/changes/archive/2026-03-26-channel-image-ingestion-strategy/specs/channel-image-ingestion/spec.mdopenspec/changes/archive/2026-03-26-channel-image-ingestion-strategy/tasks.mdopenspec/changes/archive/2026-03-26-channel-image-ingestion-strategy/verify-report.mdopenspec/specs/channel-image-ingestion/spec.md
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: sonar
- GitHub Check: pr-checks
- GitHub Check: pr-checks
- GitHub Check: Cloudflare Pages
🧰 Additional context used
📓 Path-based instructions (7)
clients/agent-runtime/src/**/*.rs
📄 CodeRabbit inference engine (clients/agent-runtime/AGENTS.md)
clients/agent-runtime/src/**/*.rs: Never log secrets, tokens, raw credentials, or sensitive payloads in any logging statements
Avoid unnecessary allocations, clones, and blocking operations to maintain performance and efficiency
Files:
clients/agent-runtime/src/config/schema.rsclients/agent-runtime/src/channels/discord.rs
clients/agent-runtime/**/*.rs
📄 CodeRabbit inference engine (clients/agent-runtime/AGENTS.md)
Run
cargo fmt --all -- --check,cargo clippy --all-targets -- -D warnings, andcargo testfor code validation, or document which checks were skipped and why
Files:
clients/agent-runtime/src/config/schema.rsclients/agent-runtime/src/channels/discord.rs
clients/agent-runtime/src/{security,gateway,tools,config}/**/*.rs
📄 CodeRabbit inference engine (clients/agent-runtime/AGENTS.md)
Do not silently weaken security policy or access constraints; keep default behavior secure-by-default with deny-by-default where applicable
Files:
clients/agent-runtime/src/config/schema.rs
**/*.rs
⚙️ CodeRabbit configuration file
**/*.rs: Focus on Rust idioms, memory safety, and ownership/borrowing correctness.
Flag unnecessary clones, unchecked panics in production paths, and weak error context.
Prioritize unsafe blocks, FFI boundaries, concurrency races, and secret handling.
Files:
clients/agent-runtime/src/config/schema.rsclients/agent-runtime/src/channels/discord.rs
**/*
⚙️ CodeRabbit configuration file
**/*: Security first, performance second.
Validate input boundaries, auth/authz implications, and secret management.
Look for behavioral regressions, missing tests, and contract breaks across modules.
Files:
clients/agent-runtime/src/config/schema.rsopenspec/specs/channel-image-ingestion/spec.mdopenspec/changes/archive/2026-03-26-channel-image-ingestion-strategy/tasks.mdopenspec/changes/archive/2026-03-26-channel-image-ingestion-strategy/specs/channel-image-ingestion/spec.mdopenspec/changes/archive/2026-03-26-channel-image-ingestion-strategy/verify-report.mdopenspec/changes/archive/2026-03-26-channel-image-ingestion-strategy/design.mdclients/agent-runtime/src/channels/discord.rs
**/*.{md,mdx}
⚙️ CodeRabbit configuration file
**/*.{md,mdx}: Verify technical accuracy and that docs stay aligned with code changes.
For user-facing docs, check EN/ES parity or explicitly note pending translation gaps.
Files:
openspec/specs/channel-image-ingestion/spec.mdopenspec/changes/archive/2026-03-26-channel-image-ingestion-strategy/tasks.mdopenspec/changes/archive/2026-03-26-channel-image-ingestion-strategy/specs/channel-image-ingestion/spec.mdopenspec/changes/archive/2026-03-26-channel-image-ingestion-strategy/verify-report.mdopenspec/changes/archive/2026-03-26-channel-image-ingestion-strategy/design.md
clients/agent-runtime/src/channels/**/*.rs
📄 CodeRabbit inference engine (clients/agent-runtime/AGENTS.md)
Implement
Channeltrait insrc/channels/with consistentsend,listen, andhealth_checksemantics and cover auth/allowlist/health behavior with tests
Files:
clients/agent-runtime/src/channels/discord.rs
🧠 Learnings (4)
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/channels/**/*.rs : Implement `Channel` trait in `src/channels/` with consistent `send`, `listen`, and `health_check` semantics and cover auth/allowlist/health behavior with tests
Applied to files:
clients/agent-runtime/src/config/schema.rsopenspec/specs/channel-image-ingestion/spec.mdopenspec/changes/archive/2026-03-26-channel-image-ingestion-strategy/specs/channel-image-ingestion/spec.mdopenspec/changes/archive/2026-03-26-channel-image-ingestion-strategy/verify-report.mdclients/agent-runtime/src/channels/discord.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/main.rs : Preserve CLI contract unless change is intentional and documented; prefer explicit errors over silent fallback for unsupported critical paths
Applied to files:
openspec/specs/channel-image-ingestion/spec.mdopenspec/changes/archive/2026-03-26-channel-image-ingestion-strategy/specs/channel-image-ingestion/spec.mdopenspec/changes/archive/2026-03-26-channel-image-ingestion-strategy/verify-report.mdclients/agent-runtime/src/channels/discord.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/**/*.rs : Run `cargo fmt --all -- --check`, `cargo clippy --all-targets -- -D warnings`, and `cargo test` for code validation, or document which checks were skipped and why
Applied to files:
openspec/changes/archive/2026-03-26-channel-image-ingestion-strategy/specs/channel-image-ingestion/spec.mdopenspec/changes/archive/2026-03-26-channel-image-ingestion-strategy/verify-report.md
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/{security,gateway,tools,config}/**/*.rs : Do not silently weaken security policy or access constraints; keep default behavior secure-by-default with deny-by-default where applicable
Applied to files:
openspec/changes/archive/2026-03-26-channel-image-ingestion-strategy/verify-report.md
🪛 LanguageTool
openspec/specs/channel-image-ingestion/spec.md
[style] ~179-~179: This phrase is redundant (‘G’ stands for ‘graphic’). Use simply “PNG”.
Context: ...app"]` When a WhatsApp user sends a PNG image with no caption Then the channel em...
(ACRONYM_TAUTOLOGY)
openspec/changes/archive/2026-03-26-channel-image-ingestion-strategy/specs/channel-image-ingestion/spec.md
[style] ~179-~179: This phrase is redundant (‘G’ stands for ‘graphic’). Use simply “PNG”.
Context: ...app"]` When a WhatsApp user sends a PNG image with no caption Then the channel em...
(ACRONYM_TAUTOLOGY)
🔇 Additional comments (6)
clients/agent-runtime/src/config/schema.rs (2)
274-275: Allowlist expansion and runtime validation are consistent.The
"discord"MVP addition and the validation error update stay aligned with fail-closed channel gating.Also applies to: 3155-3158
6035-6044: Good test coverage for the new allowlist behavior.You added both a positive Discord case and a negative unsupported-channel case, which guards this contract well.
Also applies to: 6164-6194
clients/agent-runtime/src/channels/discord.rs (4)
106-111: UUID nonce addresses temp-path collision.The per-stage UUID nonce ensures concurrent downloads of identical images don't overwrite each other. This properly addresses the previously raised concern about hash-only paths.
478-483:mention_onlybypass for attachment-only messages fixed.The guard now correctly enforces the mention check regardless of whether
contentis empty, closing the access-control gap for image-only uploads. The regression tests at lines 1177-1205 confirm the expected behavior.
571-608:parse_image_attachmentsis well-structured.The function correctly:
- Filters by
image/*content-type prefix- Validates against the allowlist via
AllowedImageMime::from_mime_str- Skips attachments with empty URLs
- Extracts optional metadata (filename, size)
This matches the patterns used by Telegram and WhatsApp channels.
984-1205: Good test coverage for image attachment parsing and mention_only regression.Tests cover:
- Single/multiple image attachments
- Non-image and unsupported MIME filtering
- Missing/empty fields edge cases
- Text+image and image-only message composition
- Regression tests for
mention_onlywith attachment-only messages
- Sanitize CDN URLs in Discord error logs to prevent token leaks - Capture and log stream read errors instead of discarding with |_| - Loosen REQ-5: non-MVP channels warn at config time, fail-closed at runtime - Update validate_multimodal_config to warn instead of reject non-MVP channels - Fix design.md observability section: events fire at pre-dispatch and staging - Update Discord status to Implemented/MVP in overview and contract sections - Fix tasks.md: header reflects issues are tracked, REQ-1 mapping includes Discord - Remove stale spec-only claims from verify-report, add runtime validation details
…ategy' into feat/channel-image-ingestion-strategy
…266) - Add stream_validate_and_stage() to media.rs for shared post-download flow - Refactor discord.rs and whatsapp.rs to use shared helper (~120 lines deduped) - Add 6 tests for the shared helper (status, size, mime, staging, cleanup) - WhatsApp now also uses UUID nonce in staging filenames (consistency) - Add http as dev-dependency for test mocking
|
1 participant
Summary
openspec/specs/channel-image-ingestion/openspec/changes/archive/cargo fmtindiscord.rsFollow-up issues created in Linear
SDD Artifacts
openspec/specs/channel-image-ingestion/spec.mdopenspec/changes/archive/2026-03-26-channel-image-ingestion-strategy/Closes Define channel ingestion strategy for multimodal image input #266