diff --git a/AGENTS.md b/AGENTS.md index d8d8e5e..efc8da3 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -75,7 +75,7 @@ static linking on FreeBSD (no `openssl-sys` dependency). | Crate | Role | | ----------------------- | ----------------------------------------------------------------------- | -| `colibri-contracts` | Manifest/capability/event schema (golden tests) | +| `colibri-contracts` | Manifest/capability/event schema (fixture tests) | | `colibri-deepseek` | DeepSeek cache-hit probe, prefix metering | | `colibri-runtime` | Host status ingestion, runtime inventory | | `colibri-glasspane` | Agent state machine + event ingestion | @@ -213,7 +213,7 @@ cargo test --workspace cargo clippy --workspace --all-targets -- -D warnings ``` -Golden tests in `crates/colibri-contracts/tests/golden.rs` validate manifest +Fixture tests in `crates/colibri-contracts/tests/fixtures.rs` validate manifest output against reference fixtures. These fixtures are cross-platform — if a Linux-produced manifest differs from FreeBSD 15, investigate and document the cross-platform difference before proceeding. diff --git a/README.md b/README.md index 2a3d3ee..50cb487 100644 --- a/README.md +++ b/README.md @@ -21,7 +21,7 @@ Optional Headroom compression sidecar: `docs/wiki/headroom-sidecar.md`. | ----------------------- | ----------------------------------------------------------------------- | | `colibri` (root) | Workspace root + probe binaries (colibri-probe, runtime-inventory) | | `colibri-mcp` | MCP bridge for editor integration (Zed, Claude Code) via stdio JSON-RPC | -| `colibri-contracts` | JSON schema contracts (golden tests) | +| `colibri-contracts` | JSON schema contracts (fixture tests) | | `colibri-deepseek` | DeepSeek cache-hit probe, prefix metering | | `colibri-runtime` | Host status ingestion, runtime inventory | | `colibri-glasspane` | Agent 5-state machine (zot/pi JSONL events → state) | diff --git a/crates/colibri-contracts/tests/golden.rs b/crates/colibri-contracts/tests/fixtures.rs similarity index 99% rename from crates/colibri-contracts/tests/golden.rs rename to crates/colibri-contracts/tests/fixtures.rs index 4b26e29..6fdee9a 100644 --- a/crates/colibri-contracts/tests/golden.rs +++ b/crates/colibri-contracts/tests/fixtures.rs @@ -1,4 +1,4 @@ -//! Golden tests: the contract structs must accept the real committed manifests +//! Fixture tests: the contract structs must accept the real committed manifests //! (produced by different agents/hosts) and round-trip without data loss. use colibri_contracts::*; diff --git a/crates/colibri-daemon/src/session.rs b/crates/colibri-daemon/src/session.rs index b266736..5dcf16f 100644 --- a/crates/colibri-daemon/src/session.rs +++ b/crates/colibri-daemon/src/session.rs @@ -609,7 +609,7 @@ impl Session { } // --------------------------------------------------------------------------- -// T1.4 golden tests — PromptAssembly + CacheMetrics +// T1.4 fixture tests — PromptAssembly + CacheMetrics // --------------------------------------------------------------------------- #[cfg(test)] diff --git a/docs/guide/architecture/colibri.md b/docs/guide/architecture/colibri.md index ec51f77..bfc08cb 100644 --- a/docs/guide/architecture/colibri.md +++ b/docs/guide/architecture/colibri.md @@ -36,7 +36,7 @@ colibri-daemon — Unix-socket server (the always-on supervisor) | `colibri-skills` | Read-only skills catalog | | `colibri-mcp` | MCP bridge — editor integration + external MCP host | | `colibri-deepseek` | Cache-hit probe + prefix metering for DeepSeek | -| `colibri-contracts` | Manifest/capability/event schemas (golden tests) | +| `colibri-contracts` | Manifest/capability/event schemas (fixture tests) | | `colibri-runtime` | Host status ingestion, runtime inventory | | `clawdie` | Host installer/deployer (ZFS layout, rc.d service) | diff --git a/docs/wiki/contracts.md b/docs/wiki/contracts.md index 6e44226..e27255b 100644 --- a/docs/wiki/contracts.md +++ b/docs/wiki/contracts.md @@ -36,7 +36,7 @@ Schema constants and structs live in `crates/colibri-contracts/src/lib.rs`. ## Golden tests -`crates/colibri-contracts/tests/golden.rs` parses every committed manifest in +`crates/colibri-contracts/tests/fixtures.rs` parses every committed manifest in `manifests/` and asserts round-trip equality. The fixtures are intended to be **cross-platform** — if a manifest produced on Linux differs from one produced on FreeBSD 15, the difference must be understood and documented before it is diff --git a/docs/wiki/index.md b/docs/wiki/index.md index d32b253..7b95ac2 100644 --- a/docs/wiki/index.md +++ b/docs/wiki/index.md @@ -63,7 +63,7 @@ warning. | [layered-soul](./layered-soul.md) | How Colibri consumes the layered-soul reviewed-context repo today vs planned | | [task-board](./task-board.md) | Capability match scoring, cron scheduling, intake drain, SQLite backing | | [quality-gates](./quality-gates.md) | `ci-checks.sh` as the pre-merge gate; why drift reached `main` before | -| [contracts](./contracts.md) | Stable JSON schemas (run-manifest, runtime-inventory, provider-test), golden tests | +| [contracts](./contracts.md) | Stable JSON schemas (run-manifest, runtime-inventory, provider-test), fixture tests | | [store-schema](./store-schema.md) | SQLite coordination schema and migration discipline | | [external-mcp](./external-mcp.md) | MCP bridge for editors + external stdio MCP host; read/write/external-call gates | | [operator-cli](./operator-cli.md) | The `colibri` CLI as a thin typed Unix-socket client over the daemon API | diff --git a/docs/wiki/pull-requests.md b/docs/wiki/pull-requests.md new file mode 100644 index 0000000..a0bae72 --- /dev/null +++ b/docs/wiki/pull-requests.md @@ -0,0 +1,273 @@ +# Pull Requests + +A **pull request (PR)** is how we propose and review changes to the codebase. It's the primary way we collaborate on code - we branch off, make changes, and request those changes be pulled into the main codebase. + +## What is a PR? + +A PR is a proposal to change code. It shows: +- **What changed** - the diff of modified files +- **Why it changed** - commit messages explaining the reasoning +- **How it works** - tests proving it works correctly + +Every code change goes through a PR before being merged to main. Even small fixes. + +## Our Workflow + +### 1. Create a Branch +```bash +# From main branch +git checkout main +git pull origin main +git checkout -b descriptive-branch-name + +# Example branch names: +# - fix/format-drift +# - feat/eval-harness +# - docs/model-selection +# - refactor/remove-legacy +``` + +### 2. Make Changes +```bash +# Edit files, run tests +cargo fmt +cargo clippy +cargo test --workspace + +# Commit with clear messages +git add -A +git commit -m "fix: resolve format drift in daemon.rs + +Ran cargo fmt to fix method chain formatting. +No functional changes - just formatting compliance. + +Sam & Claude" +``` + +### 3. Push Branch +```bash +git push -u origin descriptive-branch-name +``` + +### 4. Create PR + +Go to Forgejo web UI or use the link from the push output: +``` +https://forgejo.smilepowered.org/clawdie/colibri/compare/main...descriptive-branch-name +``` + +Fill out the PR description: +- **What**: What does this PR do? +- **Why**: Why is this change needed? +- **How**: How was it implemented? +- **Testing**: What tests prove it works? + +### 5. Review + +Another agent (or human) reviews the PR: +- Checks that tests pass +- Checks that formatting/clippy are clean +- Reviews the code for correctness +- May request changes + +### 6. Merge + +Once approved: +- Squash merge to main (keeps history clean) +- Or regular merge if commits tell a story worth preserving +- Delete the branch after merge + +### 7. Clean Up +```bash +# Back on main +git checkout main +git pull origin main + +# Delete local branch (optional) +git branch -d descriptive-branch-name +``` + +## When to Create a PR + +**Always create a PR for:** +- Any code changes (even small fixes) +- Refactors +- New features +- Bug fixes +- Documentation updates + +**Never commit directly to main.** Main is protected - you can't anyway. + +## Branch Naming + +Use descriptive, prefix-style names: +- `fix/` - bug fixes (e.g., `fix/format-drift`) +- `feat/` - new features (e.g., `feat/eval-harness`) +- `docs/` - documentation (e.g., `docs/model-selection`) +- `refactor/` - code refactoring (e.g., `refactor/remove-legacy`) +- `test/` - test additions (e.g., `test/integration-eval`) + +## Commit Messages + +Write clear, specific commit messages: + +**Good:** +``` +fix: resolve format drift in daemon.rs + +Ran cargo fmt to fix method chain formatting. +No functional changes - just formatting compliance. + +Sam & Claude +``` + +**Bad:** +``` +fixed stuff +``` + +**Structure:** +1. **Type prefix**: `fix:`, `feat:`, `docs:`, `refactor:`, `test:` +2. **One-line summary**: What changed and why (imperative mood) +3. **Body**: Details if needed (blank line after summary) +4. **Attribution**: `Sam & Claude` for collaborative work + +## Review Checklist + +Before requesting review (or when reviewing): + +- [ ] `cargo fmt` passes (formatting is clean) +- [ ] `cargo clippy` passes (no warnings) +- [ ] `cargo test --workspace` passes (all tests green) +- [ ] `./scripts/wiki-lint` passes (no dead links) +- [ ] Code is readable and well-commented +- [ ] Commit messages are clear and descriptive +- [ ] No unrelated changes in the PR + +## Common Patterns + +### Small Fix PR +One commit, one clear fix: +```bash +git commit -m "fix: resolve cargo fmt drift + +Ran cargo fmt on daemon.rs to fix method chain formatting. + +Sam & Claude" +``` + +### Feature PR +Multiple commits telling a story: +``` +feat: add eval harness for task completion + +Phase 1 implementation: +- task_evals table schema +- write_task_eval/read_task_eval methods +- record_task_completion hook in daemon +- colibri_get_task_eval MCP tool + +Tests: +- 6 new tests covering schema, CRUD, roundtrip +- All existing tests still pass + +Sam & Claude +``` + +### Refactor PR +Rename/reorganize with clear rationale: +``` +refactor: rename golden tests to fixtures + +Replace jargon "golden tests" with clearer "fixtures tests". +Updates all documentation and comments. + +Preserved intentional "golden line" metaphor in tokenomics doc. + +Sam & Claude +``` + +## What Makes a Good PR? + +✅ **Clear purpose**: One focused change, not a grab-bag +✅ **Tests pass**: All gates green before requesting review +✅ **Small scope**: Easier to review, easier to revert if needed +✅ **Clear commits**: Each commit tells part of the story +✅ **Good description**: Explains what, why, and how +✅ **Self-contained**: Doesn't break other things + +❌ **Multiple unrelated changes**: Split into separate PRs +❌ **Failing tests**: Fix tests before requesting review +❌ **Unclear commits**: "fixed stuff" doesn't explain anything +❌ **Huge scope**: Break large changes into smaller PRs +❌ **Missing description**: Reviewer shouldn't have to guess why + +## Merging Strategy + +**Squash merge** when: +- PR has messy commit history +- Commits are interdependent +- Want clean linear history + +**Regular merge** when: +- Each commit tells part of a story +- Commits are independently reviewable +- Want to preserve development history + +**When in doubt, squash.** Main stays cleaner. + +## After Merge + +```bash +# Pull latest main +git checkout main +git pull origin main + +# Delete local branch +git branch -d my-feature-branch + +# Optional: delete remote branch (Forgejo usually does this) +git push origin --delete my-feature-branch +``` + +## Troubleshooting + +### PR has merge conflicts +```bash +git checkout my-feature-branch +git pull origin main +# Resolve conflicts +git add . +git commit -m "merge main into my-feature-branch" +git push +``` + +### Tests fail after push +- Check CI logs +- Run tests locally: `cargo test --workspace` +- Fix issues, push again + +### Reviewer requests changes +- Make requested changes +- Commit with clear message: `fix: address review feedback` +- Push and re-request review + +## Examples from Our Project + +**Recent PRs in colibri:** +- `feat/eval-harness-phase1` - Added evaluation harness (732 lines changed) +- `refactor/remove-legacy` - Removed legacy references (11 files) +- `rename/golden-to-fixtures` - Renamed test terminology (10 files) +- `fix/smoke-to-test` - Renamed smoke tests to unit tests + +These all followed the pattern: branch → commit → push → PR → review → merge → cleanup. + +## Summary + +PRs are how we collaborate on code. They provide: +- **Visibility**: Everyone sees what's changing +- **Review**: Catch bugs before they hit main +- **History**: Clear record of why changes were made +- **Quality**: Gates ensure code meets standards + +Always use PRs. Never commit directly to main. Write clear descriptions. Keep tests green. diff --git a/docs/wiki/runtime-inventory.md b/docs/wiki/runtime-inventory.md index 654c77a..77dd535 100644 --- a/docs/wiki/runtime-inventory.md +++ b/docs/wiki/runtime-inventory.md @@ -77,7 +77,7 @@ socket location without recompiling: ## Golden fixtures -`crates/colibri-contracts/tests/golden.rs` parses committed inventory and +`crates/colibri-contracts/tests/fixtures.rs` parses committed inventory and host-status manifests in `manifests/` and round-trips them through the Rust structs. Those fixtures come from real hosts (`osa`, `domedog`, `debby`, the operator USB) and are treated as cross-platform source material. diff --git a/docs/wiki/sl/contracts.md b/docs/wiki/sl/contracts.md index 0ccf2b7..fbfede6 100644 --- a/docs/wiki/sl/contracts.md +++ b/docs/wiki/sl/contracts.md @@ -39,7 +39,7 @@ Konstante shem in strukture živijo v `crates/colibri-contracts/src/lib.rs`. ## Zlati testi -`crates/colibri-contracts/tests/golden.rs` razčleni vsak potrjen manifest v +`crates/colibri-contracts/tests/fixtures.rs` razčleni vsak potrjen manifest v `manifests/` in preveri enakost povratnega zapisa. Primerki so namenjeni **medplatformni** rabi — če se manifest, ustvarjen na Linuxu, razlikuje od tistega na FreeBSD 15, je treba razliko razumeti in dokumentirati, preden se diff --git a/packaging/freebsd/port/README.md b/packaging/freebsd/port/README.md index 61601ac..7deb793 100644 --- a/packaging/freebsd/port/README.md +++ b/packaging/freebsd/port/README.md @@ -73,7 +73,7 @@ cargo clippy --workspace --all-targets -- -D warnings # ✅ zero warnings cargo test --workspace # ✅ 228 passed, 0 failed ``` -228 tests cover all 13 crates including unit, integration, golden, and +228 tests cover all 13 crates including unit, integration, fixture, and live-socket tests. This confirms the `0.11.0` tree compiles and passes on Linux. FreeBSD runtime validation (`cargo test` on FreeBSD 15) is the remaining step before the first poudriere build.