Migrate off OCA.Viewer

[julien-nc]

fixes #648

• Adjust to use @nextcloud/viewer instead of OCA.Viewer
• Fix the OpenAPI specs for the saveOutputFile
• Regenerate the OpenAPI specs
• (unrelated) I found a missing attribute in the response definitions

How to test that?

• Schedule a "generate image" task
• Display it in the assistant
• Click on a result image
• => the image should open in the viewer, just like before

🤖 AI (if applicable)

• The content of this PR was partly or fully generated using AI (N/A)

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 01ee569d-e346-4ac0-a652-d8aaf11763d4

📥 Commits

Reviewing files that changed from the base of the PR and between 2ead20d and 734cee8.

⛔ Files ignored due to path filters (1)

• package-lock.json is excluded by !**/package-lock.json

📒 Files selected for processing (7)

• lib/Controller/AssistantApiController.php
• lib/ResponseDefinitions.php
• lib/Service/AssistantService.php
• openapi.json
• package.json
• src/components/fields/ListOfMediaField.vue
• src/components/fields/MediaField.vue

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

The save-output-file response now includes the saved file’s ID, path, and MIME type. Both media fields use the Nextcloud viewer API to open saved files. The chat message schema adds a required nullable reasoning field.

Priority: ⬆️ High

Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 734ce

The saved-file preview migration and updated response contracts appear mergeable after normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 734ce

No new file-access bypass was identified. The main remaining risk is compatibility: clients may depend on the previous published response shape, and the new preview flow has not been verified at runtime.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The changed preview flow concerns a file copied into the requesting user's files area; the inspected change does not add another service or privileged storage destination.

Trust Boundaries and Controls

• observed — The controller's caller-supplied task and file IDs reach server-side task-owner and output-file membership checks before file resolution. The viewer receives metadata for the saved copy rather than replacing those checks as an authorization control.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Out of Scope Changes check	⚠️ Warning	The reasoning field added to AssistantChatMessage in lib/ResponseDefinitions.php and openapi.json has no demonstrated connection to issue #648 or the viewer migration. The save-output response…	Remove the unrelated reasoning response-definition and OpenAPI changes, or link them to a separate issue.
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 3 files. (4 skipped: 4 …	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Issue #648 requires replacing OCA.Viewer calls with @nextcloud/viewer APIs that use file nodes. Both MediaField.vue and ListOfMediaField.vue now create File nodes with the saved file ID and …
Title check	✅ Passed	The title clearly and concisely describes the primary change: replacing the legacy OCA.Viewer integration.
Description check	✅ Passed	The description accurately covers the viewer migration, OpenAPI updates, response definition change, and testing steps. It is related to the changeset.

Full details: Out of Scope Changes check

Explanation

The reasoning field added to AssistantChatMessage in lib/ResponseDefinitions.php and openapi.json has no demonstrated connection to issue #648 or the viewer migration. The save-output response and MIME changes support the node construction required by the migration.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 3 files. (4 skipped: 4 unsupported.)

• Fix all pre-merge checks with AI

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[skjnldsv]

Huu, have you tried it ?
We haven't merge the server pr yet :)

[julien-nc]

@skjnldsv Yeah I tried it, it works fine. It should not?

[skjnldsv]

Yeah I tried it, it works fine. It should not?

It's supposed to be standalone, but I had doubt regarding the action registration since it server is most likely still registering them twice now 🤔
Oh well, I'll try to merge the viewer pr soon. If you can help with a review, highly appreciated!! 🙇 🤗

[skjnldsv]

Noticed a small thing after it went in: the File we build here doesn't get any permissions, so it ends up as Permission.NONE. It still opens fine, but then the viewer hides download, details, rotate and all that, since they check permissions.

I think the easiest is to just ask WebDAV for the node instead of building it ourselves, that way it's exactly what the server says it is. Pretty much what the old OCA.Viewer.open({ path }) did under the hood:

import { getClient, getDefaultPropfind, getRootPath, resultToNode } from '@nextcloud/files/dav'

const { data } = await getClient().stat(`${getRootPath()}${savedPath}`, { details: true, data: getDefaultPropfind() })
const node = resultToNode(data)
getViewer().open([node], node)

If you'd rather skip the extra request (fair enough, it's one more PROPFIND per click), building it by hand is fine too, it just needs to look like what WebDAV would give us. So at least permissions, and ideally mtime, size and the etag in attributes so previews don't go stale. saveOutputFile could return those next to the fileId and mime it already has.

With the WebDAV route we don't even need the mime in the saveOutputFile response anymore (no harm keeping it though). Want me to open a follow-up?