feat: improve dynamic workspace compatibility - #425
Conversation
️✅ There are no secrets present in this pull request anymore.If these secrets were true positive and are still valid, we highly recommend you to revoke them. 🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request. |
Obiente previewNC Native · Obiente updates this comment as the preview changes. |
99198e2 to
edfbc14
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 99198e2c5c
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
4189371 to
9aa79e0
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cc129fd90b
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 96d955970f
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 74cc12cd4a
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: af9d42fda1
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
46b2ee1 to
c2ab45a
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d633f18822
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
When two Nextcloud instances share a host and login name but use case-sensitive base paths such as /Cloud and /cloud, lowercasing the complete server URL maps both accounts to the same cache entry. A preview on the second instance can therefore immediately expose Office editing using the first instance's editor inventory and ETag before its asynchronous capability refresh finishes; normalize only the case-insensitive origin components or key this cache by the canonical local account identity.
AGENTS.md reference: AGENTS.md:L410-L413
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ca2a64fa41
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a03b13350c
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| val structuredDraftSaver = remember(structuredSpecs) { | ||
| nativeRepeatableObjectDraftSaver(structuredSpecs) | ||
| } | ||
| var repeatableObjectValues by rememberSaveable( |
There was a problem hiding this comment.
Keep large repeatable drafts out of saved state
When a repeatable form contains many long values, this rememberSaveable serializes the entire draft into Android's activity state; the custom saver permits up to 256 Ki characters, which can occupy roughly half of the process-wide Binder transaction allowance before the app's other saved state is included, causing TransactionTooLargeException during recreation. Store these drafts in a scoped draft store or impose a substantially smaller UI-enforced saved-state limit.
AGENTS.md reference: AGENTS.md:L245-L248
Useful? React with 👍 / 👎.
| that the embedded browser sees. The representative app manifest covers | ||
| native DAV and API workspaces, plus Nextcloud Office as an embedded-web | ||
| boundary. The optional catalog command can stage every App Store package |
There was a problem hiding this comment.
Describe Office as an external browser handoff
Fresh evidence in the final tree is that NextcloudDocumentPreview.kt now invokes services.openExternalUrl and explicitly tells the user the editor is opening in a browser, while these newly added lines still describe an embedded browser and embedded-web boundary. This leaves the maintained demo guide and manifest contradicting both the implementation and ROADMAP.md, so compatibility testing may validate the wrong integration model; update the guide and surface labels to describe the external Office handoff.
AGENTS.md reference: AGENTS.md:L61-L70
Useful? React with 👍 / 👎.
| LaunchedEffect(activeEditor.sameOriginUrl) { | ||
| runCatchingPreservingCancellation { | ||
| services.openExternalUrl(activeEditor.sameOriginUrl) |
There was a problem hiding this comment.
Gate browser editing for app-only certificates
When an Android account works only because the user approved its self-signed or private-CA certificate inside the app, this handoff still enables Office and creates a one-time editing session before dispatching the URL to the system browser. The approval is stored only in the app's custom OkHttp trust manager, while openExternalUrl uses an external ACTION_VIEW, so a browser that does not separately trust that certificate cannot load the editor; detect this unsupported trust state and explain how to resolve it before issuing the token-producing POST.
AGENTS.md reference: AGENTS.md:L447-L449
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b341bf8462
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return values.takeIf { encoded -> | ||
| encoded.sumOf { value -> value.length } <= MAX_TASK_EDITOR_SAVED_STATE_CHARACTERS |
There was a problem hiding this comment.
Preserve drafts that exceed the saved-state cap
When a user edits or creates a task whose combined fields exceed 32,768 characters, the text fields continue accepting input but this saver returns null; after rotation or activity/process recreation, the editor is restored without the in-progress values and the draft is silently reset. Either enforce the same bound in the editor or move larger drafts to a scoped draft store so accepted input is not lost.
AGENTS.md reference: AGENTS.md:L249-L251
Useful? React with 👍 / 👎.
| val uid = property("UID")?.value?.trim()?.takeIf(String::isNotBlank) | ||
| ?: return@mapNotNull null |
There was a problem hiding this comment.
Reject control characters in task identities
Although blank UIDs are now dropped, the final validation still accepts a nonblank UID containing NUL or another control character and exposes that task for editing. The raw UID is retained by updateGroupwareTaskContent, after which GroupwareDavMutationSpec.toGroupwareDavRequest rejects the calendar because it contains NUL, so every save for that displayed task fails; reject unsafe or oversized UIDs at this parser boundary.
AGENTS.md reference: AGENTS.md:L325-L326
Useful? React with 👍 / 👎.
| fun normalized(): TaskDraft = copy( | ||
| title = title.trim().normalizeGroupwareTextLineEndings(), | ||
| dueDate = dueDate.trim(), | ||
| description = description.trim().normalizeGroupwareTextLineEndings(), |
There was a problem hiding this comment.
Preserve task-description whitespace
When an existing description begins or ends with intentional whitespace, such as blank lines or Markdown-indented content, saving any task edit strips that whitespace here and uploads the altered description. Normalize line endings without trimming the free-form description so unrelated edits do not permanently rewrite its content.
AGENTS.md reference: AGENTS.md:L25-L26
Useful? React with 👍 / 👎.
| if (response.status in 500..599 || response.status in setOf(405, 501)) { | ||
| return loadGroupwareTaskObjectsIndividually(calendarHref, objectReferences, execute) |
There was a problem hiding this comment.
Propagate transient multiget server failures
When a multiget REPORT receives a transient 5xx response such as 503 during maintenance or throttling, this branch treats it like unsupported REPORT and performs an individual GET for every listed object; across successive batches that can turn one failed refresh into thousands of requests instead of backing off. The Contacts loader has the same 5xx fallback, so restrict fan-out to definitive method-unsupported responses or explicitly proven compatibility cases and preserve transient server failures.
AGENTS.md reference: AGENTS.md:L281-L283
Useful? React with 👍 / 👎.
Summary
Document-only Office follow-up at
3aee820fValidation for this revision:
:androidApp:lintDev,:androidApp:assembleDev, andbash tools/check-repository.shpassed on the build host.Earlier Office follow-up at
b341bf84Validation for this revision:
:androidApp:assembleDev,:androidApp:lintDev, andbash tools/check-repository.shpassed.sqliteJniin unrelated media-ledger tests. This follow-up does not claim a full-suite pass or verified end-to-end Office save behavior.Earlier compatibility evidence
dev.obiente.nextcloudnative.devAPK in place on the Android emulator; the credential-encrypted data inode remained unchanged and no write scope remained installedEarlier validation
integration/nextcloud-demo/tests/test-nextcloud-demo.shbash -n tools/nextcloud-demo.shtools/nextcloud-demo.sh validatebash tools/check-repository.shbash tools/test-prerelease-update-contract.shgit diff --checkANDROID_HOME=/opt/android-sdk ./gradlew --no-build-cache --max-workers=4 :ui:desktopTest :contractAcquisition:test :androidApp:testDevUnitTest :ui:lintDebug :androidApp:lintDebug :androidApp:assembleDevadb install -r78af985dScope
This improves the generic dynamic compatibility framework and validates a broad representative set of real apps. It does not claim that every version of every third-party Nextcloud app is already exhaustively covered. Mail write E2E still needs a configured mail account, and the current Office revision still needs maintainer phone/cloud open-save testing across providers. The local demo is stopped at the maintainer's request. No personal-cloud write testing was performed for this revision.