Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 12 additions & 8 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -103,14 +103,7 @@ Avoid creating source files that implement multiple types; instead, place each t

### Documentation comments

Documentation must be trustworthy: only write a comment you can back with the implementation in front of you. **A wrong or overreaching comment is worse than none** - when a behaviour is unclear or you cannot state it with confidence, leave it undocumented. This is the same verifiability rule that governs commit messages and PR text, applied to code comments.

- **Style.** Use Doxygen `/** @brief ... */` blocks for types and their members - the established convention across this codebase. Use a trailing `//!< ...` for a single instance variable or field, and plain `//` for free helper functions and file-local statics.
- **Document the type and each declared member.** For a type, state what it is and how it behaves. For each public property, method or protocol callback, state what it does. Add `@param` entries only where the meaning is not obvious from the name - in particular, spell out what `nil`, `NO`, an empty string or `0` does, and which event triggers a callback block.
- **Do not overreach.** Describe what the code actually does, not what it looks like it should do. Avoid absolute claims the implementation does not guarantee - for example, do not write "keeps the window on screen" for a helper that only best-effort clamps and can still overflow, or "loads a remote URL" for one that only reads local files.
- **Overload sets.** When a type has many overloaded initializers or methods that funnel into one, fully document the designated one (with its `@param` list) and simply mark the rest as convenience overloads rather than repeating the text.
- **Comment members selectively.** Document instance variables and file-local statics whose purpose is not obvious from their name and type; leave self-explanatory ones (a backing `_stack`, a counter) uncommented to avoid noise. In Objective-C(++), instance variables live in the `@implementation` block, so their comments belong there, not in the header.
- **Verify before you trust it.** Re-read the implementation and confirm every comment is literally true before considering the work done.
Documentation comments must be accurate and written for human readers. Follow [`doc/terminology.md`](doc/terminology.md) for vocabulary and [`doc/writing-style.md`](doc/writing-style.md) for comment structure, concision, language-specific conventions, and verification. A wrong or overreaching comment is worse than none. AI-generated comments must be edited to meet the same standard.

## Commit and Pull Request Guidelines

Expand Down Expand Up @@ -227,6 +220,17 @@ These instructions are restricted to `./shell_integration/MacOSX/NextcloudIntegr

### Tests

#### QML and sharing UI workflow

Changes to a QML sharing component must be validated in four stages, in this order:

1. Run the narrowest automated test and verify the component's state, properties, and supported interactions. For QtQuick behavior, use QtQuick Test; use C++ tests for component construction and C++ integration seams only.
2. Open the smallest standalone QML harness that instantiates the changed component, rather than launching the complete desktop client. Supply a deterministic mocked backend and representative data for each relevant state.
3. Observe the harness visually, capturing a screenshot when layout or interaction is relevant. Check that every expected control, state transition, and action is present and usable with the mocked backend; a passing build or test is not visual proof.
4. Report the observed result and any remaining discrepancy. If the harness does not match the expected UX, continue debugging and repeat the relevant stages before reporting the change as complete.

When a failure is only reproducible in the full application, retain the isolated harness as far as possible and collect the desktop client's application log for the full-app reproduction. Do not claim a UI issue is fixed from static validation alone.

- **Mandatory coverage for features and bugfixes.** Every feature or bugfix implemented by an AI agent must ship with corresponding automated tests in the same change. Bugfixes require a regression test for the original failure mode; features require tests for the new behavior and relevant boundary and failure cases. Tests must exercise behavior through a supported public or testable interface rather than merely increasing line coverage.
- **Testability is part of implementation.** Before changing production code, locate the relevant test target and its existing fixtures, mocks, and helpers. Prefer designs that allow deterministic isolation and reuse existing test infrastructure; if the code is not testable, make the smallest focused production change needed to establish an appropriate test seam.
- **Validation is required.** Run the smallest existing test command that covers the changed behavior, then broaden validation when the targeted test exposes integration or build issues. A feature or bugfix is incomplete if its tests are absent, unrelated, not executed, or failing without an explicitly documented blocker.
Expand Down
126 changes: 126 additions & 0 deletions doc/terminology.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
<!--
- SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
- SPDX-License-Identifier: GPL-2.0-or-later
-->

# Codebase terminology

Use these terms consistently in comments and documentation. The client has two related but different engines, so vocabulary is scoped below:

- The **standard sync engine** is the C++ `SyncEngine`/`Folder` path, including its VFS integrations.
Comment thread
i2h3 marked this conversation as resolved.
- The **File Provider engine** is the macOS Swift extension built on Apple's File Provider framework.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not an extension of Swift but macOS (and a particular kind thereof) which could have also been done in Objective-C (which the file provider framework actually is written in, hence the APIs aren't that "swifty" or fit for strict concurrency).

Suggested change
- The **File Provider engine** is the macOS Swift extension built on Apple's File Provider framework.
- The **File Provider engine** is the macOS file provider extension built on Apple's File Provider framework written in Swift.


Do not automatically carry a term from one engine into the other. The framework's terms are often the right choice for File Provider code, even when the standard sync engine has a similar concept with a different name.

## State and lifetime

| Term | Use it for | Do not use it for |
| --- | --- | --- |
| **persisted** | Data saved in Realm, settings, a journal, or a file. | Data held only in memory. |
| **cached** | A copy that can be stale, discarded, or rebuilt. | The authoritative record. |
| **runtime-only** | State that is not saved across the relevant boundary. | State saved for recovery. |
| **retained** | An object, reference, or resource kept alive. | A field copied through a merge; use **preserved**. |
| **preserved** | A value copied into an updated record without being overwritten. | Object ownership or lifetime. |
| **survives** | A value or object that remains after a named event. | An unqualified claim about persistence. |

When the lifecycle guarantee matters, describe it separately from storage: “persisted in Realm and survives an extension restart” states both facts clearly.

## State, results, and configuration

| Term | Use it for | Do not use it for |
| --- | --- | --- |
| **state** | The current condition of an object, account, or operation. | A requested setting; use **policy** or **configuration**. |
| **status** | The result or progress of an operation, especially a transfer. | Every kind of state. Use the exact enum name when one exists. |
Comment on lines +32 to +33

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a difficult linguistic distinction even for humans, in my perception. Also, there is an overlap. status includes the progress of an operating which already is covered semantically by state as "the current condition".

Suggested change
| **state** | The current condition of an object, account, or operation. | A requested setting; use **policy** or **configuration**. |
| **status** | The result or progress of an operation, especially a transfer. | Every kind of state. Use the exact enum name when one exists. |
| **state** | The current condition of an object, account, or operation. | A requested setting; use **policy** or **configuration**. |
| **result** | The result of an operation, especially a transfer. | Every kind of state. Use the exact enum name when one exists. |

| **mode** | A selected implementation or operating configuration, such as a VFS mode. | A temporary result or transfer status. |

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is a VFS mode?

| **policy** | A rule or preference that controls what should happen, such as pinning or keeping a file downloaded. | Proof that the requested result has already happened. |
| **configuration** | Values that select or set up how a component operates. | A live operation result. |
| **error** | A failure or failure result. | A conflict or warning unless the code treats it as an error. |
| **conflict** | A specific sync outcome where changes cannot be applied together automatically. | A general failure. |

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
| **conflict** | A specific sync outcome where changes cannot be applied together automatically. | A general failure. |
| **conflict** | A specific sync outcome where diverging changes between different content states cannot be reconciled automatically or without data loss. | A general failure. |


Use the exact enum or property name when the distinction matters. For example, File Provider transfer status and the standard engine's sync result status are different concepts even though both use the word “status.”

## Standard sync lifecycle

| Term | Use it for |
| --- | --- |
| **sync run** | One complete standard-engine cycle for a folder. |
| **discovery** | Reading local and remote state and building the items that need action. |
| **local discovery** | Reading the local filesystem or local database during discovery. |
| **remote discovery** | Reading the server state during discovery. |
| **sync item** | One file or folder operation represented in a sync run. |
| **propagation** | Applying accepted sync items through operations such as upload, download, move, or delete. |
| **reconcile** | Comparing incoming state with existing state and deciding how to combine or resolve it. |

Use **enumeration** for File Provider requests. Do not use it as a general replacement for standard-engine **discovery**.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this redundant and misplaced in this section?


## Standard sync engine and VFS

| Term | Use it for | Do not use it for |
| --- | --- | --- |
| **virtual file** | A VFS-managed local file representation whose contents may not be available locally. | The file's contents themselves. |
| **placeholder** | The filesystem representation of a virtual file. | The download operation itself. |
| **hydrated** | A placeholder whose contents are available locally. | A file that is merely pinned. |
| **dehydrated** | A placeholder whose contents are not available locally. | A file that has been deleted. |
| **hydration** | Downloading the contents of a virtual file. | Changing its pin state. |
| **pin state** | The VFS availability policy, represented by `PinState` and related APIs. | The current hydration state. |

In this engine, a placeholder can be hydrated or dehydrated. Hydration is the download operation; pinning is the user's availability preference. They are related, but one does not imply the other.

## File Provider engine

File Provider uses its own vocabulary. Prefer these terms in the Swift package and extension rather than translating them into standard-sync terminology:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
File Provider uses its own vocabulary. Prefer these terms in the Swift package and extension rather than translating them into standard-sync terminology:
The File Provider framework uses its own vocabulary. Prefer these terms in the Swift package and extension rather than translating them into standard-sync terminology:


| Term | Use it for |
| --- | --- |
| **domain** | A registered File Provider account scope. |
| **container** | An enumeration scope within a domain, such as root, working set, or trash. |

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
| **container** | An enumeration scope within a domain, such as root, working set, or trash. |
| **container** | An enumeration scope within a domain, such as "root", "working set" or "trash" or a specific folder. |

| **item** | A File Provider object identified by an item identifier. Use **file** or **folder** when its type matters. |
| **enumeration** | A File Provider request that lists items or reports changes. |

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
| **enumeration** | A File Provider request that lists items or reports changes. |
| **enumeration** | A process that lists items reports changes to such on the local device or server. |

| **enumerator** | The object handling an enumeration request. |
| **change batch** | A group of item changes delivered in one enumeration response. |

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
| **change batch** | A group of item changes delivered in one enumeration response. |
| **change batch** | A group of item changes delivered in one enumeration response to the File Provider framework. |

| **materialized** | An item in File Provider's local materialized set. This can include a downloaded file or a visited directory. |

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
| **materialized** | An item in File Provider's local materialized set. This can include a downloaded file or a visited directory. |
| **materialized** | An item in File Provider domain's local materialized set. Folders are materialized once their content was enumerated. Files are materialized once their actual content is present on the local device. |

| **downloaded** | The local file-content flag used for a File Provider file. |

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
| **downloaded** | The local file-content flag used for a File Provider file. |
| **downloaded** | The local file-content flag used for a File Provider item. Maps semantically to the materialized state of the File Provider framework. |

| **visitedDirectory** | A directory that has been enumerated locally; it does not mean the directory was downloaded. |

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
| **visitedDirectory** | A directory that has been enumerated locally; it does not mean the directory was downloaded. |
| **visitedDirectory** | A directory that has been enumerated locally; it does not implicate any of its descendants was materialized or enumerated. |

| **dataless** | A File Provider item with no local materialized content, typically after eviction. |

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
| **dataless** | A File Provider item with no local materialized content, typically after eviction. |
| **dataless** | A File Provider item without actual content but metadata available locally. |

| **evict** | Removing an item's local File Provider representation without treating it as a server deletion. |

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
| **evict** | Removing an item's local File Provider representation without treating it as a server deletion. |
| **evict** | Remove the local content copy of a file provider item without actually deleting the item on the server and retain the existing metadata. |

| **keepDownloaded** | The stored intent behind “Always keep downloaded”; it is not the current downloaded or materialized state. |

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
| **keepDownloaded** | The stored intent behind “Always keep downloaded”; it is not the current downloaded or materialized state. |
| **keepDownloaded** | The stored content policy behind the “Always keep downloaded” custom file provider action; it is not a state detail. |
| **content policy** | Defines how the File Provider framework is expected to handle local content copies of a file provider item. Defines policy for automatic eviction by the framework but also whether content should be downloaded from the server automatically and eagerly or local content copies evicted on case of server-side changes to the represented item. |

| **anchor** | The framework value passed between enumeration requests to identify where enumeration should continue. |
| **cursor** | The position within a stored change list or delivery session. |

An anchor may identify the container's change position or a pending batch. The specific anchor formats belong in the change-enumeration documentation, not in the general vocabulary list.

Use **materialized** in new prose. Preserve exact existing identifiers, including `MaterializedEnumerationObserver`.

The terms do not map one-to-one between engines:

| Standard sync engine | File Provider engine | Notes |
| --- | --- | --- |
| virtual file / placeholder | File Provider item | Both are local representations, but `item` is the framework object. |

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
| virtual file / placeholder | File Provider item | Both are local representations, but `item` is the framework object. |
| virtual file / placeholder | File Provider item | Both are local representations, but `item` is the actual framework object regardless off its state (either "dataless" or "materialized"), not a placeholder. |

| hydrated / dehydrated | materialized / dataless | These describe related local-content states, but are not interchangeable API terms. |
| `PinState` | `keepDownloaded` / content policy | Both express availability intent, but belong to different implementations. |
| hydration | File Provider materialization or download | Use the term exposed by the code path being discussed. |

## Paths and deletion

| Term | Use it for |
| --- | --- |
| **local filesystem** | Files and directories on the computer. |
| **local database** | Client-side stored metadata or state. |
| **remote server** | Data and operations on the Nextcloud server. |
| **directory** | A filesystem or path structure. |
| **folder** | A user-facing or sync-folder concept. |
| **delete** | A deletion operation. |
| **soft-deleted** | A record marked as deleted but not yet removed. |
| **evict** | A provider or operating-system eviction operation. |

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
| **evict** | A provider or operating-system eviction operation. |
| **evict** | A File Provider framework eviction operation. |

| **trash** | The client or server's deleted-item area and its related operations. Describe permanent removal directly when needed. |

## Data representations

| Term | Use it for | Do not use it for |
| --- | --- | --- |
| **record** | A stored database object, such as a sync-journal or Realm record. | An arbitrary in-memory object. |
| **row** | A specific SQL or table row. | Every database record. |
| **metadata** | Stored descriptive data about an item or operation. Qualify it when the source matters. | The item or file itself. |
| **properties** | Values exposed by a platform or API object, such as a File Provider item. | Stored database data unless the API calls it properties. |
| **attributes** | Filesystem or platform attributes, such as size, dates, or permissions. | Sync metadata in general. |
| **snapshot** | A copied view of values used after the live object may have changed. | The live object or database record. |
87 changes: 87 additions & 0 deletions doc/writing-style.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
<!--
- SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
- SPDX-License-Identifier: GPL-2.0-or-later
-->

# Writing style

This guide applies to comments, documentation, commit messages, and other technical writing in the repository. Write for the next human reader: make the point quickly, use ordinary words, and include only the context needed to understand the code.

Use [`doc/terminology.md`](terminology.md) when naming recurring concepts. It defines the shared vocabulary and calls out terms that differ between the standard sync engine and the File Provider engine.

## Keep it concise

- One sentence is the default for a code comment.
- Use two or three sentences only when they explain a real constraint, lifecycle boundary, or external API behavior.
- If the explanation needs a paragraph, put it in a design or user-facing document and link to it from the code.
- Remove repetition. Do not restate the function name, the next line of code, or a fact the type system already makes obvious.
- Prefer one clear point over a complete history of the implementation.

Prefer:

> Persist the change-delivery session in Realm so a new enumerator can drain the remaining batch after the current enumerator is invalidated.

Avoid:

> This state-management mechanism ensures robust continuation behavior across enumerator lifecycle transitions and related asynchronous callbacks.

For a small implementation detail, shorter is better:

> Keep `keepDownloaded` when replacing server metadata.

## Use plain, direct language

- Start with the point. Put the important action or constraint first.
- Prefer active voice: “Save the batch in Realm,” not “The batch is persisted by the session manager.”
- Name the concrete object, operation, and boundary when they matter.
- Prefer familiar verbs such as “save,” “read,” “send,” “remove,” and “keep.”
- Avoid abstract noun chains such as “state transition handling behavior.”
- Remove filler such as “in order to,” “as such,” “it should be noted,” “ensures that,” “leverages,” and “robustly.”
- Do not use impressive-sounding wording that you would not use when explaining the code to a colleague.

## Explain the reason or contract

Comments should explain why the code is necessary or describe a contract that is not obvious from the code. They should not narrate each statement.

Good reasons include:

- an ordering requirement;
- a lifecycle boundary;
- a platform or framework rule;
- a value that must survive an update;
- a failure case that looks surprising; or
- a deliberate compatibility behavior.

If the behavior is unclear enough that it cannot be described in one or two simple sentences, investigate the implementation before writing the comment. If it still cannot be stated confidently, leave the comment out or move the explanation into a design document.

## C++ and Objective-C++ comments

- Use Doxygen `/** @brief ... */` blocks for types and their members, trailing `//!< ...` for a single instance variable or field, and plain `//` for free helper functions and file-local statics.
- Document each type and each public member when its purpose or behavior is not obvious.
- For public properties, methods, and protocol callbacks, state what they do and what event triggers a callback block.
- Add `@param` entries only when the parameter's behavior is not obvious from its name, especially for `nil`, `NO`, an empty string, or `0`.
- When overloaded methods funnel into one designated implementation, document the designated method fully and identify the others as convenience overloads.
- Comment instance variables and file-local statics selectively. Leave self-explanatory fields, counters, and backing storage uncommented.
- In Objective-C++, document instance variables in the `@implementation` block rather than the header.

## Swift comments

- Use `///` documentation comments for public Swift types and members when documentation is needed.
- Use `//` for short implementation comments and `// MARK: -` for meaningful sections.
- Document the behavior and important contract of a public API, including callback or delegate conditions when they are not obvious.
- Do not use Doxygen tags or C++ comment conventions in Swift files.

## Check accuracy

Read the implementation before trusting a comment. Describe what the code actually guarantees, not what it appears intended to do.

Avoid absolute claims when the implementation is best effort. For example, do not say that a helper “keeps the window on screen” if it only clamps its position, or that it “loads a remote URL” if it only reads a local file.

AI-generated comments must be edited to the same standard as human-written comments. If a comment sounds like a paragraph of technical language, shorten it until a human can understand its point on the first read.

## Spelling and formatting

- Choose one spelling for new prose and use it consistently within the document. Preserve exact public API, platform, and framework names.
- Do not mass-rename existing symbols or comments as part of an unrelated documentation change.
- Use the terminology chosen in [`doc/terminology.md`](terminology.md), including the engine-specific terms.
- Keep normal Markdown paragraphs and list items as single logical lines so editors can wrap them naturally. Tables may use one row per line.
Loading