doc: codify upstream contribution and review conventions

AGENTS.md

@@ -33,7 +33,7 @@ This repository ships skills under `skills/` (installed at `.agents/skills/`). P
 ## Coding And Testing
 
 - Follow `CODING_GUIDELINES.md` for coding, testing, style, git, and PR conventions.
-- Keep comments minimal: Indico favours self-evident code over explanation. Default to none and let clear names carry the meaning. Add one only for genuinely non-obvious rationale (the why, never the what), and keep it to a single short line. A comment that restates the method, signal, or test name is noise. Signal and public-API docstrings that document a contract are the exception and stay.
+- Keep comments minimal: Indico favours self-evident code over explanation. Default to none and let clear names carry the meaning. Add one only for genuinely non-obvious rationale (the why, never the what). Most fit one short line; a why-comment about an external service's quirk or a fragile invariant may take the lines it needs. A comment that restates the method, signal, or test name is noise. Signal and public-API docstrings that document a contract are the exception and stay.
 - Use test-first development for production code, scripts, and helpers.
 - Prefer existing test patterns in the host repository over inventing new conventions.
 - Do not mock framework internals, ORM sessions, queries, or model classes unless the host repository explicitly instructs otherwise.
@@ -46,6 +46,7 @@ This repository ships skills under `skills/` (installed at `.agents/skills/`). P
 - Use relative Markdown links for files in the same directory.
 - Keep documents ASCII unless a quoted source or code example requires otherwise.
 - Do not add comments or prose that merely restates the heading.
+- Recurring environment or tooling pitfalls belong in the host repository's own instructions (a Known Pitfalls section), next to the commands they affect, not in this shared baseline.
 
 ## Git Workflow
 
@@ -60,10 +61,11 @@ This repository ships skills under `skills/` (installed at `.agents/skills/`). P
 
 Every changed line should trace back to the requested behavior. Avoid drive-by reformatting, renames, or refactors unrelated to the task. Iterating within a session often leaves formatting-only leftovers (a rewrapped line, a moved blank line) after you add and then remove code; revert them. Read your own diff before committing and drop every line that changed for formatting alone.
 
-Two correctness checks that repeatedly matter in review:
+Correctness checks that repeatedly matter in review:
 
-- **Honour feature gates on every surface.** When a setting or toggle enables a feature, each place that exposes it (a list, a dashboard, a search, a permission check) must test the same gate. One surface that skips the check leaks the feature while it is off.
+- **Honour feature gates on every surface.** When a setting or toggle enables a feature, each place that exposes it (a list, a dashboard, a search, a permission check) must test the same gate. One surface that skips the check leaks the feature while it is off. The same applies to declared requirements: a new scope, permission, or capability lands on every surface that lists or checks it (declaration tuples, docs, tests) in the same change.
 - **Keep comments and docstrings truthful.** When behavior changes (a default flips, an attribute or helper is replaced), fix the prose that describes it in the same change. A stale comment is worse than none.
+- **Remove what the change orphaned.** A removed or replaced call site leaves behind unused functions, imports, routes, templates, fixtures, and config keys. Grep every symbol you removed or stopped calling and delete or rewire the leftovers before finalizing.
 
 ## Onboarding A Host Repository
 

CODING_GUIDELINES.md

@@ -47,15 +47,17 @@ object with `MagicMock` or `monkeypatch`.
 - Place tests according to the host repository's existing layout, usually mirroring the production tree.
 - Group related tests in a `class TestX:` when they cover one behavior or public unit.
 - Use `@pytest.mark.parametrize` for case matrices instead of duplicating test bodies.
-- Follow the host repository's naming convention for test files. Prefer `test_*.py` for new Python tests unless
-  extending an existing `*_test.py` pattern.
+- Follow the host repository's naming convention for test files. Indico core and the plugin repositories use the
+  `*_test.py` suffix; use `test_*.py` only where the host repository already does.
 
 ### What To Test
 
 - Behavior exposed by the public API of the module under test.
 - Edge cases reachable from real callers, such as empty input, missing optional fields, duplicate records, and
   constraint violations.
 - Failure modes that production code is expected to handle gracefully.
+- For a new endpoint, the access matrix: each relevant role, including managers of unrelated events, asserted against
+  the expected status code.
 
 Do not test private helpers in isolation when they only factor a public function. Test the public surface and let helper
 coverage come through behavior.
@@ -71,6 +73,13 @@ Write tests that earn their keep:
 - **One focused test per behavior.** Merge near-identical tests that share setup and exercise the same path rather than
   keeping parallel copies.
 
+### Client-Side Tests
+
+- Frontend changes ship a jest spec alongside the component, under `client/js/**/__tests__/*.spec.js` or the host
+  repository's equivalent location.
+- Name `it()` blocks as behavior sentences ("renders the join button when the meeting is live"), not implementation
+  notes.
+
 ## Running Checks
 
 Prefer host repository Makefile targets, task runner commands, or documented scripts over direct tool invocation. They
@@ -89,7 +98,10 @@ checks when practical.
 - Keep imports at the top of the file, grouped by standard library, third-party packages, framework packages, and host
   project packages.
 - Avoid comments that explain what the code plainly does. Reserve comments for non-obvious reasons, invariants,
-  constraints, or workarounds.
+  constraints, or workarounds. A why-comment about an external service's behavior may take several lines when the
+  invariant needs them.
+- Signal and public-API docstrings document the full contract: sender, kwargs, return value, and (for signals) how
+  multiple listener returns combine.
 - Match the surrounding file's formatting, naming, and abstraction level.
 - Prefer editing existing modules over creating new ones.
 - Keep changes surgical. Do not reformat, rename, or refactor adjacent code unless required for the task.
@@ -110,13 +122,28 @@ checks when practical.
 - Stage files explicitly by name. Never use `git add -A`, `git add .`, or `git add -u`.
 - Use single-line commit messages in English: `type: imperative subject`.
 - Keep the subject lowercase after the colon and omit trailing punctuation.
+- Commit style follows the target repository when it differs. The upstream Indico repositories have their own
+  conventions; see `indico/AGENTS.md`.
+- Know the merge strategy before writing fixup commits. In repositories that merge branches unsquashed, every commit
+  subject must stand on its own; in squash-merge repositories, the PR title becomes the final subject.
 - Never add `Co-Authored-By` trailers.
 - Force-push only with explicit approval, and use `--force-with-lease`.
 
 ## PR Conventions
 
 - Write descriptions at the big-picture level: what changed and why it matters.
 - Avoid file-by-file narration, implementation details, version numbers, and CI status in the description.
+- Default to brief. A new feature does not automatically earn headers and sections; add structure only when the reader
+  needs it.
+- Bug-fix descriptions state the root cause, not only the symptom. When behavior changes, a short Before/After pair
+  makes the change reviewable at a glance.
+- When one PR fixes several independent problems, introduce each with a bold category header followed by its
+  explanation.
+- When a design choice was close, add an `## Alternatives considered` section, and flag known catches yourself ("One
+  catch worth flagging:") instead of waiting for review to find them.
+- Cross-link companion PRs (core and plugin, or stacked branches) in both descriptions.
+- Keep the description current: when review changes the scope or approach, update the description in the same push.
+- Changes visible in the UI include a screenshot or short recording.
 - Reply to review comments like a teammate: state the problem, suggest the fix, and keep the thread focused.
 - Put project-specific test instructions, deployment notes, and reviewer context in the host repository PR, not in this
   shared repository.

indico/AGENTS.md

@@ -6,7 +6,7 @@ This directory contains the upstream Indico codebase (https://github.com/indico/
 
 - This directory is a git submodule. Commits made here do not appear in the host repository unless the host repository explicitly bumps the submodule pointer.
 - Do not commit changes inside this directory without an explicit request. Most host-repository tasks should not touch upstream Indico code.
-- When a change must live in Indico itself, treat it as an upstream contribution and follow the conventions documented at https://github.com/indico/indico.
+- When a change must live in Indico itself, treat it as an upstream contribution: follow upstream's CONTRIBUTING.md plus the Contributing Upstream section below.
 
 ## Architecture
 
@@ -45,6 +45,75 @@ This directory contains the upstream Indico codebase (https://github.com/indico/
 - Follow surrounding code style. Indico predates several modern Python idioms in places; match the local file rather than introducing inconsistent modernization.
 - Keep changes surgical. Do not reformat, rename, or refactor adjacent code unless required for the task.
 
+## Contributing Upstream
+
+Conventions for PRs against `indico/indico`, `indico/indico-plugins`, and `indico/indico-plugins-contrib`, learned from upstream review. They override the defaults in `CODING_GUIDELINES.md` where they conflict.
+
+### Commits And Titles
+
+- Core commits use a capitalized plain imperative subject with no type prefix ("Add changelog entry"). Plugin repos prefix the component in sentence case ("VC/Zoom: Fix auto-checkin with multiple regforms").
+- PR titles become merged commit subjects. Core and contrib squash-merge; `indico-plugins` merges branches unsquashed, so every fixup commit subject must stand on its own.
+- A commit body is acceptable upstream when the subject cannot carry the rationale.
+
+### Changelogs
+
+- Every core PR adds a `CHANGES.rst` entry. User-visible changes go under Improvements or Bugfixes; admin- and developer-facing changes go under Internal Changes. Credit format: ``(:pr:`NNNN`, thanks :user:`username`)``.
+- Every plugin PR adds a bullet to the plugin's `README.md` changelog section.
+
+### Database Checklist
+
+- A new relationship into a core model needs the backref plus an entry in that model's alphabetized backref comment list (`User` has one).
+- A second FK to the same model needs `foreign_keys=` on the existing relationship.
+- Columns get `#:` doc comments.
+- Migration PRs carry the `alembic` label. Expect the revision to be rebased at merge time.
+
+### Comments And Docstrings
+
+- Core reviewers remove explanatory comments; the code must be self-evident. Sanctioned prose homes: signal docstrings (the full contract: sender, kwargs, return value, and how multiple listener returns combine), `#:` attribute docs, and the RST docs.
+- The exception: why-comments about external-service quirks and non-obvious invariants. Those take as many lines as the invariant needs and survive review.
+
+### Architecture Expectations
+
+- Prefer the simplest native mechanism (a browser or framework built-in) over a custom layer.
+- Shared endpoint behavior goes into an RH base class parameterized by class attributes, not duplicated per handler.
+- Hoist guard conditions into early returns instead of nesting loops and conditionals.
+- Do not mutate `field.data` inside a `validate_*` method; normalize in `process_formdata()` or `post_validate()`.
+- Invalid configuration raises; it does not warn and continue.
+- Never import plugin code while loading configuration; SQLAlchemy mappers are not set up yet.
+- Reserve an explicit namespace for generated names instead of colliding with user-defined ones.
+- Core additions need a core consumer. A hook or column used only by a plugin gets rejected until core itself uses it.
+- No UI side effects (`flash()` and similar) in service or hook methods: they also run in non-interactive contexts such as event cloning. Log instead.
+
+### Plugins
+
+- Never monkeypatch or add attributes onto core classes from a plugin. Add a real model, a plain relationship, and helpers next to the model.
+- Scope every endpoint to an event, category, or registration form. A global read leaks data across events.
+- Expensive external-API sweeps run in a daily Celery task with a scoped cache (`make_scoped_cache`) and a per-item fallback, not per request, and not behind arbitrary size thresholds.
+- A scope or capability addition lands on every surface at once: the scopes tuple, any legacy tuple, the README list, and a test.
+- Copy plugin boilerplate (`pytest.ini`, packaging basics) verbatim from a sibling plugin. No local-environment workarounds in upstream files; keep those in `.envrc` or similar.
+- Zoom: meetings and webinars are parallel API families. Check every change against both endpoints' documentation and never send a parameter the target endpoint does not document.
+- JS labels use `Translate.string`. Jinja macro call arguments need `_()`; a `{% trans %}` block does not work there. Use the plugin's bound gettext.
+
+### Testing Upstream
+
+- Test files use the `*_test.py` suffix.
+- Side-effect-only fixtures are applied with `@pytest.mark.usefixtures`, never as an unused argument.
+- Shared setup becomes a factory fixture, not a module-level helper taking `db` and plugin arguments.
+- Frontend changes ship jest specs in `client/js/**/__tests__/*.spec.js` with `it()` names that read as behavior sentences.
+- New endpoints get an access-matrix test: each relevant role, including managers of unrelated events, asserted against the expected status code.
+
+### Review Flow
+
+- Gauge maintainer appetite before building anything significant: open an issue or ask first.
+- Open as draft; ping the maintainer when ready for review.
+- Maintainers push commits onto contributor branches. Pull before pushing more work.
+- Resolve conflicts by rebasing, also while the PR is under review.
+- When claiming something works, state how it was verified. UI claims come with a screenshot.
+- Cross-link companion core and plugin PRs in both descriptions.
+- Contrib CI picks the core branch to build against from the core PR referenced in the PR body; reference the core PR whenever the change depends on one.
+- Stacked or cross-fork PRs state the target branch in the description. When the base lands, close and reopen the PR against upstream.
+- Workflows on upstream repos cannot be rerun without admin rights; recover with a new commit or ask a maintainer.
+
 ## Documentation
 
 - Upstream Indico documentation: https://docs.getindico.io/

skills/bump-indico-submodule/SKILL.md

@@ -69,6 +69,8 @@ git commit -m "chore: bump indico submodule to <short_sha>"
 
 The commit message should name the target commit or release tag. Host-repository review needs to see what landed without re-running the same diff.
 
+Every commit in the bump (pointer, plugin enablement, config) keeps the single-line `type: subject` format.
+
 ## When To Hold Off
 
 - The upstream diff contains breaking changes the host has not migrated through yet.