feat(artifacts): Save-As dialog + in-progress wiring + failed-card retry (#3162)#4127
Merged
senamakel merged 15 commits intoJun 25, 2026
Merged
Conversation
…umansai#3162) Add args.json sidecar (save_artifact_args/read_artifact_args) next to meta.json, plus a REGENERATE_TARGET_ID task-local so create_artifact can reuse an existing artifact id/dir in place instead of minting a fresh UUID. Foundation for the failed-card Retry re-dispatch.
…sai#3162) After reserving the artifact, write the verbatim tool args to args.json (best-effort) so a failed card can re-dispatch the same deck spec deterministically without round-tripping through the LLM.
…inyhumansai#3162) Add openhuman.ai_regenerate: reloads meta + args.json, rebuilds the producer tool + SecurityPolicy, and re-runs it inside REGENERATE_TARGET_ID + APPROVAL_CHAT_CONTEXT scopes so the same artifact id is reused and pending/ready/failed events route back to the originating chat. Presentation-only for now (the one producer that persists args). Wires the controller into the registry + updates the coverage tests (3 -> 4 functions).
…i#3162) default-features off + xdg-portal so Linux stays off GTK; tokio feature drives the async dialog backend. rfd talks to the OS dialog APIs directly and avoids the tauri-plugin-fs/schemars conflict that blocked a dialog in tinyhumansai#2779.
…3162) Add save_artifact_via_dialog: rfd-backed native Save-As pre-filled with the artifact filename, copies to the chosen path, returns Ok(None) on cancel. Factor shared validate_source + copy_to_path helpers; keep the macOS/Linux Downloads fallback gated and add unit tests for the new path.
…nsai#3162) Ungate the artifact_commands module (Save-As is cross-platform; the Downloads command stays macOS/Linux-gated inside), add the handler entry, and wire the allow-artifact-save permission into the default capability.
…umansai#3162) Add ArtifactPendingEvent + onArtifactPending socket listener (mirrors ready/failed, no size/path yet) so the chat runtime can render an in-progress card the moment a producer reserves an artifact. Add aiRegenerate() which calls openhuman.ai_regenerate with the socket client_id for event routing.
…inyhumansai#3162) Dispatch upsertArtifactInProgressForThread on onArtifactPending so the spinner shows before any bytes land, then swaps in place on ready/failed. Add a no-downgrade guard to the reducer: a late/duplicate pending must not regress a 'ready' card back to a spinner (failed -> in_progress is allowed — that is an explicit retry).
…3162) ArtifactCard Download now calls saveArtifactViaDialog (OS Save-As pre-filled with the filename), treating cancel as a no-op. Factor the shared ai_get_artifact resolve out of downloadArtifact; the dialog path falls back to the Downloads copy when no portal is available. Add the CANCELLED arm to the files-panel error localizer.
Pass onRetry on the live ArtifactCard so a failed deck re-dispatches via ai_regenerate, reusing the artifact id so the card swaps in place. Remove the stale tinyhumansai#3162 placeholder comment.
…umansai#3162) Store: args.json round-trip, missing-args error, fresh-UUID vs reused-id inside REGENERATE_TARGET_ID scope. Ops: ai_regenerate input guards, non-presentation + missing-args rejections, and a full happy-path that re-runs the engine in place and lands the reused id at Ready.
…sai#3162) Add specs for the artifact_pending socket listener (flatten + drop malformed), the slice no-downgrade guard (ready stays ready; failed->in_progress allowed), saveArtifactViaDialog (save/cancel/ fallback/resolve-fail), aiRegenerate routing, and the provider onArtifactPending dispatch. Point the ArtifactCard download specs at the new Save-As path.
Contributor
📝 WalkthroughWalkthroughThe PR adds a native save-file export path for artifacts, wires ChangesArtifact UX flow
Sequence Diagram(s)sequenceDiagram
participant Conversations
participant chatService
participant handle_regenerate
participant ops_ai_regenerate as ops::ai_regenerate
participant store
participant PresentationTool
Conversations->>chatService: onRetry(artifactId, threadId)
chatService->>handle_regenerate: openhuman.ai_regenerate
handle_regenerate->>ops_ai_regenerate: ai_regenerate(...)
ops_ai_regenerate->>store: read_artifact_args(workspace_dir, artifact_id)
ops_ai_regenerate->>PresentationTool: execute(...)
PresentationTool->>store: save_artifact_args(workspace_dir, meta.id, args)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
coderabbitai
Bot
added
feature
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 688f3578af
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Comment on lines
+43
to
+46
|
|
||
| // `rfd` drives the OS-native dialog. On Linux this is the xdg-desktop | ||
| // portal (no GTK link); on macOS/Windows the system panel. The await | ||
| // resolves when the user picks a path or cancels. |
There was a problem hiding this comment.
When the renderer invokes this new command directly, source_path only has to be an absolute path that exists, so any readable local file can be copied through the Save-As dialog under an artifact-looking filename. Since the command is exposed by the Tauri capability, validate that the source resolves under the workspace artifacts/ root (or accept an artifact id and resolve it in Rust) before opening the dialog/copying.
Useful? React with 👍 / 👎.
Collaborator
Author
There was a problem hiding this comment.
Addressed in 6e4562a — both export commands now reject any source that does not canonicalize to inside the OpenHuman data dir's artifacts/ tree (via assert_artifact_source), so the renderer can no longer copy an arbitrary local file out through the dialog.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src-tauri/src/lib.rs (1)
3616-3623: 🩺 Stability & Availability | 🟡 MinorWindows can still strand users if the Save-As dialog throws.
save_artifact_via_dialogfalls back todownloadArtifact(), but that path invokesdownload_artifact_to_downloads, which is still gated to macOS/Linux here and inartifact_commands.rs. If the dialog ever fails on Windows, the recovery path becomes a missing-command error instead of a Downloads copy. Either make the download command available on Windows too, or skip that fallback there and surface the dialog error directly.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src-tauri/src/lib.rs` around lines 3616 - 3623, The fallback path for `save_artifact_via_dialog` is broken on Windows because it calls `downloadArtifact()` while `download_artifact_to_downloads` remains gated to macOS/Linux. Update the handler registration in `lib.rs` and the corresponding definition in `artifact_commands.rs` so the download command is available on Windows too, or change `save_artifact_via_dialog` to avoid that fallback on Windows and return the dialog error directly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/openhuman/artifacts/store.rs`:
- Around line 449-459: Preserve the original created_at on the regenerate path
in Store::create_artifact (or the artifact metadata write path near the
REGENERATE_TARGET_ID handling), since reusing an existing artifact id should not
bump it to the top of the created_at-sorted list. Update the logic that
builds/writes the artifact meta so it only sets created_at to Utc::now() for new
UUIDs and reuses the existing timestamp when REGENERATE_TARGET_ID is present and
non-empty.
---
Outside diff comments:
In `@app/src-tauri/src/lib.rs`:
- Around line 3616-3623: The fallback path for `save_artifact_via_dialog` is
broken on Windows because it calls `downloadArtifact()` while
`download_artifact_to_downloads` remains gated to macOS/Linux. Update the
handler registration in `lib.rs` and the corresponding definition in
`artifact_commands.rs` so the download command is available on Windows too, or
change `save_artifact_via_dialog` to avoid that fallback on Windows and return
the dialog error directly.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3fa4aa6f-e970-47aa-be06-fd0ee82e6cbf
⛔ Files ignored due to path filters (1)
app/src-tauri/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (24)
app/src-tauri/Cargo.tomlapp/src-tauri/capabilities/default.jsonapp/src-tauri/permissions/allow-artifact-save.tomlapp/src-tauri/src/artifact_commands.rsapp/src-tauri/src/lib.rsapp/src/components/chat/ArtifactCard.test.tsxapp/src/components/chat/ArtifactCard.tsxapp/src/components/chat/ChatFilesPanel.tsxapp/src/components/chat/__tests__/ArtifactCard.test.tsxapp/src/pages/Conversations.tsxapp/src/providers/ChatRuntimeProvider.tsxapp/src/providers/__tests__/ChatRuntimeProvider.artifacts.test.tsxapp/src/services/__tests__/artifactDownloadService.test.tsapp/src/services/__tests__/chatService.artifactEvents.test.tsapp/src/services/artifactDownloadService.tsapp/src/services/chatService.tsapp/src/store/__tests__/chatRuntimeSlice.artifacts.test.tsapp/src/store/chatRuntimeSlice.tssrc/openhuman/artifacts/ops.rssrc/openhuman/artifacts/ops_tests.rssrc/openhuman/artifacts/schemas.rssrc/openhuman/artifacts/store.rssrc/openhuman/artifacts/store_tests.rssrc/openhuman/tools/impl/presentation/mod.rs
Reusing an artifact id in place must not bump created_at to now — that would reorder the artifact to the top of the created_at-sorted list/ panel even though it is the same logical artifact. Read the prior meta's timestamp when REGENERATE_TARGET_ID is set. (CodeRabbit on tinyhumansai#4127)
tinyhumansai#3162) Two review fixes on the artifact export surface: - Un-gate download_artifact_to_downloads to all desktop. It compiles on Windows (directories + tokio::fs::copy), and the Save-As Downloads fallback was previously broken there (missing-command error). (CodeRabbit on tinyhumansai#4127) - Restrict both export commands to sources inside the OpenHuman data dir's artifacts/ tree (canonicalized, with an 'artifacts' path component), so a compromised renderer can't copy an arbitrary local file out through the Save-As dialog. (Codex P2 on tinyhumansai#4127)
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/src-tauri/src/artifact_commands.rs (1)
17-18: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAlign the cancel documentation with the actual contract.
The frontend treats
Ok(None)asCANCELLED/no-op; fallback is only for dialog failures, not user cancellation.Proposed doc fix
-//! unavailable (e.g. no portal on headless Linux) or the user cancels. +//! unavailable (e.g. no portal on headless Linux). +//! User cancellation returns `Ok(None)` and is treated as a no-op.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src-tauri/src/artifact_commands.rs` around lines 17 - 18, The fallback/cancel comment in the artifact dialog docs is inaccurate: the frontend maps Ok(None) to CANCELLED/no-op, and fallback should only describe dialog failures, not user cancellation. Update the documentation comment in the artifact command area to reflect the actual contract around the dialog result handling (especially the artifact command and its frontend fallback behavior), so it clearly distinguishes cancel from unavailable dialog errors.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src-tauri/src/artifact_commands.rs`:
- Around line 109-112: The artifact detection in `source_path` is checking the
absolute `canon_source`, so any parent directory named `artifacts` can
incorrectly satisfy the condition. Update the `artifacts` check to inspect only
the path relative to the OpenHuman root before calling `.components().any(...)`,
using the existing `canon_source`/root path logic in `artifact_commands.rs`.
Adjust the check so only files actually inside the root’s `artifacts` directory
are accepted, and keep the rest of the flow unchanged.
---
Nitpick comments:
In `@app/src-tauri/src/artifact_commands.rs`:
- Around line 17-18: The fallback/cancel comment in the artifact dialog docs is
inaccurate: the frontend maps Ok(None) to CANCELLED/no-op, and fallback should
only describe dialog failures, not user cancellation. Update the documentation
comment in the artifact command area to reflect the actual contract around the
dialog result handling (especially the artifact command and its frontend
fallback behavior), so it clearly distinguishes cancel from unavailable dialog
errors.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 38b89631-9617-4dc8-b05b-e3367d576594
📒 Files selected for processing (4)
app/src-tauri/src/artifact_commands.rsapp/src-tauri/src/lib.rssrc/openhuman/artifacts/store.rssrc/openhuman/artifacts/store_tests.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- src/openhuman/artifacts/store.rs
- src/openhuman/artifacts/store_tests.rs
Collaborator
Author
|
Addressed the outside-diff finding (Save-As fallback broken on Windows) in 6e4562a: |
oxoxDev
added a commit
to oxoxDev/openhuman
that referenced
this pull request
Jun 25, 2026
…3162) assert_artifact_source searched the full absolute path for an 'artifacts' component, so a root (or ancestor) named 'artifacts' would let any file under the root pass. Strip the root prefix first and check the relative path; add a regression test. Also correct the module doc: dialog cancel returns Ok(None) (no-op), it does not trigger the Downloads fallback. (CodeRabbit on tinyhumansai#4127)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
artifact_pendingsocket event (backend shipped in feat(artifacts): publish ArtifactPending event + artifact_pending socket bridge (#3162 backend) #3277) so the chat surface shows a spinner the moment a producer reserves an artifact, then swaps in place to ready/failed.rfdcrate (cross-platform: macOS/Windows/Linux), pre-filled with the artifact filename. The existing Downloads-copy + reveal flow is retained as a fallback.ai_regenerate), reusing the original artifact id so the card swaps in place.Problem
#3017 landed
ArtifactCard+ the Tauri download command + backendArtifactReady/ArtifactFailedevents, but three AC items from #2779 were deferred:tauri-plugin-dialogpullstauri-plugin-fs, which conflicts with the workspace's pinnedschemarsversion. The shipped behaviour copied into~/Downloads/and revealed the file — functional but not the user-chosen-destination AC.ArtifactCardonly rendered once the core publishedArtifactReady/Failed— no spinner during generation, despite the backend now emittingartifact_pending(feat(artifacts): publish ArtifactPending event + artifact_pending socket bridge (#3162 backend) #3277).Solution
AC#1 — in-progress (
artifact_pending). AddedArtifactPendingEvent+ anonArtifactPendingsocket listener (mirrors ready/failed; validates the envelope, no size/path yet), dispatched toupsertArtifactInProgressForThread. Added a no-downgrade guard to that reducer so a late/duplicate pending can never regress areadycard back to a spinner;failed -> in_progressis allowed because that is an explicit retry.AC#3 — Save-As dialog (
rfd). Newsave_artifact_via_dialogTauri command backed byrfd(talks to the OS dialog APIs directly, notauri-plugin-fs→ noschemarsconflict).rfdis added withdefault-features = false+xdg-portalso Linux stays off GTK. The command returns the saved path, ornullon cancel (surfaced asCANCELLED, treated as a no-op). Theartifact_commandsmodule is cross-platform (both export commands compile on macOS/Windows/Linux). Newallow-artifact-savecapability. The service-layersaveArtifactViaDialogfalls back to the Downloads copy when the dialog itself is unavailable (e.g. headless Linux without a portal). Both commands restrict the source to the OpenHuman data dir'sartifacts/tree so the renderer cannot copy an arbitrary local file out.AC#4 — Retry = full re-dispatch (core-persisted args). The producing tool now persists its verbatim args as an
args.jsonsidecar next tometa.json. A newopenhuman.ai_regenerateRPC reloadsmeta+args, rebuilds thePresentationTool+ a freshSecurityPolicy, and re-runs it inside aREGENERATE_TARGET_IDtask-local (socreate_artifactreuses the original id + directory — the card swaps in place) and anAPPROVAL_CHAT_CONTEXTscope (so the fresh pending/ready/failed events route back to the originating thread + socket). Presentation-only today — the one producer that persists args.onRetryis wired at theConversationscall site.Submission Checklist
cargo-llvm-covjobs green; FE Vitest coverage in CI)docs/TEST-COVERAGE-MATRIX.md; this extends the feat(chat): ArtifactCard + Tauri download + backend artifact events (#2779) #3017 artifact surface, no row add/renamerfdis an OS-dialog crate, no network; mock policy unaffected)Closes #NNNin the## RelatedsectionImpact
rfdis a new build dependency (no network, noschemars). Both export commands (Save-As + Downloads) are now cross-platform — theartifact_commandsmodule was previously macOS/Linux-gated.args.jsonis additive — artifacts created before this change simply have no sidecar and surface a clear "not regenerable" error on Retry rather than failing silently.meta.jsonshape is unchanged.ai_regeneratere-runs under the sameSecurityPolicy::from_configthe agent harness uses.ppt-rsengine in place — no LLM round-trip.Related
args.json.AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
feat/3162-artifact-ux-polish6e4562a6aValidation Run
pnpm --filter openhuman-app format:checkpnpm typecheckcargo test --lib openhuman::artifacts(62), tauricargo test -p OpenHuman artifact_commands(12), FE vitest artifact specs (62)cargo fmt --check+cargo check(core)cargo fmt --check+cargo check(app/src-tauri)Validation Blocked
command:pnpm test:coverage/diff-cover(full coverage build)error:not run locally — heavy buildimpact:diff-coverage gate validated in CI insteadBehavior Changes
Parity Contract
meta.jsonshape unchanged; non-regenerate generation still mints a fresh UUID.args.json→ explicit non-regenerable error.Duplicate / Superseded PR Handling