Skip to content

fix(executor): use correct workspace path for attachment downloads - #8

Merged
Micro66 merged 1 commit into
weagent/fix-cancel-task-state-cleanupfrom
weagent/fix-attachment-download-path
Jan 12, 2026
Merged

Micro66 merged 1 commit into
weagent/fix-cancel-task-state-cleanupfrom
weagent/fix-attachment-download-path

Conversation

@Micro66

@Micro66 Micro66 commented Jan 11, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • Fix attachment download path to save files to the correct location
  • Changed from project_path (repo directory) to WORKSPACE_ROOT/task_id (workspace root)
  • This ensures attachments are accessible at /workspace/{task_id}/{task_id}:executor:attachments/{subtask_id}/ instead of being incorrectly nested inside the repository directory

Problem

  • Expected path: /workspace/115794/115794:executor:attachments/119916/filename.png
  • Actual path (bug): /workspace/115794/Wegent/115794:executor:attachments/119916/filename.png

Root Cause

The attachment downloader was using self.project_path (which points to the cloned repository directory) as the base path, instead of WORKSPACE_ROOT/task_id.

Test plan

  • Verify attachments are downloaded to the correct path
  • Confirm attachment references in prompts resolve correctly

Summary by CodeRabbit

  • Refactor
    • Improved attachment storage to use a standardized path structure, simplifying how attachments are organized and accessed within the system.

✏️ Tip: You can customize this high-level summary in your review settings.

Change attachment download path from project_path to WORKSPACE_ROOT/task_id to ensure attachments are saved to the correct location.

Expected: /workspace/{task_id}/{task_id}:executor:attachments/{subtask_id}/
Previous: /workspace/{task_id}/{repo_name}/{task_id}:executor:attachments/{subtask_id}/
@coderabbitai

coderabbitai Bot commented Jan 11, 2026 •

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

The attachment download functionality in the Claude code agent was modified to consistently use the workspace root with task ID as the storage location, removing logic that conditionally used project-specific paths. This simplifies attachment path handling.

Changes

Cohort / File(s) Summary
Attachment Path Simplification
executor/agents/claude_code/claude_code_agent.py
Modified _download_attachments method's workspace path calculation to directly use WORKSPACE_ROOT/task_id instead of conditionally using project-specific paths. Attachments are now stored consistently at the top level of the task workspace rather than in repo-specific subdirectories.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 Hops through the file tree with glee,
No nested paths, just clarity!
From root to task, so clean and bright,
Attachments organized just right! ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main fix: correcting the workspace path for attachment downloads from a repo subdirectory to the workspace root.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between aee7a18 and acdaebc.

📒 Files selected for processing (1)
  • executor/agents/claude_code/claude_code_agent.py
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-18T02:09:09.776Z
Learnt from: CR
Repo: wecode-ai/Wegent PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T02:09:09.776Z
Learning: Applies to executor/agents/claude_code/**/*.py : ClaudeCode agent MUST support Claude Code SDK, Git clone, Skills, MCP servers, and custom instructions (.cursorrules, .windsurfrules)

Applied to files:

  • executor/agents/claude_code/claude_code_agent.py
⏰ 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). (6)
  • GitHub Check: E2E Tests (Shard 2/3)
  • GitHub Check: E2E Tests (Shard 1/3)
  • GitHub Check: E2E Tests (Shard 3/3)
  • GitHub Check: Test Frontend
  • GitHub Check: Test Backend (3.10)
  • GitHub Check: Test wegent CLI Integration
🔇 Additional comments (1)
executor/agents/claude_code/claude_code_agent.py (1)

386-390: Incorrect line numbers and invalid concern about directory creation.

The review references lines 386-390, which are not about attachment downloads. The actual workspace path calculation is at approximately lines 1386-1390 in the _download_attachments method. More importantly, the concern about missing os.makedirs() is invalid—AttachmentDownloader.download_all() explicitly creates the attachments directory internally using Path(attachments_dir).mkdir(parents=True, exist_ok=True).

The code change itself is correct and well-documented. It properly uses WORKSPACE_ROOT/task_id as the base path for attachments, eliminating the buggy behavior of storing attachments inside the cloned repository.

Likely an incorrect or invalid review comment.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@Micro66
Micro66 changed the base branch from main to weagent/fix-cancel-task-state-cleanup January 12, 2026 02:44
@Micro66
Micro66 merged commit fefc698 into weagent/fix-cancel-task-state-cleanup Jan 12, 2026
14 checks passed
@qdaxb
qdaxb deleted the weagent/fix-attachment-download-path branch February 4, 2026 09:32
qdaxb pushed a commit that referenced this pull request May 12, 2026
…e base (#1057)

* feat(knowledge): add multi-level folder support for personal knowledge base

Add real folder hierarchy with API-backed CRUD operations, replacing
the frontend-only virtual folders based on document name parsing.

Backend changes:
- Add knowledge_folders table with parent_id for tree hierarchy
- Add folder_id column to knowledge_documents (0 = root level)
- Implement KnowledgeFolderService with tree building and cascade delete
- Add folder CRUD API endpoints under /knowledge-bases/{id}/folders
- Add document move endpoint under /knowledge-documents/{id}/move
- Update document list to support folder_id filtering

Frontend changes:
- Refactor FolderTree to use real folder data from API
- Add CreateFolderDialog and DeleteFolderDialog components
- Add useFolders hook for folder state management
- Add folder-related i18n keys (en and zh-CN)
- Add folder TypeScript types and API functions

* fix: resolve lint errors - remove unused variables and fix formatting

- Apply Black/isort formatting to schema and folder_service files
- Remove unused import KnowledgeFolder from DocumentList
- Remove unused destructuring of foldersLoading and moveDocument
- Remove unused rootDocs parameter from convertFolderToNode

* fix: resolve TypeScript errors in FolderTree and test files

- Add treeNodeKey helper to fix discriminated union type narrowing
- Add folder_id field to mock KnowledgeDocument objects in test

* fix: address CodeRabbit review feedback for folder feature

- Add field_validator to reject whitespace-only folder names in schemas
- Add kind_id filter to _collect_descendant_ids for scoped BFS traversal
- Add data-testid attributes to CreateFolderDialog inputs and buttons
- Replace hardcoded English toast strings with i18n translation keys
- Fix expandedFolders to sync with asynchronously loaded folder data

* fix: improve HTTP status mapping and prevent premature input clearing

- Use _raise_document_detail_http_error for folder endpoint error mapping (404/403/400)
- Remove premature setName('') in CreateFolderDialog to preserve user input on API failure

* fix(frontend): improve knowledge base detail page UX and folder features (#8)

* fix(frontend): improve knowledge base detail page UX and folder features

- Fix double page refresh when opening notebook KB detail page by deferring
  selectedKb update until full data arrives, and skipping selectKb on main page
- Fix folders not showing when knowledge base has no documents
- Add folder selector in document upload dialog for all upload modes
- Add move-to-folder functionality for moving documents between folders

* fix(frontend): add missing useCallback import in DocumentList

The useCallback hook was used but not imported from React, causing a
ReferenceError when rendering the DocumentList component.

* fix: support folder selection when creating web documents via scraping

Add folder_id support to the web document creation flow, enabling
users to specify a target folder when adding web documents to a
knowledge base.

Backend changes:
- Add folder_id to WebDocumentCreateRequest schema
- Pass folder_id through orchestrator's create_web_document
- Add folder_id to KnowledgeDocumentCreateV1 schema
- Pass folder_id in knowledge_open v1 API endpoint

Frontend changes:
- Add folderId parameter to createWebDocument API function
- Pass selectedUploadFolderId in handleWebAdd handler

* perf(backend): optimize knowledge folder service SQL queries for large-scale tables

Fix orphaned KnowledgeFolder records when deleting knowledge base

Replace iterative BFS with recursive CTE for descendant collection:
- Single-query traversal using WITH RECURSIVE instead of N+1 round-trips
- Fallback to level-batch BFS for databases without CTE support

Batch large IN-clause operations to prevent query plan degradation:
- _bulk_update_folder_id(): batch size 1000 for document moves
- _bulk_delete_folders(): batch size 1000 for folder deletions

* fix(backend): add folder_id parameter to MCP tools and orchestrator

- Add folder_id parameter to create_document_with_content() in orchestrator
- Add folder_id parameter to MCP tool create_document for AI Agent document creation
- Add folder_id filter parameter to MCP tool list_documents for folder filtering
- Pass folder_id in knowledge_open.py for non-web document types (text/file/attachment)

Fixes review issues:
1. MCP create_document missing folder_id parameter
2. MCP list_documents missing folder_id filter parameter
3. orchestrator.create_document_with_content() missing folder_id parameter

All 66 related tests pass.

* fix(knowledge): add folder ownership validation and fix UI touch targets

- Add knowledge_base_id validation in folder_service update/delete methods
- Add folder ownership check in create_document before persisting
- Fix stale async race condition in useKnowledgeSidebar with ref token guard
- Fix SelectTrigger touch targets in DocumentUpload (44x44px baseline)
- Add data-testid and mobile sizing to move button in DocumentItem
- Fix Create folder button sizing in DocumentList
- Enforce ge=0 constraint on folder_id in web_scraper

* fix(knowledge): address race conditions and error handling in folder operations

- Add row-level locks (SELECT FOR UPDATE) in update_folder() move logic
- Narrow exception catch from Exception to ProgrammingError for CTE fallback
- Map permission errors to HTTP 403 in _raise_document_detail_http_error()
- Add missing return type annotations to folder/document endpoints
- Fix duplicate error handler call in delete_folder()

* fix: folder move locking order to avoid deadlocks and broaden recursive CTE fallback to handle OperationalError.
qdaxb pushed a commit that referenced this pull request Jun 10, 2026
* feat(chat): add context status metrics and toolbar indicator

* feat(chat): finalize Phase 1 chat status toolbar and reconnect recovery

- Replace QuotaUsage with ChatToolbarStatus driven by chat:status_updated
- Wire useChatStatusIndicator to derive a fallback snapshot from the
  latest persisted AI message so toolbar status survives cold reloads
  before any live socket event arrives
- Type UnifiedMessage.result.context_metrics so persisted snapshots
  flow through the state machine without runtime casts

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(chat_shell): add TruncationPolicy and GuardSource Protocol (T1)

Introduce the shared types module for Phase 2 context governance:
- TruncationPolicy: frozen dataclass with kind (bytes/tokens) and limit
- GuardSource: runtime_checkable Protocol for guard source adapters
- DEFAULT_EMERGENCY_RATIO = 0.3 and default_emergency_policy helper

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(chat_shell): add ToolOutputGuardAdapter and compact tool string format (T2)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(chat_shell): tighten compact footer detection and dedupe encoding (T2 review)

* feat(chat_shell): add UnifiedContextGuard with source + compression pipeline (T3)

Phase 2 unified context guard. Runs as LangGraph pre_model_hook before every
model call (pre-turn and mid-turn alike) and applies:

1. Source-level pass: convert raw tool messages to compact form via the
   registered GuardSource adapters; mark `compacted=true` so subsequent
   passes treat them as already-handled.
2. Request-level compression: when live state exceeds the model trigger
   limit, run MessageCompressor and translate its diff into LangGraph
   state updates (RemoveMessage for dropped ids, fresh BaseMessage for
   synthesized summaries; compacted=true marker propagates).
3. Emergency re-truncation: stub for T7; logs a warning when compression
   alone is insufficient.

Skips messages without a stable id since add_messages cannot upsert them
without doubling content.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(chat_shell): wire UnifiedContextGuard as pre_model_hook (T4)

Phase 2 wiring task:

- chat_service constructs UnifiedContextGuard with ToolOutputGuardAdapter
  as the only source. The guard is chained AFTER guidance_consumer's hook
  via chain_pre_model_hooks so guidance injection observes already-budgeted
  state and writes its llm_input_messages on top of the compacted view.
- agent.build_messages no longer invokes MessageCompressor — budget
  enforcement now lives entirely in the pre_model_hook, which fires on
  pre-turn AND every mid-turn follow-up. Cache-breakpoint application
  remains in build_messages on the assembled layout.
- Drops the now-unused MessageCompressor import from agent.py.
- Adds guard/composition.py with chain_pre_model_hooks, supporting mixed
  sync/async hooks; messages updates concatenate, llm_input_messages from
  the last hook wins.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(chat_shell): finalize Phase 2 context governance (T7+T8+T9 + review)

Completes Phase 2 by adding emergency re-truncation, removing legacy
serialization-time truncation, refactoring the metrics tracker to
emit-only, and addressing the review findings on the resulting code.

T7 — Emergency re-truncation
- UnifiedContextGuard stage 3 now re-renders source-owned messages
  under each source's emergency policy, biggest-first with early stop
  once usage drops below trigger.
- ToolOutputGuardAdapter accepts an emergency_ratio (defaults to
  DEFAULT_EMERGENCY_RATIO); chat_service passes settings.EMERGENCY_TOOL_OUTPUT_RATIO.
- Synthesized messages from stage 2 are excluded from the candidate set
  so the upsert-by-id contract holds.

T8 — Delete legacy _truncate_tool_result_for_storage
- graph_builder._serialize_messages_chain no longer chops tool results
  at serialization time; budget enforcement is owned by the guard.
- Removed MAX_TOOL_RESULT_LENGTH from config and its tests.

T9 — Tracker → emit-only wrapper
- ContextMetricsTracker no longer keeps its own messages duplicate.
  Single API: capture(messages, phase). Snapshot computation is
  delegated to an injected metrics_fn so accounting flows from one
  source.
- chat_service captures BUILD_MESSAGES at turn start and FINAL at end
  with messages + agent_builder._last_messages_chain.
- tools/events drops the per-tool tracker calls.

Review fixes
- #1: extract_raw now parses already-compact strings via the new
  _split_compact helper, so stage 3 emergency re-render no longer
  produces nested headers / fabricated total_tokens counts.
- #2: UnifiedContextGuard.__call__ is async and emits a metrics
  snapshot via the injected tracker on every invocation. Phase is
  PHASE_AFTER_COMPACTION when compaction happened, PHASE_AFTER_TOOL_END
  otherwise; bucket-throttling in should_emit_status_update suppresses
  redundant emits. Tests converted to async.
- #3: ToolResultTruncationStrategy now skips messages flagged
  additional_kwargs.compacted=True. Without this, stage 2 compression
  would corrupt the guard's fixed-format compact strings, leaving a
  misleading total_tokens count and mixing two truncation notices.
- #5: Documented the source-disjointness assumption in
  _apply_source_pass — registered GuardSources must have non-overlapping
  applies_to predicates.
- #6: calculate_context_metrics accepts pre-built TokenCounter and
  ModelContextConfig; guard.metrics() reuses its instance state
  instead of constructing fresh ones on every snapshot.
- #8: Kept default_emergency_policy as a documented public reference
  helper for future GuardSource impls.
- #9: _dict_to_base_message forwards d.get('id') so the compressor's
  synthesized ids survive the round-trip.
- #10: New test verifies cache_control markers in additional_kwargs
  survive guard compaction (Anthropic prompt-cache compatibility).

Tests
- 14 new tests across guard composition, tracker emit, compact-string
  roundtrip, id propagation, cache-breakpoint preservation, and
  compressor skip behavior. Total chat_shell suite: 549 passing.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* feat(chat-shell): finalize phase2 context guard

* fix(chat-shell): tune tool guard controls and diagnostics

* feat(knowledge): add listing pagination and guard fixes

* fix(frontend): improve chat toolbar status sections

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant