ouroboros/CONTRIBUTING.md
Anton Razzhigaev 88b17fbeba Merge branch 'fix/autoupdate-gate-hardening' into fix/preflight-two-pass
Owner decision Q2=A: the managed-update engine keeps PR #90's semantics
(blocking assisted post-commit gate with auto-rollback, boot retry of a
gate_blocked tx, exact-SHA promotion, Q1=C dirty-stash lifecycle); from
this branch only the two-pass hermetic pytest gate and its containment
machinery are carried forward. Resolution policy:

- Version carriers (VERSION, pyproject, README, api_types.js,
  package.json, ARCHITECTURE header) resolve to the target's 6.87.6 —
  the maintainer assigns versions at release.
- ouroboros/tools/git.py and supervisor/update_merge.py resolve to the
  PR #90 engine wholesale; this branch's parallel gate_blocked/rollback
  machinery (_post_commit_gate_block, _managed_update_gate_rollback,
  UPDATE_TX_GATE_BLOCKED) is dropped. Two of its hardening properties
  are ported INTO the kept engine: _managed_commit_gate_failure now
  re-phases to gate_blocked even when the rollback RAISES (and stops
  claiming the tx is pinned when the re-phase itself fails), and
  mark_update_tx_gate_blocked no longer invents a phantom transaction
  from an absent marker. The managed test-gate record now carries the
  same triad/scope/fingerprint forensics as sibling failure records.
- The two-pass core lands intact: preflight_runner.py (parallel `not
  serial` + serial passes, LANE_EXCLUSION_EXPR SSOT, worker-crash /
  plugin-missing / parallelism-lost hard blocks, per-phase deleted-suite
  baseline), CI env flags, pytest-xdist/pytest-timeout dependencies,
  conftest/marker docs, and the post-commit entry point in git.py drops
  its tests/-existence shortcut exactly like the pre-commit one.
- ProcessContainer and the containment helpers move to a new
  ouroboros/process_containment.py (platform_layer stays the thin OS
  layer and returns under the 1600-line gate); a dead duplicate
  process_group_is_alive that shadowed the fail-closed variant is
  removed, and the Windows Job Object/NtResumeProcess ctypes calls gain
  explicit argtypes/restype (64-bit HANDLE truncation) with the
  INVALID_HANDLE_VALUE comparison fixed to compare values.
- Gate tests are adapted to the kept engine; the gate_blocked boot test
  now pins PR #90's semantics (rollback retry, never promotion).

Co-authored-by: Ouroboros <311266734+ouroboros-agent@users.noreply.github.com>
2026-08-04 18:36:14 +03:00

14 KiB

Contributing to Ouroboros

Thank you for helping Ouroboros evolve. This guide explains how to prepare a pull request that is easy to understand, reproduce, review, and integrate.

The short version:

  1. Base your work on the lowercase ouroboros branch and open the pull request against ouroboros, not main or ouroboros-stable.
  2. Read the project governance and engineering documents before making a substantive change.
  3. Keep the pull request focused, test the changed behavior, and show rendered evidence for visible UI changes.
  4. Do not bump the project version. Maintainers assign the final release version when integrating the pull request.
  5. Optionally attach a current triad + scope review packet to enter the faster review path.

The pull request template in .github/PULL_REQUEST_TEMPLATE.md mirrors this flow.

Read the Project Before Changing It

For substantive changes, read these files in full before designing or editing:

  • BIBLE.md — constitutional principles and design priorities.
  • docs/ARCHITECTURE.md — the current structural and operational map, including the rationale for non-obvious decisions.
  • docs/DEVELOPMENT.md — engineering conventions, module boundaries, testing guidance, and the commit/review protocol.
  • docs/CHECKLISTS.md — the review checklist single source of truth.

Use the existing modules, contracts, and single sources of truth described in those documents before introducing a new mechanism. If a coding agent is doing the work, give it the same instruction explicitly. A useful starting prompt is:

Before editing, read BIBLE.md, docs/ARCHITECTURE.md, docs/DEVELOPMENT.md, and docs/CHECKLISTS.md in full. Follow their current principles and reuse the architecture and existing extension seams instead of inventing parallel mechanisms. Keep the change focused and verify it against the repository's tests and review checklist.

These documents are authoritative descriptions of the current project, not immutable external rules. A pull request may improve BIBLE.md, the architecture, development practices, or review checklists. Explain why the change is needed, what invariant it preserves or intentionally evolves, and keep related code, tests, and documentation consistent. Constitutional changes must also respect the change process and semantic protections stated in BIBLE.md itself.

Choose a Focused Change

Prefer one coherent purpose per pull request. The description should make the following clear:

  • the problem or opportunity;
  • what changed and why this approach fits the existing architecture;
  • what is deliberately out of scope;
  • compatibility, migration, safety, or operational risks;
  • the exact verification performed and its result.

For a broad, ambiguous, security-sensitive, constitutional, or direction-changing proposal, opening an issue or discussion first can avoid expensive rework. Small, well-understood fixes do not need ceremonial design work.

Do not commit local settings, API keys, runtime state, logs, caches, benchmark runs, generated review runs, or build artifacts. Runtime data belongs outside the git repository.

Branch and Pull Request Flow

Keep an upstream remote pointed at the official repository and start from the latest working branch:

git remote add upstream https://github.com/razzant/ouroboros.git  # once
git fetch upstream
git switch -c your-focused-branch upstream/ouroboros

If upstream already exists, verify it instead of adding it again. Before final verification and review evidence, update your branch and resolve any drift:

git fetch upstream
git rebase upstream/ouroboros

Open the pull request with the base branch set to lowercase ouroboros. main is the stable public/update branch, and ouroboros-stable is the QA promotion branch. Neither is the contribution target.

Keep the worktree clean when producing final evidence: commit the intended changes on your pull request branch, then confirm that git status --short is empty. The contributor review command examines a committed base-to-head range; it does not approve uncommitted edits.

Do not bump the version

External contributors should not edit release-only version carriers for a normal pull request: VERSION, the package versions, README badge/history, the architecture version header, or release tags. Ouroboros treats an integrated commit as a release, but maintainers assign the collision-free version during final integration, normally while squash-integrating the pull request. This avoids unrelated version conflicts between concurrent contributions.

You may still update ordinary README or architecture content when the behavior or structure changes; leave only the release number to the maintainer.

Verification

Install the source environment using README.md → Run from Source. The default local suite is documented in README.md → Run Tests:

make test

Run the narrowest relevant tests while developing, then the repository's default local test suite when practical. Record the exact commands and outcomes in the pull request; “tests pass” without a command is not reproducible evidence. Follow the marker and environment guidance in docs/DEVELOPMENT.md for integration, browser, portable, and skill-smoke lanes.

Mark non-parallel-safe tests serial

CI and the maintainers' hermetic commit gate both run the suite as two passes: a parallel -m "not serial" -n auto pass, then a serial pass. So your test runs concurrently with unrelated tests unless you say otherwise.

If a test spawns a real OS process, binds a real port, or mutates a module-level global or registry, mark it @pytest.mark.serial. Everything else must be parallel-safe: use tmp_path rather than fixed /tmp/... paths, monkeypatch.setenv/setattr rather than bare os.environ[...] =, assume no execution order, and restore any module global you touch.

This is not a style preference. An unmarked real-process test does not merely flake — it can kill an xdist worker, which fails that worker's whole co-located batch and surfaces as failures in files you never touched. The commit gate reports a dead worker as a PARALLEL_WORKER_CRASH hard block and never retries it, so the fix is never a re-run.

A dead worker has two causes, and the gate's message tells you which one you have. If the worker crashed, the marker is the fix. If the parallel pass's 300s per-test timeout killed it, the fix is to make that test faster or split it — do not mark a merely-slow test serial, because the serial pass carries no per-test timeout, so the hang would simply move into the one lane that cannot bound it. See docs/DEVELOPMENT.md “The commit gate mirrors the CI split” and item 18 of docs/CHECKLISTS.md.

Add or update tests for changed behavior. If a test cannot be run in your environment, say which one and why rather than marking it as passed.

For a visible UI change:

  1. Open at least one relevant real user flow in an available browser.
  2. Inspect the rendered result, including the states and viewport sizes that matter for the change.
  3. Attach before/after screenshots or equivalent evidence and state what was actually inspected.

A saved screenshot that nobody inspected is not visual verification. Mobile or WebKit evidence is risk-driven rather than mandatory for every UI change.

Optional Fast Path: Triad + Scope Review

A contributor-supplied triad + scope packet is optional, but it materially reduces the work needed to evaluate a pull request. A current, complete packet places the pull request on the faster review path. A pull request without one may still be considered, but Ouroboros and the maintainers must reproduce the review and may need to revise the change manually; that path is slower and the pull request is more likely to be deferred when maintainer capacity is limited.

Neither a clean packet nor its absence decides acceptance. A passing review is evidence, not a promise to merge, and maintainers may rerun it or request additional changes.

If the pull request changes the review substrate itself — for example the review script, reviewer configuration, review checklists, or production review code — its contributor packet is diagnostic only. A maintainer must rerun the review from a trusted target-base implementation before integration.

What the contributor review runs

The checked-in scripts/run_external_review.py contributor mode runs the production triad and scope review over the committed range from the target base to your branch head. It deliberately:

  • resolves the reviewer models and reasoning efforts from the shipped defaults in the target ouroboros revision, not from values modified by the pull request;
  • routes every triad and scope reviewer through OpenRouter;
  • applies the clean blocking contract for the contributor packet;
  • does not run the Claude advisory pre-review and does not require an Anthropic key;
  • records the exact base/head binding and full reviewer evidence without committing or modifying the reviewed branch.

Triad completeness follows Ouroboros's production quorum contract together with an authoritative scope verdict; it does not require every configured actor to respond. The packet preserves actor statuses and degraded reasons, so any failed, partial, or unparsable actor must still be disclosed even when the remaining actors satisfy quorum.

Do not override the reviewer models, scope-review model, reasoning efforts, or provider. The value of this packet is that every contributor uses the review configuration selected by the current target version of Ouroboros.

Produce a packet

First fetch and rebase on the current target, commit the final change, and confirm the worktree is clean. Configure your own OPENROUTER_API_KEY without placing it in a command argument, tracked file, shell history, or attachment. Also set TOTAL_BUDGET to a positive finite USD ceiling that you explicitly authorize for this run; contributor mode fails before model calls when that ceiling is missing or invalid. The review calls may incur OpenRouter charges, so use a key with sufficient remaining provider balance and check current pricing before choosing the ceiling. For example:

export TOTAL_BUDGET="<authorized USD ceiling>"

Then run from the repository root:

python scripts/run_external_review.py \
  --contributor \
  --base-ref upstream/ouroboros \
  --head-ref HEAD \
  "<PR title>" \
  --goal "<goal>" \
  --scope "<scope>"

The script creates:

  • review-evidence.json — machine-readable base/head, configuration, actor, verdict, and outcome evidence;
  • full-output.txt — full human-readable triad and scope output;
  • review-packet.zip — the attachment-ready packet.

Use the fresh output directory printed by the command. Review packets are operator artifacts: attach them to the pull request, but do not add them to the git diff.

Attach and summarize the evidence

In the pull request's Review evidence section:

  1. Attach review-packet.zip and, when useful for quick inspection, review-evidence.json and full-output.txt.
  2. Record the reviewed base SHA and head SHA.
  3. Record the exact command/profile and the resolved reviewer model IDs.
  4. Summarize every triad actor verdict, the aggregate triad result, and the scope verdict.
  5. Disclose known advisory findings, accepted trade-offs, infrastructure failures, skipped coverage, or other non-clean conditions. Do not present a failed or incomplete run as a pass.

The evidence is valid only for the recorded head and base. Rerun it after any code change, rebase, conflict resolution, or other change to the reviewed range. Do not cherry-pick only favorable actor outputs or substitute a screenshot for the raw records.

Protect secrets and private data

Treat review output as public before uploading it. Attach only the generated packet, not the review drive, local observability store, settings.json, or an environment dump. Inspect the packet for API keys, authorization headers, credentials, private paths, or unrelated local data even when automatic redaction reports success.

If redaction is necessary, preserve all verdicts and finding structure and state exactly what was removed. Never upload a secret and never commit a credential in order to make review reproducible; revoke any credential that was exposed.

Pull Request Checklist

Before marking the pull request ready for review, confirm that:

  • the base branch is ouroboros and the branch is current with its intended base revision;
  • the pull request has one coherent purpose with explicit non-goals;
  • the description explains the architectural fit and important trade-offs;
  • relevant tests pass, with exact commands and outcomes recorded;
  • behavior and architecture documentation are updated where necessary;
  • visible UI changes include inspected rendered-flow evidence;
  • no version or release-only carrier was bumped by the contributor;
  • no secrets, local state, logs, caches, generated runs, or build artifacts are present in the diff;
  • triad + scope evidence is attached and current, or the reason it was not run is stated honestly;
  • limitations, unresolved findings, and follow-up work are disclosed.

Disclosure of coding-agent assistance is optional. If you include it, name the agent/model, what it changed, and what a human or independent process verified. Agent use is context, not a positive or negative quality signal.

What Happens After Submission

Maintainers may reproduce tests, rerun triad + scope review, ask for a smaller diff, request changes, or integrate the contribution with additional fixes and the final release-version update. Review evidence accelerates that process but does not replace maintainer judgment or Ouroboros's own governance gates.

Contributions are licensed under the repository's LICENSE. By submitting a contribution, you confirm that you have the right to provide it under those terms.