diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 42273602..0f934e3e 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -1,15 +1,56 @@ -## Summary +## Intent and scope - + -## Test plan +Intent and resulting behavior: - +Affected contracts and consumers: -- [ ] `pnpm validate` -- [ ] `pnpm test:fast` -- [ ] Manually tested in the app (describe the flow) +## Verification evidence -## Related issues +Exact checked commit SHA: - +Node and pnpm versions: + + + +| Command or check | Outcome and evidence | +| --- | --- | +| `pnpm validate` | | +| Closest changed-surface check | | + +Regression evidence: + + + +Unverified cases and skipped checks, with reasons: + +## Attribution + +Accountable contributor: + +Authoring tool/configuration, if known (otherwise `unknown`): + +Session reference, if known (otherwise `unknown`): + + + +## Corrective change + + + +Original finding link: + +Introducing PR or commit, with causal evidence (or `unknown`): + +Reproducible failing scenario: + +Review/check gap that let it escape: + +Prevention test or check and its result: + +## Related issue + + + +Closes #N diff --git a/doc/maintenance/2026-10-14-weekly-dry-run/README.md b/doc/maintenance/2026-10-14-weekly-dry-run/README.md new file mode 100644 index 00000000..ed6c79eb --- /dev/null +++ b/doc/maintenance/2026-10-14-weekly-dry-run/README.md @@ -0,0 +1,153 @@ +# Weekly maintenance review, 2026-10-14 + +Dry run of the process in [review-process.md](../review-process.md), produced +against this worktree's checked head. It exercises the report format and records +real check output. It is not a scheduled run; no recurring job exists. + +Owner: unassigned. The owner field is defined in +[review-process.md](../review-process.md#cadence-and-owner). This dry run was +produced by an implementing agent, and assigning the recurring owner is a +maintainer decision tracked in [#199](https://github.com/bravew/Neumar/issues/199). +Reviewer: implementing agent for #230. +Head: `d182f23f27100a18066d5804f2ec4f774f41dd98` (`issue/230-weekly-maintenance-review`, +rebased onto `origin/epic/194-maintenance-process`). +Node and pnpm: Node v26.11.1, pnpm 11.22.0. The Node baseline CI uses is 22; see +Unverified. +Previous report: none. This is the first run, so every trend is a baseline. + +## Comparability with the earlier review + +The earlier review was measured at `c0926618`. This head differs from it only in +tracked documentation: `git diff --stat c0926618 HEAD -- src src-api packages` +is empty, so the source, API, and shared-package trees are byte-identical. The +warning counts, allowlist entries, suppression counts, orphan candidates, and +migration files below are therefore directly comparable to the baseline. No +product behavior changed between the two heads. + +## Check outcomes + +Every command below was run in this worktree on the head above. The exit code and +counts are the real output. + +| Command | Result | Notes | +| --- | --- | --- | +| `pnpm validate` | Pass, exit 0 | Emitted 453 frontend lint warnings and 0 lint errors. All sub-checks passed: brand, critique-i18n, locale parity, design-system flags, design media catalog, uuid routing, ag-ui version (0.0.59 lockstep), plugin registry (226 entries), HyperFrames skill, FFmpeg skill (2.4.2, 42 tools), HyperFrames upgrade check, frontend and API typecheck, root and API format, component size (897 files scanned, 68 allowlisted). The HyperFrames upgrade check reports an available update 0.8.80 to 0.8.155 and changed no files. | +| `pnpm test:fast` | Fail, exit 1 | Frontend: 399 files, 1776 tests, all passed. API: 2 failed, 593 passed, 3 skipped files; 2 failed, 3859 passed, 7 skipped tests. Both failures are detailed below. | +| `pnpm --filter neumar-api lint` | Pass, exit 0 | 10 warnings, 0 errors. | +| `pnpm --filter neumar-api exec tsc -p tsconfig.json --noEmit` | Pass, exit 0 | Includes API test files. | +| `pnpm lint` (frontend) | Pass, exit 0 | 453 warnings, 0 errors. | +| `cargo check --manifest-path src-tauri/Cargo.toml --locked` | Fail, exit 101 | Failed in the `neumar` build script because the sidecar resource `src-api/dist/neumar-api-aarch64-apple-darwin` does not exist. This is a missing build artifact, not a source defect; see Unverified. | + +### `pnpm test:fast` failures + +1. `test/unit/agent-runtimes/resolve.test.ts` line 251, "resolves + npm-installed OpenCode through the shared extended PATH". This is the known + test-isolation failure tracked in [#195](https://github.com/bravew/Neumar/issues/195) + and named in [LESSONS.md](../../../LESSONS.md). The resolver finds the host's + Homebrew OpenCode before the temporary npm fixture. It reproduces on this host + because Homebrew OpenCode is installed. +2. `test/unit/plugins/loader.test.ts`, "invalidates loader generation when + watched skill files change". The failure is `Error: Hook timed out in + 30000ms` in the file's `afterEach`, which tears down the plugin hot-reload + watcher. Re-running the file alone passed: 7 tests passed in 22.00s. This is a + load-sensitive timing failure under the full parallel run, not a reproduction + in isolation. It is a new observation with no tracking issue, and it is + selected for a bounded cleanup pull request below. It is not the `#195` + failure and should not be reported as one. + +## Findings + +Severity follows the earlier review: P0 urgent containment, P1 fix before +expanding the affected feature, P2 planned cleanup. A finding carried from the +earlier review names its source review and commit. + +| ID | Surface | Finding | Evidence | Owner | Status | +| --- | --- | --- | --- | --- | --- | +| W1 | New warnings and suppressions | The frontend warning baseline is unchanged at 453, but 28 `eslint-disable-next-line` directives exist under `src/`, of which 25 suppress `react-hooks/exhaustive-deps`. API code adds 71 more, and 5 `@ts-expect-error` directives exist. No new suppression was introduced in this window. | `pnpm lint`, `grep -rn eslint-disable-next-line src/` | unassigned | Open, carried from the 2026-10-13 review | +| C1 | Size ceilings | The allowlist still holds 68 entries and no ceiling changed in this window. Nine entries are obsolete: the file is at or below the 350-line default, so the allowance is unnecessary. | `scripts/component-size-allowlist.json`; `node scripts/check-component-size.mjs` | unassigned | Open, carried from the 2026-10-13 review; tracked in [#217](https://github.com/bravew/Neumar/issues/217) | +| O1 | Orphan candidates | Four API modules named in the earlier review remain present with no importer found by exact-path search: `extensions/mcp/sandbox-server.ts`, `core/config/hierarchy.ts`, `shared/services/pipeline/cli-dispatch.ts`, `shared/services/video-analysis.ts`. This run confirmed the absence of importers, not their removal. | `grep -rn` for each path under `src-api/src` | unassigned | Open, carried from the 2026-10-13 review; tracked in [#218](https://github.com/bravew/Neumar/issues/218) | +| U1 | Unsupported surfaces | The chat plugin apply route returns 501, and the memory extract and consolidate routes return 501. All three messages are truthful and direct callers elsewhere. Both `/task/:taskId` and `/task-v2/:taskId` remain mounted, and both settings shells remain live. No change in this window. | `src-api/src/app/api/plugins.ts:330`, `src-api/src/app/api/memory.ts:497`, `:524`, `src/app/router.tsx:57,65` | unassigned | Open, carried from the 2026-10-13 review; tracked in [#223](https://github.com/bravew/Neumar/issues/223) | +| M1 | Migrations | The migrations directory holds 60 files: 58 numbered migrations plus `runner.ts` and `utils.ts`. None changed in this window. The migration-drift finding from the earlier review is unchanged and still needs the full-registry run against fresh, current, and drifted snapshots. | `git diff c0926618 HEAD -- src-api/src/shared/db` is empty | unassigned | Open, carried from the 2026-10-13 review; tracked in [#225](https://github.com/bravew/Neumar/issues/225) | +| S1 | Security | The P0 daemon binding finding is unchanged and unfixed. The default listener still omits `hostname`, and privileged routes are reachable without JWT unless `WEBUI_AUTH=true`. | `src-api/src/http-daemon.ts:478`, `src-api/src/app/middleware/jwt.ts:12`, `src-api/src/app/api/sandbox.ts:179` | unassigned | Open, carried from the 2026-10-13 review; tracked in [#200](https://github.com/bravew/Neumar/issues/200) | +| S2-S9 | Security | The remaining security findings S2 through S9 are unchanged. The source trees are identical to the reviewed baseline, so none has been fixed. | [01-findings.md](../2026-10-13-regression/01-findings.md) | unassigned | Open, carried from the 2026-10-13 review; tracked in [#201](https://github.com/bravew/Neumar/issues/201) to [#207](https://github.com/bravew/Neumar/issues/207) | +| R1-R6 | Unresolved regressions | All six regressions from the earlier review are unchanged. R6 (the `resolve.test.ts` isolation bug) reproduced in this run. | `pnpm test:fast`; [01-findings.md](../2026-10-13-regression/01-findings.md) | unassigned | Open, carried from the 2026-10-13 review; tracked in [#211](https://github.com/bravew/Neumar/issues/211) to [#216](https://github.com/bravew/Neumar/issues/216) and [#195](https://github.com/bravew/Neumar/issues/195) | +| T1 | Unresolved regression (new) | The plugin loader test times out under the full parallel run and passes alone. This is a load-sensitive timing assumption a user meets on a busy machine, and it is the class [LESSONS.md](../../../LESSONS.md) calls out. | `pnpm test:fast` (2 failed), isolated `pnpm test:api test/unit/plugins/loader.test.ts` (7 passed, 22.00s) | unassigned | New, observed in this run | +| E1 | Expired exceptions | No exception registry exists yet. Exceptions are named in the earlier review's findings, but none carries an owner, a reason, and an expiry in a machine-readable place, so none can be listed as expired. | [review-process.md](../review-process.md#review-scope) surface 8 | unassigned | New, observed in this run | + +## Metrics + +The previous column is the value the 2026-10-13 review recorded, carried from +[README.md](../2026-10-13-regression/README.md) and +[03-lessons-and-contributors.md](../2026-10-13-regression/03-lessons-and-contributors.md). +This run is the baseline for the metrics the earlier review did not record. + +| Metric | This run | Previous | Trend | +| --- | ---: | ---: | --- | +| Escaped regressions introduced since the last report | 1 (T1, new) | 6 (R1 to R6) | Baseline. R1 to R6 predate this run and are not counted again. | +| Time to repair, escaped regressions closed since the last report | Not applicable, none closed | R6 introduced `83c425bb`, still open | Baseline. R6 has been open since the initial snapshot. | +| Required-check execution on merged heads | Not measured. No merges in the window, and the CI job conditions were read rather than exercised. | 77 of 84 merged PRs never ran the automatic quality and test jobs | Unchanged by this run. The `quality` and `test` jobs in `.github/workflows/ci.yml` still require a `release/` or `release-v` head or a manual dispatch. | +| Unresolved P0 findings | 1 (S1) | 1 (S1) | No change. | +| Unresolved P1 findings | 9 (S2, S3, S4, S5, S6, S7, S9, R1, R3) | same set | No change. Source trees are identical to the baseline. | +| Expired exceptions | 0, because no registry exists | not recorded | Baseline. | + +Deletion count is not a metric and is not recorded. + +## Cleanup selected + +These are the bounded cleanup pull requests this run selects. Each names one +finding and the check that will show the change. None is started by this report. + +| Scope | Finding | Closing check | +| --- | --- | --- | +| Isolate the plugin hot-reload teardown in `src-api/test/unit/plugins/loader.test.ts` so the watcher closes within the hook timeout, and assert the watcher stops deterministically instead of relying on a 30s hook budget. | T1 | `pnpm test:api test/unit/plugins/loader.test.ts` passes, and `pnpm test:fast` shows no hook timeout. | +| Remove the nine obsolete allowlist entries whose files already fit the 350-line default. | C1 | `pnpm check:component-size` passes with 59 allowlisted entries. | +| Remove one proven API orphan, starting with the smallest, after an exact-path caller search across the workspaces and the CLI and plugin entry points. | O1 | `pnpm --filter neumar-api lint`, API typecheck, and the affected API tests pass. | + +The other findings are carried, not selected this run. S1 remains the highest +priority and is already tracked in [#200](https://github.com/bravew/Neumar/issues/200). + +## LESSONS.md + +No change. This run found no failure pattern that is not already a lesson. The +two failures it observed map to existing rules: the `resolve.test.ts` failure is +the "test isolation: stub the seam, not the environment" lesson, and the plugin +loader timeout is the "a red under load is a timing assumption a user will meet +on a busy machine" clause of the "when a check fails in your area, fix it or file +it" lesson. Neither adds a new pattern, so neither "Seen" count is incremented +here; incrementing requires a review day, and this is a dry run. + +The promoted lesson, seen on three review days and moved to +[AGENTS.md](../../../AGENTS.md), is the same process the runbook defines. This dry +run is the first review day for the new process itself, so nothing is promoted +yet. + +## Unverified + +- **Node 22 baseline.** This run used Node v26.11.1, the same local runner the + earlier review used. CI uses Node 22. The `resolve.test.ts` and plugin loader + failures were not reproduced under Node 22. +- **`cargo check`.** The command failed in the Tauri build script because the + sidecar resource `src-api/dist/neumar-api-aarch64-apple-darwin` is absent. The + Rust source was not checked. Producing that artifact needs the API binary build + step, which was not run. This is not evidence of a Rust defect. +- **Browser end-to-end tests.** `pnpm test:e2e:browser` was not run. It starts its + own API and a browser, and no browser scenario was in scope for a + documentation change. +- **Gate evals.** `pnpm test:gate` was not run. It needs `EVALS_TIER=gate` and + the eval fixtures, and no eval surface changed. +- **Live providers and packaged desktop.** No provider credentials were used and + no packaged build was produced, so no live-provider or desktop behavior is + verified. +- **Required-check execution on merged heads.** Read from + `.github/workflows/ci.yml` job conditions, not exercised against a pull request + in this window. The metric is carried from the earlier review, not re-measured. +- **Exception registry.** No machine-readable exception registry exists, so the + expired-exception metric cannot be measured beyond "none recorded". +- **Link check.** Every relative link in the runbook and this report was resolved + against the checkout by hand; no automated link checker was run. + +## Scheduling + +Scheduling the review is a separate maintainer decision and is not live. No +workflow, cron, or external job runs this process. This report is a manual dry +run. diff --git a/doc/maintenance/incidents/README.md b/doc/maintenance/incidents/README.md new file mode 100644 index 00000000..36c97f6e --- /dev/null +++ b/doc/maintenance/incidents/README.md @@ -0,0 +1,43 @@ +# Contributor policy records + +Date: 2026-10-14. This directory holds the records that the contributor quality +policy in +[03-lessons-and-contributors.md](../2026-10-13-regression/03-lessons-and-contributors.md) +asks for: the incident record template, the empty blocklist, and the replay of +the thresholds against the documented historical cases. + +The policy starts without names. No contributor, vendor, model family, or agent +session is identified here, and the blocklist is empty. + +| File | Purpose | +| --- | --- | +| [incident-record-template.md](incident-record-template.md) | Blameless incident record format | +| [blocklist.json](blocklist.json) | Empty blocklist with the entry schema | +| [blocklist-schema.mjs](blocklist-schema.mjs) | Schema constants and validation, shared by the check and its tests | +| [fixtures/blocklist-valid.json](fixtures/blocklist-valid.json) | A valid entry used only to exercise the schema | +| [fixtures/blocklist-invalid.json](fixtures/blocklist-invalid.json) | An entry missing a required field, used only to exercise rejection | +| [policy-replay.md](policy-replay.md) | Each documented historical case placed against the thresholds | + +## Rules this directory follows + +- An incident record names the contract that failed and the gate that missed it. + It does not judge a person. The practice is blameless postmortem + ([Google SRE](https://sre.google/sre-book/postmortem-culture/)). +- A record counts only after two reviewers or a maintainer with an independent + verification artifact confirm it. +- The blocklist stays empty until the policy's own evidence bar is met and a + maintainer records a decision. Each entry must carry evidence, affected scope, + effective date, expiry, remediation, and an appeal link. +- A block applies to a demonstrated contributor or tool configuration. It is not + a blacklist of a vendor or a model family. + +## Checks + +```bash +node doc/maintenance/incidents/check-blocklist.mjs doc/maintenance/incidents/blocklist.json +node --test doc/maintenance/incidents/check-blocklist.test.mjs +``` + +The first command validates the committed blocklist. The test runs the same +validator against both fixtures and asserts the valid entry is accepted and the +entry missing a required field is rejected. diff --git a/doc/maintenance/incidents/blocklist-schema.mjs b/doc/maintenance/incidents/blocklist-schema.mjs new file mode 100644 index 00000000..2f5a77a4 --- /dev/null +++ b/doc/maintenance/incidents/blocklist-schema.mjs @@ -0,0 +1,136 @@ +// Single source for the blocklist entry contract. The committed +// doc/maintenance/incidents/blocklist.json documents this schema and must +// declare the same required fields, which check-blocklist.mjs enforces. Keep +// the validator and the documentation derived from these constants. + +export const BLOCKLIST_VERSION = 1; + +// Required on every entry. The contributor policy in +// doc/maintenance/2026-10-13-regression/03-lessons-and-contributors.md names +// evidence, affected scope, effective date, expiry, remediation and an appeal +// link; id, contributorReference and incidentRecords identify the entry and the +// confirmed records behind it. +export const REQUIRED_FIELDS = [ + 'id', + 'contributorReference', + 'incidentRecords', + 'evidence', + 'affectedScope', + 'effectiveDate', + 'expiry', + 'remediation', + 'appealLink', +]; + +const ISO_DATE = /^\d{4}-\d{2}-\d{2}$/; + +// kind is one of string, date, stringList. date validates the ISO form. +export const FIELD_SPECS = { + id: { + kind: 'string', + description: + 'Stable record id for this entry, used by the incident records and the weekly review.', + }, + contributorReference: { + kind: 'string', + description: + 'Opaque reference to the demonstrated contributor or tool configuration. Never a vendor, model family, or agent session name.', + }, + incidentRecords: { + kind: 'stringList', + description: + 'Ids of the confirmed incident records behind this entry. Each must meet the evidence bar.', + }, + evidence: { + kind: 'stringList', + description: + 'Links to the confirming artifacts, such as incident records and verification runs.', + }, + affectedScope: { + kind: 'string', + description: + 'What is restricted and where, such as autonomous submission or merge for a named repository surface.', + }, + effectiveDate: { + kind: 'date', + description: 'ISO date the restriction starts.', + }, + expiry: { + kind: 'date', + description: + 'ISO date the entry is reviewed. A block is renewed only with stated evidence.', + }, + remediation: { + kind: 'string', + description: + 'What must be done to clear the entry, such as repairing outstanding incidents and passing reviewed scoped contributions.', + }, + appealLink: { + kind: 'string', + description: + 'Where an appeal is filed. An appeal can correct attribution, severity, or causation at any time.', + }, +}; + +function checkValue(kind, value) { + if (kind === 'string') { + return typeof value === 'string' && value.trim().length > 0; + } + if (kind === 'date') { + return typeof value === 'string' && ISO_DATE.test(value); + } + if (kind === 'stringList') { + return ( + Array.isArray(value) && + value.length > 0 && + value.every((item) => typeof item === 'string' && item.trim().length > 0) + ); + } + return false; +} + +// Returns { ok, errors }. errors is a list of human-readable strings. +export function validateBlocklist(doc) { + const errors = []; + if (doc === null || typeof doc !== 'object' || Array.isArray(doc)) { + return { ok: false, errors: ['blocklist must be a JSON object'] }; + } + + const declaredRequired = doc.schema?.entry?.required; + if (!Array.isArray(declaredRequired)) { + errors.push('schema.entry.required must be an array of field names'); + } else { + const declared = [...declaredRequired].sort().join(','); + const expected = [...REQUIRED_FIELDS].sort().join(','); + if (declared !== expected) { + errors.push( + `schema.entry.required does not match the validator contract: expected ${expected}`, + ); + } + } + + if (!Array.isArray(doc.entries)) { + errors.push('entries must be an array'); + return { ok: errors.length === 0, errors }; + } + + doc.entries.forEach((entry, index) => { + const where = `entries[${index}]`; + if (entry === null || typeof entry !== 'object' || Array.isArray(entry)) { + errors.push(`${where} must be an object`); + return; + } + for (const field of REQUIRED_FIELDS) { + if (!(field in entry)) { + errors.push(`${where} is missing required field "${field}"`); + continue; + } + const spec = FIELD_SPECS[field]; + if (!checkValue(spec.kind, entry[field])) { + errors.push(`${where}.${field} must be a non-empty ${spec.kind}`); + } + } + }); + + return { ok: errors.length === 0, errors }; +} diff --git a/doc/maintenance/incidents/blocklist.json b/doc/maintenance/incidents/blocklist.json new file mode 100644 index 00000000..190d6b5c --- /dev/null +++ b/doc/maintenance/incidents/blocklist.json @@ -0,0 +1,37 @@ +{ + "version": 1, + "description": "Contributor policy blocklist. Empty by design. No contributor, vendor, model family, or agent session is named here. Entries are added only when the policy's evidence bar is met and a maintainer records a decision. See README.md and the contributor policy in ../2026-10-13-regression/03-lessons-and-contributors.md.", + "policy": { + "probation": "Two confirmed escaped material defects attributable to the same contributor or tool configuration within 90 days.", + "block": "Three confirmed material incidents within 90 days, including at least one repeated failure class after its documented correction, or a verified deliberate bypass of required validation.", + "reinstatement": "Repair outstanding incidents, demonstrate the failed contracts under the new gate, and pass three independently reviewed scoped contributions. A block is reviewed after 30 days and renewed only with stated evidence.", + "appeal": "An appeal can correct attribution, severity, or causation at any time. Link it from the entry's appealLink." + }, + "schema": { + "entry": { + "required": [ + "id", + "contributorReference", + "incidentRecords", + "evidence", + "affectedScope", + "effectiveDate", + "expiry", + "remediation", + "appealLink" + ], + "fields": { + "id": "Stable record id for this entry.", + "contributorReference": "Opaque reference to the demonstrated contributor or tool configuration. Never a vendor, model family, or agent session name.", + "incidentRecords": "Ids of the confirmed incident records behind this entry.", + "evidence": "Links to the confirming artifacts, such as incident records and verification runs.", + "affectedScope": "What is restricted and where.", + "effectiveDate": "ISO date the restriction starts.", + "expiry": "ISO date the entry is reviewed.", + "remediation": "What must be done to clear the entry.", + "appealLink": "Where an appeal is filed." + } + } + }, + "entries": [] +} diff --git a/doc/maintenance/incidents/check-blocklist.mjs b/doc/maintenance/incidents/check-blocklist.mjs new file mode 100644 index 00000000..87e66634 --- /dev/null +++ b/doc/maintenance/incidents/check-blocklist.mjs @@ -0,0 +1,42 @@ +// Validates a contributor policy blocklist against the entry contract in +// blocklist-schema.mjs. +// +// node doc/maintenance/incidents/check-blocklist.mjs +// +// Exits 0 when the document and every entry match the schema, 1 otherwise. An +// empty blocklist is valid: the policy starts with no entries. + +import { validateBlocklist } from './blocklist-schema.mjs'; + +import { readFileSync } from 'node:fs'; +import process from 'node:process'; + +const file = process.argv[2]; +if (!file) { + console.error( + 'Usage: node doc/maintenance/incidents/check-blocklist.mjs ', + ); + process.exit(1); +} + +let doc; +try { + doc = JSON.parse(readFileSync(file, 'utf-8')); +} catch (error) { + console.error(`Cannot read ${file}: ${error.message}`); + process.exit(1); +} + +const { ok, errors } = validateBlocklist(doc); +if (!ok) { + console.error(`${file} is not a valid blocklist:`); + for (const error of errors) { + console.error(`- ${error}`); + } + process.exit(1); +} + +const count = doc.entries.length; +console.log( + `ok: ${file} is a valid blocklist with ${count} entr${count === 1 ? 'y' : 'ies'}`, +); diff --git a/doc/maintenance/incidents/check-blocklist.test.mjs b/doc/maintenance/incidents/check-blocklist.test.mjs new file mode 100644 index 00000000..a9fb4864 --- /dev/null +++ b/doc/maintenance/incidents/check-blocklist.test.mjs @@ -0,0 +1,57 @@ +import { REQUIRED_FIELDS, validateBlocklist } from './blocklist-schema.mjs'; + +import assert from 'node:assert/strict'; +import { readFileSync } from 'node:fs'; +import { test } from 'node:test'; +import { fileURLToPath } from 'node:url'; + +const read = (name) => + JSON.parse( + readFileSync(fileURLToPath(new URL(name, import.meta.url)), 'utf-8'), + ); + +test('committed blocklist is valid and empty', () => { + const { ok, errors } = validateBlocklist(read('./blocklist.json')); + assert.equal(ok, true, errors.join('; ')); +}); + +test('valid fixture entry passes', () => { + const { ok, errors } = validateBlocklist( + read('./fixtures/blocklist-valid.json'), + ); + assert.equal(ok, true, errors.join('; ')); +}); + +test('fixture missing a required field is rejected', () => { + const { ok, errors } = validateBlocklist( + read('./fixtures/blocklist-invalid.json'), + ); + assert.equal(ok, false); + assert.match(errors.join('\n'), /missing required field "remediation"/); +}); + +test('an entry missing any required field is rejected', () => { + for (const field of REQUIRED_FIELDS) { + const entry = read('./fixtures/blocklist-valid.json').entries[0]; + delete entry[field]; + const { ok, errors } = validateBlocklist({ + schema: { entry: { required: REQUIRED_FIELDS } }, + entries: [entry], + }); + assert.equal(ok, false, `expected ${field} to be required`); + assert.match( + errors.join('\n'), + new RegExp(`missing required field "${field}"`), + ); + } +}); + +test('a documented schema that drops a required field is rejected', () => { + const doc = read('./blocklist.json'); + doc.schema.entry.required = doc.schema.entry.required.filter( + (field) => field !== 'appealLink', + ); + const { ok, errors } = validateBlocklist(doc); + assert.equal(ok, false); + assert.match(errors.join('\n'), /does not match the validator contract/); +}); diff --git a/doc/maintenance/incidents/fixtures/blocklist-invalid.json b/doc/maintenance/incidents/fixtures/blocklist-invalid.json new file mode 100644 index 00000000..f342397f --- /dev/null +++ b/doc/maintenance/incidents/fixtures/blocklist-invalid.json @@ -0,0 +1,33 @@ +{ + "version": 1, + "description": "Schema fixture. This entry is intentionally missing the required remediation field, so the check must reject it. It is not a real record.", + "schema": { + "entry": { + "required": [ + "id", + "contributorReference", + "incidentRecords", + "evidence", + "affectedScope", + "effectiveDate", + "expiry", + "remediation", + "appealLink" + ] + } + }, + "entries": [ + { + "id": "example-0002", + "contributorReference": "opaque-ref-example", + "incidentRecords": ["incident-example-0003"], + "evidence": [ + "doc/maintenance/incidents/records/incident-example-0003.md" + ], + "affectedScope": "Autonomous merge for the named repository surface.", + "effectiveDate": "2026-10-14", + "expiry": "2026-11-13", + "appealLink": "https://github.com/bravew/Neumar/issues/231" + } + ] +} diff --git a/doc/maintenance/incidents/fixtures/blocklist-valid.json b/doc/maintenance/incidents/fixtures/blocklist-valid.json new file mode 100644 index 00000000..28eac794 --- /dev/null +++ b/doc/maintenance/incidents/fixtures/blocklist-valid.json @@ -0,0 +1,35 @@ +{ + "version": 1, + "description": "Schema fixture. The entry is a placeholder that exercises the blocklist schema; it is not a real record and names no contributor.", + "schema": { + "entry": { + "required": [ + "id", + "contributorReference", + "incidentRecords", + "evidence", + "affectedScope", + "effectiveDate", + "expiry", + "remediation", + "appealLink" + ] + } + }, + "entries": [ + { + "id": "example-0001", + "contributorReference": "opaque-ref-example", + "incidentRecords": ["incident-example-0001", "incident-example-0002"], + "evidence": [ + "doc/maintenance/incidents/records/incident-example-0001.md", + "doc/maintenance/incidents/records/incident-example-0002.md" + ], + "affectedScope": "Autonomous submission for the named repository surface.", + "effectiveDate": "2026-10-14", + "expiry": "2026-11-13", + "remediation": "Repair the outstanding incidents, demonstrate the failed contracts under the current gate, and pass three independently reviewed scoped contributions.", + "appealLink": "https://github.com/bravew/Neumar/issues/231" + } + ] +} diff --git a/doc/maintenance/incidents/incident-record-template.md b/doc/maintenance/incidents/incident-record-template.md new file mode 100644 index 00000000..4068cddb --- /dev/null +++ b/doc/maintenance/incidents/incident-record-template.md @@ -0,0 +1,93 @@ +# Incident record template + +One record per confirmed incident. Copy this file, fill every field, and link it +from the weekly review. A record is a description of a system failure, not a +judgement of a person. Write what contract failed and which gate let it through. + +An incident counts toward the policy thresholds only when two reviewers, or a +maintainer holding an independent verification artifact, confirm it. Until then +it is a candidate, and the `Maintainer disposition` field says so. + +Attribution fields use `unknown` when evidence is unavailable. Do not guess. +Do not name a vendor, model family, or agent session from a product reference in +code or a title; that is not authorship evidence. Keep private session URLs and +identifiers out of this file and put an opaque reference here instead. + +- **Record ID:** +- **Status:** candidate | confirmed | repaired | closed | appealed +- **Date recorded:** +- **Recorded by:** + +## Introducing change + +- **Introducing PR or commit:** +- **Causal evidence:** the specific diff, line, or squash that introduced the + behavior. A stacked PR total is not causal evidence; use the squash diff. +- **Attribution evidence:** the human accountable for landing it, and the + authoring tool or session when known. Use an opaque local reference. Write + `unknown` when the evidence does not exist, and say what would settle it. + +## Reproducible scenario + +- **Failed contract:** the behavior, schema, or invariant that was expected. +- **Steps:** the shortest sequence that reproduces the failure on the real + surface a caller uses, such as the HTTP route, MCP handler, bridge, packaged + build, or chosen runtime. +- **Observed result:** +- **Expected result:** +- **Artifact:** a command, test name, or log that shows the result. + +## Severity + +- **Level:** P0 urgent containment | P1 fix before expanding the feature | + P2 planned cleanup. Use the same scale as + [01-findings.md](../2026-10-13-regression/01-findings.md). +- **Impact:** what a user or system could not do, or what was put at risk. +- **Breadth:** how many callers, routes, platforms, or installs are affected. +- **Reversibility:** whether existing data or published output was changed. + +## Review and check gap + +- **Gate that should have caught it:** +- **Why it did not:** the check skipped, the boundary the test stopped short of, + the fixture that did not match the environment, or the job that only ran on + release heads. +- **What was observed instead:** the green check names, a PR-body claim, or a + manual run that did not cover this path. + +## Repair + +- **Repair PR:** +- **Fixing change:** the cause that was fixed, not the symptom that was hidden. +- **Prevention test or check:** the new or changed test at the boundary above, + and where it runs. + +## Verification + +- **Verified commit:** the exact head the checks ran on. +- **Commands and results:** each command, its outcome, and any failure, including + Node and pnpm versions for a local run. +- **Independent artifact:** the reviewer or maintainer artifact that confirms it. +- **Unverified:** what remains untested, and why. + +## Disposition + +- **Maintainer disposition:** accepted | disputed | no action | probation | + block | reinstatement. Record who decided and when. +- **Policy effect:** does this record count toward the 90-day thresholds, and + why. Note if it is one incident across an epic and its sub-PRs, a repair by a + different author, or an infrastructure failure that does not count. +- **Effective date:** for a block or probation, the date the restriction starts. +- **Expiry:** the date it is reviewed, or `not applicable`. + +## Appeal status + +- **Appeal status:** none | open | upheld | corrected | withdrawn +- **Appeal link:** the issue or record where an appeal is filed. +- **Appeal outcome:** for a closed appeal, what was corrected, such as + attribution, severity, or causation. + +## Notes + +Anything else that helps the next reviewer, including related records and links +to the weekly review that raised it. diff --git a/doc/maintenance/incidents/policy-replay.md b/doc/maintenance/incidents/policy-replay.md new file mode 100644 index 00000000..070c5c28 --- /dev/null +++ b/doc/maintenance/incidents/policy-replay.md @@ -0,0 +1,96 @@ +# Policy replay against the documented historical cases + +Date: 2026-10-14. This note applies the contributor quality policy in +[03-lessons-and-contributors.md](../2026-10-13-regression/03-lessons-and-contributors.md) +to the cases that review recorded, and shows each one landing on the intended +side of the thresholds. It is a paper replay of the policy, not a new +investigation. It adds no names and confirms no new incident. + +## The thresholds under test + +- **Probation:** two confirmed escaped material defects attributable to the same + contributor or tool configuration within 90 days. +- **Block from autonomous submission or merge:** three confirmed material + incidents within 90 days, including at least one repeated failure class after + its documented correction, or a verified deliberate bypass of required + validation. +- **Reinstatement:** repair outstanding incidents, demonstrate the failed + contracts under the new gate, and pass three independently reviewed scoped + contributions. Review a block after 30 days; renew only with stated evidence. + An appeal can correct attribution, severity, or causation at any time. + +A record counts only after two reviewers, or a maintainer with an independent +verification artifact, confirm it. The blocklist stays empty until that evidence +bar and a maintainer decision both exist. + +## Counting rules the replay applies + +1. **One incident, once.** A defect introduced by an epic and carried by its + sub-PRs is a single incident, counted at the introducing change. Do not count + it again at each sub-PR. +2. **A repair is not a strike.** A corrective PR that fixes a preexisting defect + does not receive its own incident. The defect counts where it was introduced. +3. **A transient infrastructure failure is not a code-quality incident.** + Environment-specific or flaky failures do not count against a contributor. +4. **Attribution must be proven.** A reference to a tool or vendor in code, a + title, or a session trailer is not authorship evidence. Use `unknown` and do + not count when the evidence does not exist. +5. **A current check rollup does not establish what was known at merge.** Only a + check that completed before merge, on the matching head, is merge-time + evidence. + +## Case replay + +| Case | Candidate incident | Decision | Policy landing | +| --- | --- | --- | --- | +| Stacked PRs #102, #103, #105 | The duplicate `toolResultText` import in #103 squash `10edc9f3`, after #102 introduced the symbol, plus the API test type and format errors #105 repaired. | One incident. It is one failure chain across a stacked branch, counted once at the integration defect, not three times. #102 is mostly a legitimate lockfile pin; #103 is smaller than its stacked totals suggest; #105 is the corrective author and gets no strike. | 1 confirmed incident, below probation (2) and block (3). | +| Manual CI #176 | The mock and toast drift that #175 introduced and #176 repaired, which automatic PR CI did not catch because the quality and Test jobs run only on release heads. | One incident, counted at #175, the introducing change. #176 is the repair and is not counted. The missing automatic PR gate is the check gap, recorded as a process fix, not a second strike. | 1 confirmed incident for the introducing configuration, below probation and block. The gate gap is a process item, not a contributor incident. | +| Different authors | Two or more contributors are said to have authored the held PRs, so the same defect pattern might accumulate per contributor. | No count. All sampled merged PRs share one GitHub identity and tool or session attribution is incomplete, so the policy cannot fairly attribute repeated defects to a named human, vendor, model family, or agent session. Even with attribution, each documented case above is a single incident. | 0 counted, below probation and block. | +| Infrastructure failure: `resolve.test.ts:251` | The `pnpm test:fast` failure on a host with Homebrew OpenCode installed, tracked in #195. | No count. It is an environment-specific test-isolation defect, not a contributor's code-quality incident. | 0 counted. Handled as test-isolation work, not a strike. | +| Timing case: #104 audit | An audit check that completed after #104 merged. | No count. A current rollup alone cannot establish what was known at merge, and this failure completed after merge, so it is not an already-red merge. | 0 counted. | + +Every documented case lands below the probation threshold of two, so the replay +produces no probation and no block. The intended side of each threshold is the +side with fewer than two confirmed, attributable incidents. + +## Per-configuration check + +The policy counts per contributor or tool configuration within 90 days, not per +repository. + +| Configuration | Confirmed incidents in the replay | Threshold reached | +| --- | ---: | --- | +| Introducing configuration for the stacked-PR chain | 1 | None; below probation. | +| Introducing configuration for the #175 to #176 chain | 1 | None; below probation. | +| Every other sampled configuration | 0 | None. | + +No configuration reaches two incidents, so no probation entry is created. With no +probation entries, no configuration can reach the third incident that would +trigger a block, including the repeated-failure-class condition. The blocklist +stays empty. + +## Reinstatement path, if an entry existed + +For a hypothetical block created from this policy, clearing it follows the same +reinstatement path the policy states, and the entry's own fields carry the +details: + +1. Repair the outstanding incidents named in the entry's `incidentRecords`. +2. Demonstrate the failed contracts under the current gate, using the boundary + the incident record names. +3. Pass three independently reviewed scoped contributions. +4. At the entry's `expiry`, review the block and renew it only with stated + evidence, or let it lapse. +5. At any time, an appeal filed at `appealLink` can correct attribution, + severity, or causation, which can remove the entry. + +## What this replay does not establish + +- It does not confirm any new incident. It reuses the review's recorded findings + and its attribution limits. +- It does not identify an author. Attribution stays `unknown` where the review + could not prove it. +- It does not evaluate the thresholds against code that changed after the + reviewed baseline `c0926618`. Re-run it when a new confirmed record exists. +- It does not create, extend, or lift a restriction. The blocklist is empty and + no repository setting has changed. diff --git a/doc/maintenance/review-process.md b/doc/maintenance/review-process.md new file mode 100644 index 00000000..b966ef2d --- /dev/null +++ b/doc/maintenance/review-process.md @@ -0,0 +1,194 @@ +# Weekly and pre-release maintenance review + +This runbook defines the recurring maintenance review. It is written so one +person or one agent can run it from a checkout, publish a report, and select +bounded cleanup work without inventing a process each time. + +The review exists because a fix that nothing rechecks decays. The first review +([2026-10-13 regression review](2026-10-13-regression/README.md)) found that 77 +of 84 merged pull requests never ran the automatic quality and test jobs, that +four release pull requests merged with a quality or test failure already +completed on the matching head, and that four recent feature pull requests +proved the wrong layer. Repeating the review is how the repository notices the +same class of failure coming back. + +Scheduling is not part of this document. A recurring job, a scheduled workflow, +or an external reminder is a separate maintainer decision, and none is live. The +process below is run by hand until the maintainer decides otherwise. + +## Cadence and owner + +- **Weekly.** Run the review once a week against the current default branch head. +- **Pre-release.** Run the same review against the exact head to be released + before any `release:new` or `release:publish` step. The pre-release run is the + gate, so it does not wait for the weekly slot. +- **Owner.** Every report names one accountable owner in its `Owner` field, plus + the reviewer who produced it when they differ. The owner is the person + answerable for the open findings and the cleanup selections in that report, not + every contributor named in it. When no owner is recorded, the report is + incomplete and the review did not happen. + +A weekly run that finds nothing new still publishes a report. The report records +what was checked and the exact check outcomes, so a quiet week is visible +evidence rather than an absent one. + +## Review scope + +Each run inspects these surfaces. A surface with no change is reported as +unchanged with the command or query that showed it. + +1. **New lint warnings and suppressions.** Compare the warning count and the + per-rule breakdown against the previous report. Count new `eslint-disable`, + `oxlint-disable`, `@ts-ignore`, `@ts-expect-error`, and `@ts-nocheck` + directives. A new suppression in changed code is a finding unless a reviewed + exception with an owner, a reason, and an expiry exists. See + [LESSONS.md](../../LESSONS.md), "Never raise a threshold or allowlist ceiling + to make a check pass." +2. **Size-ceiling changes.** Diff + [component-size-allowlist.json](../../scripts/component-size-allowlist.json). + A raised `current` ceiling is a finding. A lowered ceiling or a removed entry + is progress. Do not treat a deletion count as a quality measure on its own. +3. **Orphan candidates.** Recheck the dead-code candidates from the previous + report and the list in [01-findings.md](2026-10-13-regression/01-findings.md). + A candidate is only removed after an exact-path and symbol search across every + workspace, the build entry points, the plugin manifests, and the desktop + consumers. Knip output is a lead, never a deletion authority. +4. **Unsupported feature surfaces.** Find routes that return 501 or an explicit + "not implemented" and confirm each one is intentional and truthful. A control + that offers a feature the API refuses is a finding. The current known surfaces + are the chat plugin apply route and the memory extract and consolidate routes. +5. **Migration changes.** Any change under `src-api/src/shared/db/migrations/` + gets a full-registry run against fresh, current, and drifted snapshots. A + missing table or column means unknown, never empty. Do not renumber applied + migrations. +6. **Security findings.** Recheck the open findings S1 to S9 in + [01-findings.md](2026-10-13-regression/01-findings.md). A security gap a + document names is an open bug, not a description, so a finding that is + documented but unfixed stays open until an issue tracks it and the fix lands. +7. **Unresolved regressions.** Recheck R1 to R6 and any regression added since. + A regression closes only when a test at the caller boundary fails before the + fix and passes after it. +8. **Expired exceptions.** Every exception carries an owner, a reason, and an + expiry. List the ones that have expired and the ones that expire before the + next scheduled run. An expired exception is an open finding until it is + renewed with a new reason or resolved. + +## Report format + +Publish each run as a new dated directory under `doc/maintenance/`, for example +`doc/maintenance/2026-10-14-weekly/README.md`. Use these headings in this order. +Every row that names a check records the exact command, its exit status, and the +counts it printed. Every finding links the pull request or the source path and +line that shows it. + +``` +# maintenance review, + +Owner: +Reviewer: +Head: +Node and pnpm: +Previous report: + +## Check outcomes +| Command | Result | Notes | +| --- | --- | --- | +| pnpm validate | | | +| pnpm test:fast | | | +| pnpm --filter neumar-api lint | | | +| pnpm --filter neumar-api exec tsc -p tsconfig.json --noEmit | | | +| closest changed-surface checks | | | + +## Findings +| ID | Surface | Finding | Evidence | Owner | Status | +| --- | --- | --- | --- | --- | --- | + +## Metrics +| Metric | This run | Previous | Trend | +| --- | ---: | ---: | --- | + +## Cleanup selected + + +## LESSONS.md + + +## Unverified + +``` + +The report distinguishes what this run observed from evidence carried over from +an earlier review. A carried-over line names the review and the commit it came +from. A check that could not run is listed under Unverified with the reason; it +is never reported as a pass. + +## Selecting bounded cleanup pull requests + +The review selects cleanup work, it does not perform it in the report. A cleanup +pull request is bounded when it meets all of these: + +- It has one goal and one subject line. A fix found on the way gets its own pull + request. +- It names the finding or the metric it moves, and the exact check that will show + the change. +- It migrates every caller and deletes the superseded path in the same change. +- It does not raise a threshold or an allowlist ceiling to pass a check. An + exception is recorded with an owner, a reason, and an expiry instead. +- It is small enough to review against the finding in one sitting. When a + candidate is larger, split it and select the first slice. + +The review does not reward deletion count, and it does not punish every +follow-up fix. A corrective pull request that repairs an escaped regression is +responsible work; the escaped regression is the finding, not the repair. The +review selects work by the risk the finding carries and the evidence that will +close it, not by how many lines it removes. + +## The LESSONS.md step + +Every run ends with one of these outcomes for [LESSONS.md](../../LESSONS.md): + +- **Add a lesson.** When a run finds a failure pattern that is not already a + lesson, add it with its evidence: a pull request number, a commit, or a file + and line, plus what was run and what was missed. Keep the rule first. +- **Increment a lesson.** When a run finds the same pattern again, increment that + lesson's "Seen" count and record the review date. Say which review day, not + only the count. +- **No change.** When no new pattern and no repeat was found, record that in the + report. This is a valid outcome. + +When a lesson reaches three review days, move the rule into +[AGENTS.md](../../AGENTS.md) so every agent applies it without a separate read, +and leave a pointer to it in LESSONS.md. The pointer keeps the evidence +findable; the rule lives in AGENTS.md from then on. + +## Metrics + +Record these each run. The trend across runs is the point; a single run's number +is only a baseline. + +- **Escaped regressions.** Regressions found after merge that a required check + should have caught. Count the ones introduced since the last report. +- **Time to repair.** From the merge that introduced a regression to the merge + that fixed it, for each escaped regression closed since the last report. +- **Required-check execution.** The share of merged pull requests in the window + whose required checks actually ran on the merged head. Skipped, cancelled, and + absent checks do not count as executed. +- **Unresolved high-priority findings.** The count of open P0 and P1 findings + from [01-findings.md](2026-10-13-regression/01-findings.md) plus any added + since, and how many are older than the last two runs. +- **Expired exceptions.** The count of exceptions past their expiry, and the + count expiring before the next run. + +Deletion count is not a metric. It is not recorded as progress, and a run with +more deletions is not a better run than one with fewer. + +## Related documents + +- [Regression review and maintenance plan](2026-10-13-regression/README.md): the + evidence, the plan, and the baseline this process rechecks. +- [Findings and evidence](2026-10-13-regression/01-findings.md): the S1 to S9 and + R1 to R6 findings the review rechecks. +- [PR lessons and contributor policy](2026-10-13-regression/03-lessons-and-contributors.md): + the merge metrics and the contributor policy. +- [LESSONS.md](../../LESSONS.md): the rules the review adds to and increments. +- [AGENTS.md](../../AGENTS.md): where a lesson moves after three review days.