chore: remove dead upload-path remnants by hieptl · Pull Request #1236 · OpenHands/agent-canvas

hieptl · 2026-06-08T09:15:07Z

A human has tested these changes.

Why

Description:

src/hooks/mutation/use-conversation-upload-files.ts (104 LOC). Its last caller was removed in feat(chat): attachment UX, upload-as-file, and home/cloud submit fixes #712 (attachment UX rework); since then, live uploads flow exclusively through src/api/conversation-file-upload.api.ts. The hook has been maintained only collaterally — most recently fix(upload): resolve relative working dirs against /api/file/home #1106 updated its path handling without it having any consumer. Zero references repo-wide (useConversationUploadFiles / use-conversation-upload-files grep both empty outside the file).
toAbsoluteWorkspacePath() in src/api/workspace-upload-path.ts (~15 LOC with JSDoc). The naive /-prefix helper whose behavior was the root cause of the read-only-filesystem upload bug fix(upload): resolve relative working dirs against /api/file/home #1106 fixed. fix(upload): resolve relative working dirs against /api/file/home #1106 migrated every real caller to the home-anchored resolveAbsoluteWorkspacePath and kept this one only "for cosmetic/log use" — but no such caller exists (only its own unit test referenced it). Its own JSDoc already marks it @deprecated. Removing it eliminates the foot-gun of someone reaching for it again.

The spec is updated to record this: specs/workspace-upload-path.md WUP-001's last bullet now asserts the resolver is the only sanctioned mechanism and notes the legacy helper's removal.

Scope (1 deletion + 3 surgical edits, ~120 LOC removed):

Change	Detail
Delete `src/hooks/mutation/use-conversation-upload-files.ts`	104 LOC
Edit `src/api/workspace-upload-path.ts`	remove `toAbsoluteWorkspacePath` + its `@deprecated` JSDoc (15 LOC); `resolveAbsoluteWorkspacePath` & friends untouched
Edit `__tests__/api/workspace-upload-path.test.ts`	drop the import + the single `it("toAbsoluteWorkspacePath is a naive…")` block (-1 test); all resolver/builder/safe-filename tests untouched
Edit `specs/workspace-upload-path.md`	WUP-001 final bullet updated to record the removal

Acceptance Criteria:

Hook deleted; helper + JSDoc removed; test import + block removed (no new tests written); spec bullet updated.
npm run lint, npm test, npm run build:lib all pass.
git grep "toAbsoluteWorkspacePath\|useConversationUploadFiles\|use-conversation-upload-files" returns only the spec's historical note.
fix(upload): resolve relative working dirs against /api/file/home #1106 author (or team) has confirmed neither removal conflicts with in-flight work.

Out of scope / Notes:

Live upload plumbing is untouched: conversation-file-upload.api.ts, resolveAbsoluteWorkspacePath, buildWorkspaceUploadPath, getSafeUploadFileName, and the /api/file/home caching all remain exactly as fix(upload): resolve relative working dirs against /api/file/home #1106 left them.
Both removals are one git revert from recovery if the team wants them back.

Summary

Removes the last two dead remnants of the pre-#1106 upload-path handling (~120 LOC): the useConversationUploadFiles hook (caller-less since #712) and the @deprecated toAbsoluteWorkspacePath helper (the naive /-prefix function whose behavior caused the bug #1106 fixed; kept then for cosmetic/log callers that never existed). The WUP-001 spec is updated to record the removal.

⚠ Reviewer note: both files were collaterally touched by #1106 three days ago — flagging @chuckbutkus for a quick confirm that nothing in-flight is about to consume them.

Changes

Delete src/hooks/mutation/use-conversation-upload-files.ts (104 LOC)
Edit src/api/workspace-upload-path.ts — remove toAbsoluteWorkspacePath + JSDoc; everything else (resolver, cache, builders) untouched
Edit __tests__/api/workspace-upload-path.test.ts — remove the helper's import and single test block (−1 test; no new tests)
Edit specs/workspace-upload-path.md — WUP-001 final bullet now records the removal and names the resolver as the only sanctioned mechanism

Verification

npm run lint ✅ · npm run build:lib ✅
npm test ✅ — 403 files passed | 2 skipped, 3015 tests passed | 12 skipped | 9 todo; count delta reconciles exactly (−1 = the removed helper test). Full transparency: one unrelated test flaked on the first full run and passed on two consecutive identical re-runs (exit 0, zero code changes between runs) — consistent with the repo's documented Vitest v4 parallel-contamination flakiness (see the use-websocket.test.ts skip TODO).
git grep for both symbols returns only the spec's historical note

Risk

Low, with a social caveat. Reference analysis is unambiguous (zero callers for either symbol), but #1106's recency means the team should confirm intent — hence the explicit reviewer flag. Functionally, the live upload path (conversation-file-upload.api.ts → resolveAbsoluteWorkspacePath → /api/file/home anchor) is byte-for-byte untouched.

Rollback

Revert this single commit; independent of all other units.

Issue Number

Resolves #1235

Type

🐳 Docker images for this PR

• GHCR package: https://github.com/OpenHands/agent-canvas/pkgs/container/agent-canvas

Component	Value
Image	`ghcr.io/openhands/agent-canvas`
Architectures	amd64, arm64
Agent Server	`ghcr.io/openhands/agent-server:1.26.0-python`
Automation	`openhands-automation==1.0.0a6`
Commit	`04560ce4fa89ecf28b2b6d6061856eee53c0eb27`

Pull (multi-arch manifest)

# Multi-arch manifest — Docker automatically pulls the correct architecture
docker pull ghcr.io/openhands/agent-canvas:sha-04560ce

Run

docker run -it --rm \
  -p 8000:8000 \
  ghcr.io/openhands/agent-canvas:sha-04560ce

All tags pushed for this build

ghcr.io/openhands/agent-canvas:sha-04560ce-amd64
ghcr.io/openhands/agent-canvas:hieptl-app-2241-amd64
ghcr.io/openhands/agent-canvas:pr-1236-amd64
ghcr.io/openhands/agent-canvas:sha-04560ce-arm64
ghcr.io/openhands/agent-canvas:hieptl-app-2241-arm64
ghcr.io/openhands/agent-canvas:pr-1236-arm64
ghcr.io/openhands/agent-canvas:sha-04560ce
ghcr.io/openhands/agent-canvas:hieptl-app-2241
ghcr.io/openhands/agent-canvas:pr-1236

About Multi-Architecture Support

Each tag (e.g., sha-04560ce) is a multi-arch manifest supporting both amd64 and arm64
Docker automatically pulls the correct architecture for your platform
Individual architecture tags (e.g., sha-04560ce-amd64) are also available if needed

vercel · 2026-06-08T09:15:14Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
agent-canvas	Ready	Preview, Comment	Jun 8, 2026 9:15am

github-actions · 2026-06-08T09:22:21Z

✅ Mock-LLM E2E Tests

42/42 passed

Commit: 04560ce4 · Workflow run · Test artifacts

Status	Test	Duration
✅	mock-llm-acp-agent.spec.ts › mock-LLM ACP agent conversation › step 1: configure ACP agent via Settings → Agent UI	13.9s
✅	mock-llm-acp-agent.spec.ts › mock-LLM ACP agent conversation › step 2: reload and verify ACP settings are persisted in UI	5.5s
✅	mock-llm-acp-agent.spec.ts › mock-LLM ACP agent conversation › step 3: start ACP conversation and verify agent reply	6.7s
✅	mock-llm-acp-agent.spec.ts › mock-LLM ACP agent conversation › step 4: resume ACP conversation from sidebar after navigating away	5.7s
✅	mock-llm-auth-modes.spec.ts › auth mode: fresh install with runtime-injected key › reaches the onboarding modal without pre-seeded localStorage	1.3s
✅	mock-llm-auth-modes.spec.ts › auth mode: non-public key rotation › recovers when localStorage has a stale session API key	5.3s
✅	mock-llm-auth-modes.spec.ts › auth mode: public gate › shows the auth screen when no key is configured	1.2s
✅	mock-llm-auth-modes.spec.ts › auth mode: public gate › rejects an incorrect key with an inline error	1.4s
✅	mock-llm-auth-modes.spec.ts › auth mode: public gate › allows access after pasting the correct key	1.8s
✅	mock-llm-auth-modes.spec.ts › auth mode: public gate › skips auth screen for returning user with valid stored key	727ms
✅	mock-llm-auth-modes.spec.ts › auth mode: public gate › re-prompts when the server rotates its key (stale localStorage)	1.5s
✅	mock-llm-automation.spec.ts › mock-LLM automation lifecycle › step 1: setup LLM profile and register automation trajectory	7.3s
✅	mock-llm-automation.spec.ts › mock-LLM automation lifecycle › step 2: create automation and dispatch run via the UI	34.4s
✅	mock-llm-automation.spec.ts › mock-LLM automation lifecycle › step 3: verify automation and run on the automations page	6.1s
✅	mock-llm-conversation.spec.ts › mock-LLM agent-server conversation › step 1: create an LLM profile pointing at the mock LLM server	6.2s
✅	mock-llm-conversation.spec.ts › mock-LLM agent-server conversation › step 2: activate the mock-llm profile and verify settings API	6.5s
✅	mock-llm-conversation.spec.ts › mock-LLM agent-server conversation › step 3: run a conversation with the mock LLM	6.6s
✅	mock-llm-conversation.spec.ts › mock-LLM agent-server conversation › step 4: resume conversation from sidebar after navigating away	5.7s
✅	mock-llm-cross-connect.spec.ts › cross-connect: frontend-only → backend-only › frontend-only connects to a separate backend-only instance	15.8s
✅	mock-llm-cross-connect.spec.ts › cross-connect: frontend-only → multiple backends › connects to two separate backends and switches between them	20.6s
✅	mock-llm-image-upload.spec.ts › mock-LLM image upload › attaching an image embeds it as base64 in the LLM completion call	13.5s
✅	mock-llm-model-switch.spec.ts › mock-LLM /model slash command › step 1: configure LLM, create switch-target profile, register trajectory	7.2s
✅	mock-llm-model-switch.spec.ts › mock-LLM /model slash command › step 2: start conversation, switch profile via /model, verify switch	7.0s
✅	mock-llm-onboarding-happy-path.spec.ts › onboarding happy path › completes the full onboarding flow and launches a conversation	4.4s
✅	mock-llm-onboarding-regressions.spec.ts › onboarding recent regressions › keeps the modal open on backdrop click and Escape	1.3s
✅	mock-llm-onboarding-regressions.spec.ts › onboarding recent regressions › defaults the LLM setup step to OpenAI GPT-5.5	1.6s
✅	mock-llm-partial-stack.spec.ts › partial stack: --frontend-only › serves the frontend but returns 503 for backend routes	7.3s
✅	mock-llm-partial-stack.spec.ts › partial stack: --backend-only › serves backend APIs but returns 503 for the frontend root	14.1s
✅	mock-llm-partial-stack.spec.ts › partial stack: port conflict › fails with a clear error when the ingress port is occupied	108ms
✅	mock-llm-partial-stack.spec.ts › partial stack: port conflict › starts successfully on a free port after a conflict	6.0s
✅	mock-llm-preset-automation.spec.ts › preset automation → slash command conversation › automation card sends the correct slash command to a conversation	16.0s
✅	mock-llm-preset-automation.spec.ts › preset automation → slash command conversation › direct slash command from home page triggers skill activation	13.4s
✅	mock-llm-profile-management.spec.ts › active profile deletion + reconciliation › active profile is deletable and reconciliation activates another profile	6.4s
✅	mock-llm-profile-management.spec.ts › same-model profile identity › chat header shows the correct profile when two profiles share the same model	7.6s
✅	mock-llm-profile-management.spec.ts › litellm_proxy proxy base_url preservation › re-saving a litellm_proxy profile from Basic view preserves the proxy base_url	7.5s
✅	mock-llm-skills.spec.ts › skill loading: project, user, and deletion › project skill in workspace/.agents/skills/ triggers on matching keyword	7.5s
✅	mock-llm-skills.spec.ts › skill loading: project, user, and deletion › user skill in ~/.openhands/skills/ triggers on matching keyword	6.4s
✅	mock-llm-skills.spec.ts › skill loading: project, user, and deletion › deleting a user skill removes it from subsequent conversations	6.4s
✅	mock-llm-ui-regressions.spec.ts › UI regressions › scopes standalone styles to the agent-server-ui shell	1.3s
✅	mock-llm-ui-regressions.spec.ts › UI regressions › loads older events when scrolling up	1.6s
✅	mock-llm-ui-regressions.spec.ts › UI regressions › selected workspace persists after navigating away and returning	2.5s
✅	mock-llm-ui-regressions.spec.ts › UI regressions › cleared sessionStorage yields empty workspace selection	905ms

_{Posted by the Mock-LLM E2E workflow · results are deterministic (scripted LLM responses)}

github-actions · 2026-06-08T09:25:32Z

📸 Snapshot Test Report

Warning

Snapshot comparison step crashed (timeout, OOM, or runner error) — diff results below may be incomplete or absent.
Check the CI logs for the full error output (look for the "Run snapshot comparison" step).

✅ All snapshots match the main branch baselines.

Category	Count
🔴 Changed	0
🆕 New	0
✅ Unchanged	73
Total	73

✅ Unchanged snapshots (73)

archived-conversation

conversation-panel-with-archived-badges
conversation-view-archived
conversation-view-sandbox-error

automations

automations-delete-modal
automations-list-active-inactive
automations-no-automations
automations-search-no-results

backends-extended

backend-add-blank-disabled
backend-add-cloud-advanced-open
backend-add-cloud-no-key-disabled
backend-add-cloud-with-key-enabled
backend-add-form-partially-filled
backend-add-invalid-url-disabled
backend-add-local-ready
backend-add-name-only-disabled
backend-add-two-column-layout
backend-add-whitespace-host-disabled
backend-after-switch
backend-cancel-nothing-saved
backend-dropdown-two-backends
backend-edit-prefilled
backend-manage-after-removal
backend-manage-two-listed
backend-remove-cancelled
backend-remove-confirmation
backend-switch-overlay

backends

backend-add-modal
backend-manage-modal
backend-selector-open

changes-tab

changes-deleted-file
changes-diff-viewer
changes-empty

collapsible-thinking

reasoning-content-collapsed
reasoning-content-expanded
think-action-collapsed
think-action-expanded

mcp-page

mcp-custom-server-1-editor-open
mcp-custom-server-2-url-filled
mcp-custom-server-3-all-filled
mcp-custom-server-4-installed
mcp-custom-server-editor
mcp-empty-installed
mcp-search-filtered
mcp-slack-install-1-marketplace
mcp-slack-install-2-modal
mcp-slack-install-3-filled
mcp-slack-install-4-installed

onboarding

onboarding-step-0-check-backend
onboarding-step-1-choose-agent
onboarding-step-2-setup-llm
onboarding-step-3-say-hello

projects-workspace-browser

projects-workspace-browser

settings-page

add-backend-modal
analytics-consent-modal
home-screen
settings-app-page
settings-page

settings-secrets

secrets-add-form-filled
secrets-add-form
secrets-after-save
secrets-delete-confirm
secrets-list

settings-verification

condenser-settings
verification-settings-off
verification-settings-on

sidebar

sidebar-collapsed
sidebar-conversation-panel
sidebar-filter-menu

skills-page

skills-empty
skills-loaded
skills-no-match
skills-search-filtered
skills-type-filter

Generated by the Snapshot Tests workflow. This comment was created by an AI agent (OpenHands) on behalf of the repo maintainers.

github-actions · 2026-06-08T09:27:23Z

🔶 Mock-LLM Docker E2E Test Results

37/42 passed · 5 skipped

Commit: 04560ce4 · Workflow run · Test artifacts

Status	Test	Duration
✅	mock-llm-acp-agent.spec.ts › mock-LLM ACP agent conversation › step 1: configure ACP agent via Settings → Agent UI	14.0s
✅	mock-llm-acp-agent.spec.ts › mock-LLM ACP agent conversation › step 2: reload and verify ACP settings are persisted in UI	5.6s
✅	mock-llm-acp-agent.spec.ts › mock-LLM ACP agent conversation › step 3: start ACP conversation and verify agent reply	6.8s
✅	mock-llm-acp-agent.spec.ts › mock-LLM ACP agent conversation › step 4: resume ACP conversation from sidebar after navigating away	5.8s
✅	mock-llm-auth-modes.spec.ts › auth mode: fresh install with runtime-injected key › reaches the onboarding modal without pre-seeded localStorage	1.4s
✅	mock-llm-auth-modes.spec.ts › auth mode: non-public key rotation › recovers when localStorage has a stale session API key	5.4s
✅	mock-llm-auth-modes.spec.ts › auth mode: public gate › shows the auth screen when no key is configured	1.2s
✅	mock-llm-auth-modes.spec.ts › auth mode: public gate › rejects an incorrect key with an inline error	1.4s
✅	mock-llm-auth-modes.spec.ts › auth mode: public gate › allows access after pasting the correct key	1.5s
✅	mock-llm-auth-modes.spec.ts › auth mode: public gate › skips auth screen for returning user with valid stored key	797ms
✅	mock-llm-auth-modes.spec.ts › auth mode: public gate › re-prompts when the server rotates its key (stale localStorage)	1.5s
✅	mock-llm-automation.spec.ts › mock-LLM automation lifecycle › step 1: setup LLM profile and register automation trajectory	9.0s
✅	mock-llm-automation.spec.ts › mock-LLM automation lifecycle › step 2: create automation and dispatch run via the UI	37.6s
✅	mock-llm-automation.spec.ts › mock-LLM automation lifecycle › step 3: verify automation and run on the automations page	6.3s
✅	mock-llm-conversation.spec.ts › mock-LLM agent-server conversation › step 1: create an LLM profile pointing at the mock LLM server	6.3s
✅	mock-llm-conversation.spec.ts › mock-LLM agent-server conversation › step 2: activate the mock-llm profile and verify settings API	6.1s
✅	mock-llm-conversation.spec.ts › mock-LLM agent-server conversation › step 3: run a conversation with the mock LLM	6.3s
✅	mock-llm-conversation.spec.ts › mock-LLM agent-server conversation › step 4: resume conversation from sidebar after navigating away	5.7s
⏭️	mock-llm-cross-connect.spec.ts › cross-connect: frontend-only → backend-only › frontend-only connects to a separate backend-only instance	189ms
⏭️	mock-llm-cross-connect.spec.ts › cross-connect: frontend-only → multiple backends › connects to two separate backends and switches between them	178ms
✅	mock-llm-image-upload.spec.ts › mock-LLM image upload › attaching an image embeds it as base64 in the LLM completion call	13.3s
✅	mock-llm-model-switch.spec.ts › mock-LLM /model slash command › step 1: configure LLM, create switch-target profile, register trajectory	7.2s
✅	mock-llm-model-switch.spec.ts › mock-LLM /model slash command › step 2: start conversation, switch profile via /model, verify switch	6.6s
✅	mock-llm-onboarding-happy-path.spec.ts › onboarding happy path › completes the full onboarding flow and launches a conversation	3.4s
✅	mock-llm-onboarding-regressions.spec.ts › onboarding recent regressions › keeps the modal open on backdrop click and Escape	1.4s
✅	mock-llm-onboarding-regressions.spec.ts › onboarding recent regressions › defaults the LLM setup step to OpenAI GPT-5.5	1.6s
⏭️	mock-llm-partial-stack.spec.ts › partial stack: --frontend-only › serves the frontend but returns 503 for backend routes	189ms
✅	mock-llm-partial-stack.spec.ts › partial stack: --backend-only › serves backend APIs but returns 503 for the frontend root	26.1s
⏭️	mock-llm-partial-stack.spec.ts › partial stack: port conflict › fails with a clear error when the ingress port is occupied	0ms
⏭️	mock-llm-partial-stack.spec.ts › partial stack: port conflict › starts successfully on a free port after a conflict	1ms
✅	mock-llm-preset-automation.spec.ts › preset automation → slash command conversation › automation card sends the correct slash command to a conversation	16.1s
✅	mock-llm-preset-automation.spec.ts › preset automation → slash command conversation › direct slash command from home page triggers skill activation	13.3s
✅	mock-llm-profile-management.spec.ts › active profile deletion + reconciliation › active profile is deletable and reconciliation activates another profile	6.4s
✅	mock-llm-profile-management.spec.ts › same-model profile identity › chat header shows the correct profile when two profiles share the same model	7.5s
✅	mock-llm-profile-management.spec.ts › litellm_proxy proxy base_url preservation › re-saving a litellm_proxy profile from Basic view preserves the proxy base_url	7.0s
✅	mock-llm-skills.spec.ts › skill loading: project, user, and deletion › project skill in workspace/.agents/skills/ triggers on matching keyword	6.6s
✅	mock-llm-skills.spec.ts › skill loading: project, user, and deletion › user skill in ~/.openhands/skills/ triggers on matching keyword	6.3s
✅	mock-llm-skills.spec.ts › skill loading: project, user, and deletion › deleting a user skill removes it from subsequent conversations	6.3s
✅	mock-llm-ui-regressions.spec.ts › UI regressions › scopes standalone styles to the agent-server-ui shell	1.4s
✅	mock-llm-ui-regressions.spec.ts › UI regressions › loads older events when scrolling up	1.6s
✅	mock-llm-ui-regressions.spec.ts › UI regressions › selected workspace persists after navigating away and returning	1.9s
✅	mock-llm-ui-regressions.spec.ts › UI regressions › cleared sessionStorage yields empty workspace selection	930ms

_{Posted by the Mock-LLM E2E workflow · results are deterministic (scripted LLM responses)}

chore: remove dead upload-path remnants

04560ce

hieptl self-assigned this Jun 8, 2026

vercel Bot deployed to Preview June 8, 2026 09:15 View deployment

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: remove dead upload-path remnants#1236

chore: remove dead upload-path remnants#1236
hieptl wants to merge 1 commit into
mainfrom
hieptl/app-2241

hieptl commented Jun 8, 2026 •

edited by github-actions Bot

Loading

Uh oh!

vercel Bot commented Jun 8, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Jun 8, 2026

Uh oh!

github-actions Bot commented Jun 8, 2026

Uh oh!

github-actions Bot commented Jun 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

hieptl commented Jun 8, 2026 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why

Summary

Changes

Verification

Risk

Rollback

Issue Number

Type

Uh oh!

vercel Bot commented Jun 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Jun 8, 2026

✅ Mock-LLM E2E Tests

Uh oh!

github-actions Bot commented Jun 8, 2026

📸 Snapshot Test Report

Uh oh!

github-actions Bot commented Jun 8, 2026

🔶 Mock-LLM Docker E2E Test Results

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

hieptl commented Jun 8, 2026 •

edited by github-actions Bot

Loading

vercel Bot commented Jun 8, 2026 •

edited

Loading