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
74 changes: 24 additions & 50 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@

Real-time networked music jamming app. Qt/C++ qmake project. Client and server share one codebase; entry point: `src/main.cpp`. Configure `CONFIG` flags in `Jamulus.pro`.

**Make the smallest possible change. One logical change per PR. Never mix refactoring with fixes/features.**
**[`CONTRIBUTING.md`](CONTRIBUTING.md) is the source of truth for everything this project requires of a contribution.** Read it before changing code, and before opening or commenting on an issue, Pull Request or discussion here. This file deliberately does not restate those requirements β€” a second copy drifts, and then two documents disagree. Where they ever do, `CONTRIBUTING.md` wins.

Priority order: Stability > Low latency / real-time safety > Backwards compatibility > Maintainability > New features. This order resolves conflicts only β€” new features are welcome.
What is below is orientation only: where things are, and how to build and run them.

---

Expand All @@ -14,60 +14,34 @@ Linux: `qmake && make` (use `qmake-qt5` on Fedora). Headless server: `qmake "CON

macOS: `qmake QMAKE_APPLE_DEVICE_ARCHS=arm64 QT_ARCH=arm64 -spec macx-xcode Jamulus.pro` (Use `x86_64` on Intel Macs; `macx-clang` if using `make`). Then `xcodebuild build`, and `macdeployqt ./{Debug,Release}/Jamulus.app`.

**Testing:** run headless server (args `-s -n`), connect a client (e.g. via: `-n -c localhost`; may need jackd running on Linux. Run dummy Jack via: `jackd -d dummy`), exercise the change; use the JSON-RPC API (`docs/JSON-RPC.md`) where possible. State what you tested in the PR with evidence. GitHub Actions builds multiple platforms β€” on failure read the failing step's log.
## Run it

## Never Do
A plain build gives one binary that is both client and server. Run the server headless with `-s -n`; connect a client with `-n -c localhost` (on Linux this may need jackd β€” `jackd -d dummy`). A `CONFIG+=serveronly` binary rejects `-c`. Drive it through the JSON-RPC API (`docs/JSON-RPC.md`) where that is possible. GitHub Actions builds several platforms; on failure, read the failing step's log.

**`Never Do` rules are absolute**
What you then owe the reviewer β€” what you tested, and the evidence for it β€” is in [CONTRIBUTING.md Β§ Testing](CONTRIBUTING.md#testing).

- Introduce code that prevents processing of audio within the minimum cycle time for any frame (i.e. worst case must remain viable); DO test this and produce evidence to support the change
- this covers sound process in `src/sound`, network processing in `src/socket.cpp` and mixing in `src/server.cpp`
- potential problems include (but not limited to): memory allocation, file I/O, locks
- where possible, move processing off the real-time thread with queued signals
- Trust values from remote clients β€” validate size/bounds on all network input (malformed input crashes).
- Edit generated files (`moc_*.cpp`, `ui_*.h`, `qrc_*.cpp`, `*.qm`) β€” regenerate; don't edit/reformat third-party code in `libs/`.
- Edit `ChangeLog` directly β€” use a `CHANGELOG:` line in the PR.
## Where the rules are

## Always

- Attach test evidence (logs/output) to the PR β€” never just assert something works.
- Say so if you did not run or verify something.

## Ask first

- Architecture changes (networking/protocol, threading, build system) β€” open an issue to discuss (see `CONTRIBUTING.md`).

## Qt / portability

- Minimum Qt: **5.12.2**. Qt 6 recommended (iOS: Qt 5.15+ required, Qt 6 iOS buggy). Guard newer APIs with `#if QT_VERSION >= QT_VERSION_CHECK(...)`.
- C++11 (C++17 on Android for Oboe).
- Preserve platform support.
- Desktop: Windows 10+, macOS 10.10+, Ubuntu 20.04+/Debian 11+.

## Style (C / C++ / Obj-C++)

- **CI uses clang-format** (version in `.github/workflows/coding-style-check.yml`).
- Run `make clang_format` before committing (works only after qmake).
- CI runs **shellcheck + shfmt** on `.sh` files; **pylint** (config: `.pylintrc`) on `.py` files in `tools/`.
- New contributions: AGPL 3.0+ license header. Pre-3.12.1dev code: GPL 3.0+ (see `CONTRIBUTING.md`).
- Use `tr ( "Hello %1" ).arg ( name )` for user-facing strings β€” never string concatenation.

## JSON-RPC

- Changing RPC methods (e.g. `src/clientrpc.cpp` / `src/serverrpc.cpp`) requires regenerating `docs/JSON-RPC.md` with `tools/generate_json_rpc_docs.py` (CI fails otherwise).
- Requires `--jsonrpcport` + `--jsonrpcsecretfile` at runtime. Binds to localhost by default. Secret requires β‰₯16 characters.

## PR expectations

- One logical change per PR β€” no unrelated cleanup or reformatting of untouched code. Discuss features in an issue before implementing. See `CONTRIBUTING.md`.
- Branch names starting with `autobuild` trigger CI builds on your fork.
- Follow `.github/pull_request_template.md`. Include `CHANGELOG:` line. Add `AUTOBUILD: Please build all targets` for skipped targets (iOS, Windows JACK, Linux armhf/arm64) if touched; see `.github/workflows/autobuild.yml`.
- Builds? Tested? Smallest change possible? Self reviewed against "Priority order" above?
- Disclose AI-generated text at the end of Comments/PRs. (e.g: `> πŸ€– Used AI: <model>, <harness>`) β€” never in code comments.
| Before you… | Read |
|---|---|
| start writing anything at all | [the opening bullets](CONTRIBUTING.md#contributing-to-jamulus) β€” agree the specification first; one logical change per PR |
| resolve a design tradeoff | [general principles](CONTRIBUTING.md#jamulus-projectsource-code-general-principles) β€” stability > low latency and real-time safety > backwards compatibility > maintainability > new features |
| touch `src/sound`, `src/socket.cpp` or `src/server.cpp` | [Real-time safety](CONTRIBUTING.md#real-time-safety) |
| parse anything that arrived over the network | [Input arriving over the network](CONTRIBUTING.md#input-arriving-over-the-network) |
| change an existing protocol message | [Wire compatibility](CONTRIBUTING.md#wire-compatibility) |
| format code, or edit a generated file or `libs/` | [Source code consistency](CONTRIBUTING.md#source-code-consistency) |
| use AI for any part of the work | [Using AI](CONTRIBUTING.md#using-ai) |
| add a file, or copy code in | [Licensing](CONTRIBUTING.md#licensing) |
| use a Qt or C++ feature that may be too new | [Supported platforms](CONTRIBUTING.md#supported-platforms) |
| add a dependency | [Dependencies](CONTRIBUTING.md#dependencies) |
| write user-facing text | [User experience](CONTRIBUTING.md#user-experience) |
| open a Pull Request | [Submitting code](CONTRIBUTING.md#submitting-code-and-getting-started), [Testing](CONTRIBUTING.md#testing), [Ownership](CONTRIBUTING.md#ownership) |
| post a comment or a review | [Commenting and reviewing](CONTRIBUTING.md#commenting-and-reviewing), and `docs/agents/COMMENTING.md` |
| write a `CHANGELOG:` line | [Documentation/Acknowledgements](CONTRIBUTING.md#documentationacknowledgements) |

## Read when relevant
- `CONTRIBUTING.md` β€” process, style, licensing

- `COMPILING.md` β€” full build per platform, CONFIG flags table
- `docs/JAMULUS_PROTOCOL.md` β€” network protocol, packet IDs, ack rules
- `docs/agents/COMMENTING.md` β€” rules when commenting on GitHub
- `docs/agents/COMMENTING.md` β€” commenting on GitHub
- `SECURITY.md` β€” security reporting
53 changes: 49 additions & 4 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,34 @@ We’d really appreciate your support! Please ensure that you understand the fol
- If a [Github issue](https://github.com/jamulussoftware/jamulus/issues) for your feature/bug fix already exists, write a message in that issue indicating that you want to work on it.

- Otherwise, please [post on the GitHub Discussions](https://github.com/jamulussoftware/jamulus/discussions) and say that you are planning to do some coding and explain why. Then we can discuss the specification.
- Please begin coding only after we have agreed on a specification to avoid putting a lot of effort into something that may not be accepted later.
- Please begin coding only after we have agreed on a specification to avoid putting a lot of effort into something that may not be accepted later. Changes to networking or the wire protocol, to threading, or to the build system always need this discussion first.

- Keep commits and Pull Requests focussed on one logical change only. In case you implement multiple features, open multiple smaller PRs instead of large one. Large PRs may become stale since they are not reviewable and be closed after a long time of inactivity.
- Keep commits and Pull Requests focussed on one logical change only, and make the smallest change that does it. Do not mix refactoring with a fix or a feature, and do not reformat untouched code: both hide the change under review. In case you implement multiple features, open multiple smaller PRs instead of large one. Large PRs may become stale since they are not reviewable and be closed after a long time of inactivity.

## Jamulus project/source code general principles

Where these principles pull against each other, resolve the conflict in this order: **stability > low latency and real-time safety > backwards compatibility > maintainability > new features.** This order settles conflicts only - new features are welcome.

### 1. Stability

Instabilities during live performances such as WorldJam are not acceptable. As a result, stability has been, and must continue to be the most important requirement. The following principles are designed to support this.

#### Real-time safety

Do not introduce code that prevents processing of audio within the _minimum_ cycle time for _any_ frame (i.e. worst case must remain viable); DO test this and produce evidence to support the change.

- This covers sound processing in `src/sound`, network processing in `src/socket.cpp` and mixing in `src/server.cpp`.
- Potential problems include (but are not limited to): memory allocation, file I/O, locks.
- Where possible, move processing off the real-time thread with queued signals.

#### Input arriving over the network

Do not trust values sent by remote clients, servers or directories. Validate the size and bounds of everything read from the network before it reaches an array index, a length calculation or an allocation. Malformed input is how a crash gets into a release; see [SECURITY.md](SECURITY.md) for reporting one you find in a released version.

#### Wire compatibility

Clients and servers of different versions have to keep understanding each other, so do not renumber `PROTMESSID_*` and do not change the layout of a protocol message that already exists. Retired message IDs stay reserved - see the `OLD` entries in `src/protocol.h`. Extend the protocol by adding a new message ID.

### 2. [Keep it Simple and Stupid](https://en.wikipedia.org/wiki/KISS_principle) and 3. [Do One Thing and Do It Well](https://en.wikipedia.org/wiki/Unix_philosophy#Do_One_Thing_and_Do_It_Well)

If a feature or function can be accomplished in another way by another system or method, it is preferable not to build that feature into Jamulus. Rather than implementing each and every feature as part of Jamulus, we concentrate on a stable core and implement interfaces for interaction with third-party components as needed. The [JSON-RPC](https://github.com/jamulussoftware/jamulus/blob/main/docs/JSON-RPC.md) API for example, allows you to communicate with the client and server from outside the application.
Expand All @@ -32,6 +50,8 @@ There are several ways to run clang-format:

- By hand: run `clang-format -i <path/to/changed/files>`

Adding a source directory or a new file extension? `make clang_format` and the CI check read different lists, so update them together: `CLANG_FORMAT_SOURCES` in `Jamulus.pro`, and, in [.github/workflows/coding-style-check.yml](.github/workflows/coding-style-check.yml), both the `extensions:` list and the `paths:` filter under `on:` - a missed `paths:` entry means the check silently never runs. Third-party exclusions live in `.clang-format-ignore`.

##### Style definition

Please see the [.clang_format file](https://github.com/jamulussoftware/jamulus/blob/main/.clang-format) in the root folder. In summary:
Expand All @@ -47,10 +67,22 @@ Please install and use [pylint](https://pylint.org/) to scan any Python code.
There is a configuration file that defines some overrides,
and note the [Editorconfig file](.editorconfig) in the project too.

#### Shell scripts
Our CI runs [shellcheck](https://www.shellcheck.net/) and `shfmt` on `.sh` files. Please run both before committing.

#### Files not to edit by hand

- Generated sources - `moc_*.cpp`, `ui_*.h`, `qrc_*.cpp` and `*.qm` - are build products. Change what they are generated from and regenerate.
- Third-party code under `libs/` keeps its upstream formatting. Do not reformat it, and keep any change to it to the minimum needed.
- `docs/JSON-RPC.md` is generated. If you change a JSON-RPC method (for example in `src/clientrpc.cpp` or `src/serverrpc.cpp`), regenerate the document with `tools/generate_json_rpc_docs.py` in the same Pull Request - our CI fails otherwise.
- The `ChangeLog` file - see [Documentation/Acknowledgements](#documentationacknowledgements) below.

### Using AI

Use AI responsibly: If you choose to use AI, judge its outputs and review it for any kind of issues before submitting. Do not answer every comment with AI. Judge if AI use is beneficial for your usecase.

Please disclose AI-generated text at the end of the comment, issue or Pull Request description that carries it, for example `> πŸ€– Used AI: <model>, <harness>`. Do not put such notes in code comments.

AI assisted contributions **must** follow the same standards as every other contribution. The submitter remains the author and is expected to understand and stand behind every submitted line. [AGENTS.md](AGENTS.md) is the entry point into this repository for AI Agents. Low-effort contributions might be closed without comment.

### Licensing
Expand Down Expand Up @@ -84,7 +116,8 @@ _While Android and iOS aren't officially supported, please don't break their bui

Please try to avoid breaking any build by introducing platform-specific code. Check the Github builds all worked before raising a pull request.
Check to see if any newly introduced Qt calls, parameters, properties or constants are available in the minimum supported Qt version, which is currently **5.12.2**. Note that code _style_ in a file may be Qt 4.x. While you should normally stick to existing style, if you make large-scale modifications, updating to Qt 5.12.2 style is recommended.
Maintain C++11 compatibility throughout the code.
Guard any call that needs a newer Qt with `#if QT_VERSION >= QT_VERSION_CHECK(...)`.
Maintain C++11 compatibility throughout the code (the Android build uses C++17 for Oboe).

### Dependencies

Expand All @@ -103,9 +136,14 @@ We're using git to develop Jamulus. To contribute, you should get familiar to gi

Have a look at our [guide for translators](docs/TRANSLATING.md) - especially read the git related part. If you need more in depth information, the [git-scm book](https://git-scm.com/book/en/v2) might also help you getting started. If you have any questions, don't hesitate to ask, as git can be very confusing.

Please fill in the [Pull Request template](.github/pull_request_template.md) - it is the checklist reviewers work from.

Some build targets (iOS, Windows JACK, Linux armhf/arm64) are skipped by default. If your change touches one of them, add the line `AUTOBUILD: Please build all targets` to the Pull Request description.

### Testing

To check that there are no errors, please run a local (build/feature) test. It is good practice to briefly explain what and how you tested your work - for AI contributions specifically, you should provide strong testing and proof. This may include logs, screenshots, small testing scripts or similar depending on your change.
To check that there are no errors, please run a local (build/feature) test. Briefly explain in the Pull Request what and how you tested your work, and attach the evidence - logs, output, screenshots, small testing scripts or similar depending on your change. If you did not run or verify part of your change, say so rather than leaving it implied.
A claim in a code comment, in documentation or in a commit message is a claim too: test it before you write it, or leave it out. A wrong sentence in the tree outlives a wrong sentence in a Pull Request thread, because it is what the next reader trusts.
Keep an eye on the CI checks for quality or compile issues after opening a pull request and fix them as needed. You can also test the build on your repository by naming your branch `autobuild/<branchName>` which will start the building process on your repo.

### Ownership
Expand All @@ -115,6 +153,13 @@ The submitter of an issue or a Pull Request is responsible for its care and feed
Authors are strongly encouraged to update their initial posts/PR descriptions or title to reflect the current state of play, amends, enhancements, outstanding issues, etc., to reduce effort for others in understanding a PR or an Issue.
Admins reserve the right to do this as they see fit.

### Commenting and reviewing

- Test what you can test before you claim it - a build, a log, a run - and cut what you cannot. Words like *presumably*, *should* and *likely* usually mark a sentence that needs a measurement, or needs deleting.
- Comment when you add evidence or an answer the thread does not have yet, in the shortest form that carries it. Let an exchange between others finish, and re-read the thread just before posting - it may have moved while you were writing.
- If a comment turns out to be wrong or incomplete, edit it so that the error leaves the page. Further evidence about the same finding belongs in that comment rather than in a new one.
- Open an issue for a defect you can reproduce, and put the reproduction in the body.

### Documentation/Acknowledgements

The ChangeLog must be updated for each new feature or bug fix. Please include a single-sentence suggestion for that as part of your pull request description after the `CHANGELOG: ` keyword. Do not modify the ChangeLog file as part of your PR as it will lead to conflicts.
Expand Down
17 changes: 12 additions & 5 deletions docs/agents/COMMENTING.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,14 @@
## Posting (comments, issues, discussions)

- Test any claim you can test before posting it β€” a build, a log, a run β€” and cut what you can't; *presumably*, *should*, and *likely* mark sentences that need a measurement or deletion.
- Comment only when you add evidence or an answer the thread lacks, in the shortest form that carries it; wait out an active human exchange, re-read the thread just before posting β€” it may have moved while you drafted β€” and treat a maintainer's stated preference about engagement as the rule.
- If a posted comment proves wrong or incomplete, edit it in place so the error leaves the page; new evidence on the same finding also belongs in the existing comment, not a new one.
- Open a new issue only for a defect you can reproduce, with the reproduction in the body; open a new PR only after a maintainer has agreed the change is wanted; leave starting discussions to humans.
- Disclose AI-generated text
What is expected of anyone posting here is in
[CONTRIBUTING.md Β§ Commenting and reviewing](../../CONTRIBUTING.md#commenting-and-reviewing): test
what you can test before claiming it, comment where you add evidence or an answer the thread does
not have yet, edit a comment in place when it turns out to be wrong, and put the reproduction in
the body of a defect report. Disclosure of AI-generated text is in
[Using AI](../../CONTRIBUTING.md#using-ai). Those apply to an agent exactly as they apply to a
person, so they are not repeated here.

Only these are specific to an agent:

- Leave the starting of new discussions to humans.
- If a maintainer states how they want to be engaged, that is the rule for that person and that thread.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

We aren't kings. Arguing against it is welcome as it could show up misconceptions...

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

πŸ€– AI: The line has a history worth stating. It came in through the Posting section suggested on #3785 as "treat a maintainer's stated preference about engagement as the rule", two days after a maintainer asked on #3898 that the AI not post on that thread. The intent was narrow: an agent must not relitigate a request like that on the same thread. The wording overshot into deference, and moving it up to one of two headline agent rules made that visible. Once disagreement is welcome, the rule is no longer specific to agents, so the proposal is one bullet in CONTRIBUTING.md Β§ Commenting and reviewing, addressed to everyone:

If someone states how they want to be engaged on a thread - for example, no AI-written replies - follow it while it stands. Disagreeing is welcome: say so once, with your reason; a preference can rest on a misunderstanding on either side.

docs/agents/COMMENTING.md then keeps a single agent-only line, leaving the start of new discussions to humans. The change goes into the upstream PR rather than into this one.