mirror of https://github.com/J3vb/OwnCord.git synced 2026-09-03 03:50:00 +03:00

Files

T

J3vbandClaude 70473e8e10 B1-2: truth, entry points, and contributor path (#1412 )

* fix(hooks): guard on the command the hook actually runs

pre-commit probed one binary and invoked another. The protocol block
guarded on `command -v go` and then ran `make protocol-verify`; the sqlc
block guarded on `command -v sqlc` and ran `make sqlc-verify`. `make` is
not on PATH on a stock Windows box, so a contributor with Go installed
but no make had their commit rejected with

    pre-commit: FAIL: protocol constants are stale — run 'make
    protocol-generate' in Server/ and stage the result

when nothing had been generated and nothing compared. The real cause was
`make: not found`, and the advice the message gives fails the same way.

Rather than add a `command -v make` guard, inline what the two Makefile
targets reduce to — `sqlc generate` / `go run ./scripts/genprotocol`
followed by `git diff --exit-code`. Same semantics, one less prerequisite,
and it doubles as the make-free equivalent B1-2 asks for. The Makefile
targets stay for anyone who prefers them.

Also: the protocol block was the only one with no `else`, so a
contributor without Go got no check and no notice. It now warns like its
two siblings. And gofmt is a separate binary from go, so it is probed
separately.

Verified both directions with the hook body replayed verbatim:
- Go present, make absent -> passes, no staleness claimed.
- schema edited without regenerating -> fails, as it must.

Refs RL-20 / L-14.

* docs: state one branch and PR model

Active documents contradicted each other head-on. README.md and
docs/contributing.md said branch from `dev` and target `dev`; CLAUDE.md
said branch from `main`, PR to `main`. That is R-02, and the 2026-08-19
audit had already recorded it as D-06 without it being resolved.

`dev` is the answer, and the repository already behaves that way: B0 made
`dev` PR-only with ten required checks enforced on admins, and #1409,
#1410 and #1411 all landed there. `main` carries releases.

docs/contributing.md becomes the single source of truth. It now states the
model, what protection is actually applied, and the two consequences a
contributor meets on their first PR — that a self-mergeable PR still cannot
merge red, and that Docker and Tauri Full Build report as skipped against
`dev` rather than failing. Everywhere else summarises and links here.

- CLAUDE.md: corrected, with a link rather than a second copy.
- CONTRIBUTING.md: new. GitHub's contributing-guidelines affordance only
  resolves the root, .github/ or docs/ — `docs/contributing.md` is not a
  path it finds, so the link never appeared on issues or PRs.
- PULL_REQUEST_TEMPLATE.md: names the base branch, which it did not.
- bughunt-run skill: reviewed the branch against `origin/main`, which is
  the wrong base once every PR targets `dev`.

README.md already said `dev` and is left as the short summary it should be.
Dated audits and the historical remediation plan keep their `main`-era
wording — they are records.

Refs R-02.

* fix(hooks): pick the pre-push base from the nearest integration branch

pre-push decided which side's gates to run from
`git diff --name-only origin/main...HEAD`. That was right when everything
targeted `main`. Once `dev` became the integration branch it stopped being
right: a branch cut from `dev` diffed against `main` counts everything on
`dev` and not yet on `main` as "changed".

Measured on this branch: the old base reported 609 changed files, the new
one reports 6. So in practice the hook was running the full server build
matrix and the client typecheck plus eslint on every push, whatever the
change touched — the file-based narrowing it exists for never engaged.

Now it picks whichever of origin/dev, origin/main is nearest, by commits
between merge-base and HEAD, skipping a candidate that scores 0. Verified:
a branch cut from dev picks origin/dev (2 ahead); dev itself scores 0
against dev and picks origin/main (8 ahead), which is what a dev -> main
release PR wants. With no candidate resolvable it falls back to the
existing `__all__`, so an unfetched or shallow clone still runs everything.

Refs RL-20 / L-14, R-02.

* chore(node): one Node source of truth

`.nvmrc` and all ten `actions/setup-node` pins said 24; five active
documents and the repo's only `engines` block still said 20. A contributor
following the docs installed a version CI does not run.

Node 24 wins — it is what CI already runs. Every manifest now declares
`engines`, and `engine-strict=true` turns a wrong major into a failed
install rather than an `EBADENGINE` warning nobody reads. `>=24` rather
than `^24` so a Node 26 box keeps working; `Client/.nvmrc` stays the
human-facing pin and the docs point at it instead of restating a number.

The `.npmrc` is per package root, not one at the top. npm reads the
project `.npmrc` from the package directory and does not walk parents —
verified with a throwaway package requiring node >=99: with only a parent
`.npmrc` npm warned and exited 0; with one in the package directory it
failed `notsup`. A single root file would have left `Client/`, the package
that matters most, on warnings.

Five docs, not the four previously identified — `docs/mcp-introspect.md`
also said 20. And `docs/contributing.md` claimed "`.nvmrc` + CI both say
Node 20", which was wrong about both.

Verified both directions in all three package roots: Node 22 fails
`notsup`; Node 24 installs clean and `npm ci` passes in Client/.

Refs RL-17 / C-01, ENV-01.

* docs: add the documentation landing page

`docs/` had 24 top-level files and no index. The root README carried a
flat list of 22 links that had drifted: six documents were reachable from
nowhere at all — including both 2026-08-23 audits and the test audit — and
two entries were labelled "latest" while newer unlinked audits existed.

docs/README.md is the index RL-12 asked for. It groups by what a document
*is*, because that is what decides whether to trust it: guidance tells you
how to do something, reference describes a contract the code implements,
audits are dated snapshots nobody updates, plans record intent. Every
tracked file under docs/ now appears exactly once, and the audit table says
plainly that audit-2026-08-19.md still claims "0 open findings" when the
ledger has 38.

The root README keeps a short curated list and defers to the index, rather
than maintaining a second copy that drifts again. Two fixes while there:
`docs/plans/` was linked as a bare directory, unlike its two sibling
directory entries, and was annotated "each carries a verified status
header" — which docs/plans/README.md:7-9 explicitly contradicts, since a
plan's header is exactly the thing that drifts and the index is the
authority.

Verified: 78 relative links across the new and edited files resolve, and
no tracked docs/ file is unreachable from the index.

Refs RL-12 / R-06.

* feat(scripts): root command facade

Entry points existed only inside Server/ (a Makefile) and Client/ (npm
scripts). Nothing at the root told a new contributor where to start, and
the root package.json had three scripts, none of which built or tested
anything.

`npm run check` from the root now runs what CI gates on, and
check:server / check:client / check:rust run one stack. scripts/run.mjs
is dependency-free Node — the shape render-ledger.mjs already uses — so
`npm run check` works before `npm install` has.

Cross-platform by construction: every step is spawned with an explicit cwd
and no shell, so there is nothing to quote and no `cd &&` to behave
differently on Windows. npm and npx get their .cmd suffix there. No step
shells out to make.

The facade orchestrates; it is not a new required path. Each step prints
the command and the directory before running it, and those are exactly the
commands documented per-stack — so a server contributor can read the output
and type them instead, and still never needs Node. Tools CI installs but a
contributor may not have (golangci-lint, which has no wrapper in this repo
at all; sqlc, pinned by Server/sqlc.version) are skipped with a printed
reason rather than failing.

Three corrections to the ci-check skill while aligning it:

- `make sqlc-verify protocol-verify` replaced by what those targets reduce
  to, so the documented path does not require make either.
- `cargo test` -> `cargo test --lib`, which is what ci.yml actually runs.
- "NODE_OPTIONS=--no-experimental-webstorage is mandatory on Node 22+" was
  false. tests/setup.ts installs the shim, CI runs Node 24 without the
  flag, and the suite was measured passing without it — 192 files / 5257
  tests, identical to the flagged run.

Also documents the third RL-20 problem, which needed no code: core.hooksPath
is exclusive, so `npm run hooks:install` silently disables any
.git/hooks/post-commit — including the one `graphify hook install` writes,
which CLAUDE.md tells agents to install. Nothing warned about that.

Verified: check:client 5257/192 green, check:rust 123 tests + clippy green,
--list prints every command, and the optional-tool skip path reports rather
than fails.

Refs RL-04 / L-04, RL-20 / L-14.

* feat(ci): fail on a document that contradicts the findings ledger

G-04's remaining half. The ledger is the source of truth for defect counts,
but nothing stopped a planning document from stating a different number and
nothing noticed when one did. `render-ledger.mjs --check` cannot help: it
validates the JSON schema and returns before rendering, so it never reads
FINDINGS.md and cannot see drift at all — and no workflow ran it anyway.

scripts/check-doc-counts.mjs counts ledger statuses and compares them to
what an allow-list of active documents claims, failing with file, line,
claimed value and actual. Wired into ci.yml as a job with no npm ci, since
the script imports nothing outside node:, and into the facade as
`npm run check:docs` — first in `check`, so a contradicted count does not
wait behind ten minutes of -race.

The patterns are narrow on purpose. A first attempt matched any
"<number> <status>" and flagged nineteen things, all false: "the 45 open P1
rows" (issue-register rows, not ledger findings), "All 8 findings F1-F8"
(a different register), "G-05 **refuted**" (an identifier), `">=20"` and
`CGO_ENABLED=0` (not counts at all). A check that cries wolf gets ignored,
which is the failure G-04 already describes. So a number is only read as a
claim in three shapes that cannot mean anything else: an enumeration of two
or more "<n> <status>" pairs, a status table row in a table that totals
itself, and "<n> records/findings" where the ledger is named within three
lines. Fifteen selftest assertions pin both directions, and the job runs
them before it runs the check.

It reads findings-ledger.json directly rather than importing
render-ledger.mjs for `validate`/`render`: that module ends in a bare
top-level `await main()` with no import.meta.main guard, so importing it
rewrites FINDINGS.md as a side effect.

Dated docs/audit-*.md are reported, never failed — they are snapshots
nobody maintains. audit-2026-08-19.md does claim zero open findings against
38 open, so b0-baseline's "No plan was found claiming '0 open findings'"
holds for docs/plans/ but not for docs/.

Not included: a real FINDINGS.md render-drift check. That is RL-07 and
belongs with the generated-artifact work, not here.

Verified: 27 claims across 9 documents agree; corrupting one count in
docs/plans/README.md fails the check naming that line, for both the status
and the total.

Refs G-04.

---------

Co-authored-by: Claude <noreply@anthropic.com>

2026-08-26 10:39:24 +02:00

11 KiB

Raw Blame History

Contributing

How to set up the development environment and contribute to OwnCord.

Development Setup

Prerequisites

Platform	Server	Client
Windows 10+ x64	✅	✅
Linux x64	✅	✅
Linux ARM64	✅	✅ (CI only)

Go 1.26+ (server)
Node.js 24+ (client) — pinned in Client/.nvmrc; engine-strict makes a wrong major a hard failure, not a warning
Rust / Cargo (Tauri client — not needed for server-only work)
Docker + Compose v2 (optional — alternative to building the server locally)

Available Commands

Root facade — one entry point

From the repository root. These orchestrate the per-stack commands below; they are a convenience, not a replacement. Nothing here needs make, and everything works the same on Windows, macOS and Linux.

Command	Description
`npm run bootstrap`	`npm ci` in all three package roots
`npm run check`	Everything CI gates on: server, client, Rust
`npm run check:server`	Server only — build variants, vet, race, deadlock, lint, generated-output drift
`npm run check:client`	Client only — typecheck, lint, format, unit + integration tests
`npm run check:rust`	Tauri backend — `cargo test --lib` and clippy
`npm run check:docs`	Fail if a watched document states a finding count the ledger contradicts
`npm run format`	Prettier over the client, `gofmt -w` over the server
`npm run generate`	Regenerate protocol constants and the sqlc query layer
`npm run release:preflight`	`check` plus a client production build
`node scripts/run.mjs --list`	Print the exact command every task runs, and where

Tools CI installs but you may not have — golangci-lint, sqlc — are skipped with a printed reason rather than failing the run.

Working on the server only? You never need Node. The facade prints each command it runs and the directory it runs it in; those are the commands in the next section, and using them directly is equally correct.

Server (Go)

Command	Description
`go build -o chatserver.exe -ldflags "-s -w" .`	Build server binary (Windows)
`CGO_ENABLED=0 go build -o chatserver -ldflags "-s -w" .`	Build server binary (Linux)
`go build -tags otel .`	Build with OpenTelemetry SDK (requires `go get` first — see Phase B)
`go build -tags wazero .`	Build with Wazero plugin runtime (requires `go get` first — see Phase C)
`go test ./...`	Run all server tests
`go test ./... -cover`	Run server tests with coverage
`go test -race ./...`	Run server tests with race detection

Make targets (run from Server/):

Command	Description
`make test`	Run the test suite the way CI does (`-race`, 20 min timeout)
`make test-deadlock`	Run the deadlock-detection pass CI also runs (`-tags deadlock`)
`make cover`	Per-package coverage (what CI uploads) + a function summary
`make cover-all`	Cross-package coverage — the honest number (also lists 0.0% functions)
`make sqlc-install`	Install the pinned sqlc version into `$GOBIN`
`make sqlc-generate`	Regenerate the type-safe Go query layer (`db/dbgen/`, SQLite engine)
`make sqlc-verify`	Fail if the committed `dbgen` output is stale (used by CI)
`make protocol-generate`	Regenerate the WS message-type constants (Go + TS) from `docs/protocol-schema.json`
`make protocol-verify`	Fail if the committed protocol constants are stale (used by CI)
`make otel-up`	Start Jaeger (traces) + Prometheus (metrics) via Docker for local OTel development
`make otel-down`	Stop and remove the OTel dev containers

Client (Tauri v2)

Build & dev

Command	Description
`npm run dev`	Start Vite dev server with hot reload
`npm run build`	TypeScript check + Vite production build
`npm run tauri dev`	Launch Tauri app in dev mode
`npm run tauri build`	Build release installer (NSIS on Windows, AppImage+deb on Linux)

Tests

Command	Description
`npm test`	Run all tests (vitest)
`npm run test:unit`	Unit tests only
`npm run test:integration`	Integration tests only
`npm run test:e2e`	Playwright E2E (mocked Tauri)
`npm run test:e2e:native`	Playwright E2E (real Tauri exe + CDP)
`npm run test:e2e:prod`	Playwright E2E (prod build)
`npm run test:e2e:ui`	Playwright UI mode
`npm run test:watch`	Vitest watch mode
`npm run test:coverage`	Coverage report
`npm run test:mutate`	Stryker mutation testing
`npm run test:mutate:dry`	Stryker dry-run (no mutations applied)
`npm run test:browser`	Vitest browser-mode tests

Type checking, linting & formatting

Command	Description
`npm run typecheck`	Full typecheck (all sources)
`npm run typecheck:build`	Typecheck build config only
`npm run lint`	oxlint + ESLint check (src/)
`npm run lint:fix`	ESLint auto-fix
`npm run lint:ox`	oxlint only (fast correctness checks)
`npm run format`	Prettier format (src/ + tests/)
`npm run format:check`	Prettier check only (no writes)
`npm run knip`	Dead code and unused export detection

Git hooks (recommended)

Committed hooks in .githooks/ catch the most common CI failures locally. Enable once per clone (from the repo root):

npm run hooks:install    # = git config core.hooksPath .githooks

Hook	What it runs
`pre-commit`	gofmt + `go vet` (when Go files staged), oxlint + prettier + `tsc --noEmit` (when client TS staged), `sqlc-verify` / `protocol-verify` (when their inputs staged)
`pre-push`	Server build in all build-tag variants, client typecheck + type-aware ESLint. Set `OWNCORD_PREPUSH_TESTS=1` to also run `go test -race ./...`

Bypass with --no-verify or OWNCORD_SKIP_HOOKS=1 when needed — CI still enforces everything.

Neither hook needs make, and neither needs Node for the Go checks.

core.hooksPath is exclusive. Once set, Git resolves every hook against .githooks/ and never looks in .git/hooks/ again. .githooks/ holds only pre-commit and pre-push, so hooks:install silently disables any post-commit you installed there — graphify hook install writes one. Nothing warns you. Put it at .githooks/post-commit instead (untracked, so it stays yours), or skip hooks:install and use npm run check before pushing.

Plugin Development

Plugins are WASM modules loaded at runtime when the server is built with -tags wazero. See Server/plugin/examples/hello/README.md for the full plugin ABI and build instructions.

Toolchain requirements for building .wasm plugins with TinyGo:

Tool	Version	Notes
TinyGo	0.40.1	Supports Go 1.19–1.25 only
Go SDK	1.25.x	Install alongside the system Go via `go install golang.org/dl/go1.25.3@latest && go1.25.3 download`
wasm-opt	Binaryen 129	Required by TinyGo for the `wasi` target; download from Binaryen GitHub releases

Any WASM toolchain (Rust/wasm32-wasi, AssemblyScript, etc.) that exports the five ABI functions is equally valid — TinyGo is just the example toolchain used by examples/hello/.

Branch and PR model

This section is the single source of truth for the branch model. Everywhere else -- the root README.md, CLAUDE.md, the PR template -- summarises it and links here rather than restating it.

dev -- the integration branch. All contributions target dev.
main -- releases only. dev is merged to main for a release, and release tags are cut from main.

dev is protected and PR-only: direct pushes are rejected, ten status checks are required, required_approving_review_count is 0, and the rule is enforced on admins. So a PR is self-mergeable once CI is green, but no commit reaches dev without CI having run on it. Settings and rationale live in docs/plans/b0-dev-branch-protection.sh.

Two consequences worth knowing before you open a PR:

The Docker and Tauri Full Build jobs are gated on main and report as skipped on a PR into dev. That is expected, not a failure.
Squash merge, and a conventional commit subject on the squashed commit.

Branch Naming

feature/<name> -- new features
fix/<name> -- bug fixes
docs/<name> -- documentation changes

Commit Format

Use conventional commits:

feat: add thread support to channels
fix: prevent duplicate WebSocket connections
refactor: extract permission checks into middleware
docs: update quick-start guide
test: add integration tests for invite flow
chore: bump Go dependencies
perf: cache role permissions in memory
ci: add lint step to GitHub Actions

Pull Request Process

See Branch and PR model above for what to branch from and target.

Branch from dev
Open the PR against dev
All ten required checks must pass -- dev is protected, so a red PR cannot merge
Request code review
Squash merge, conventional commit subject

Testing

The client suite enforces 70% coverage thresholds in vitest.config.ts; the Go suite has deliberately no floor (T-2026-07-25-19) — use make cover-all to see the honest cross-package number. Follow a test-driven workflow and never lower a threshold to make a change fit.

Code Style

TypeScript: See Client Architecture
Go: gofmt + golangci-lint, standard library preferred
Rust: cargo fmt + cargo clippy, minimal code (native APIs only)

Dependency Policy

The policy behind what the lockfiles already enforce (decided 2026-08-05, closing audit findings 2026-04-07 #8 / DC-11):

Lockfiles are authoritative. package-lock.json, go.sum and Cargo.lock pin every transitive dependency; CI installs only from them (npm ci, module/registry verification — never a bare npm install in CI or hooks). package.json keeps ordinary caret ranges: exact-pinning it would duplicate what the lockfile does while making every security patch a manual edit.
Upgrades arrive as reviewed PRs, not ambient drift. Dependabot runs weekly per ecosystem (.github/dependabot.yml) with semver-major updates ignored across the board — majors are adopted deliberately, by a human, reading the changelog. Peer-coupled groups (vitest/@vitest/*, @stryker-mutator/*) update as one PR so exact peer pins cannot wedge.
Security gates run on every PR: npm audit --omit=dev --audit-level=high (shipped deps only — dev-tooling advisories are triaged in the workflow comment instead of blocking on unfixable pins), govulncheck for Go, cargo audit for Rust, and knip refuses unused client dependencies outright.
Version skew is pinned at the toolchain level too: Client/.nvmrc, every actions/setup-node in CI, and an engines block in all three package.json files say Node 24 — with engine-strict=true in each package's .npmrc, so a wrong major fails the install instead of warning. Server/sqlc.version pins sqlc, Go pins via go.mod (GOTOOLCHAIN=auto), and GitHub Actions are SHA-pinned with Dependabot bumping the pins.

11 KiB Raw Blame History Unescape Escape