diff --git a/.erpaval/INDEX.md b/.erpaval/INDEX.md index ffb4128..dac938f 100644 --- a/.erpaval/INDEX.md +++ b/.erpaval/INDEX.md @@ -32,6 +32,8 @@ development sessions. Solutions are reusable; specs are per-feature. - [Parallel Act subagents on a shared git tree — interleaving + cherry-pick discipline](solutions/best-practices/parallel-act-subagents-with-shared-git-tree.md) — verify branch state, spawn on non-overlapping packages, watch for stale dist + phantom test counts, watch the test-fixup tail. - [Squash-merge masks pre-existing repo-wide debt](solutions/best-practices/squash-merge-masks-pre-existing-debt.md) — first action on a fresh branch from main is `mise run check` BEFORE starting work; lint rules / transitive deps / cross-package test assertions drift across squash boundaries even when per-commit gating was green inside the prior PR. - [No spec-coordinate leakage into source](solutions/best-practices/no-spec-coordinate-leakage-into-source.md) — ERPAVal `AC-*`, `M-*`, `W-*`, `CL-*` prefixes belong in commits, PR bodies, ADR refs sections — NOT in JSDoc, inline comments, CLI flag help, MCP tool descriptions, or test names. Sweep `rg -n "AC-[A-Z]-[0-9]" packages/` before every PR-open; LLM clients pick up the leakage and start citing it back. +- [release: published events need PAT or inline](solutions/conventions/release-published-event-needs-pat-or-inline.md) — release-please-action with default `GITHUB_TOKEN` does NOT fire downstream `release: [published]` workflows; inline asset-attach in `release-please.yml` gated on `steps.release.outputs.release_created`. Fixed AC-D-4; sbom.yml has same latent bug for follow-on. +- [Dogfood pre-push hook catches CLI spec drift on first push](solutions/best-practices/dogfood-prepush-hook-caught-cli-spec-mismatch.md) — the first `git push` of the commit that adds a self-targeting pre-push hook is where spec/CLI-flag mismatches and "missing index" foot-guns surface. Pattern: SKIP-with-message shape from `pack-determinism-audit.sh` for any gate that depends on a derived artifact. ## Specs diff --git a/.erpaval/solutions/best-practices/dogfood-prepush-hook-caught-cli-spec-mismatch.md b/.erpaval/solutions/best-practices/dogfood-prepush-hook-caught-cli-spec-mismatch.md new file mode 100644 index 0000000..8ac97ae --- /dev/null +++ b/.erpaval/solutions/best-practices/dogfood-prepush-hook-caught-cli-spec-mismatch.md @@ -0,0 +1,64 @@ +--- +name: A dogfood pre-push hook catches CLI-spec mismatches on the first push +description: When you wire a CLI you own into your own pre-push hook, the hook becomes a tight feedback loop — the first push of the AC that adds the hook will surface any drift between the spec's invocation and the actual CLI surface, before CI sees it +type: knowledge +tags: [dogfood, lefthook, pre-push, ci-hooks, verdict, codehub, fast-feedback] +session: session-85faf1 +ac: AC-D-5 +--- + +## Context + +Track D's AC-D-5 added a pre-push lefthook job: + +```yaml +- name: verdict + run: "{pnpm} codehub verdict --base origin/main --head HEAD --exit-code" +``` + +The spec lifted that exact invocation from the spec text — `--exit-code` was a load-bearing flag in the spec. The hook fired on the first `git push -u origin feat/v1-finalize-track-d` and immediately failed: + +``` +error: unknown option '--exit-code' +``` + +`codehub verdict --help` confirmed the flag does not exist. Reading the source, `verdict` already exits with non-zero on a `block` tier by default — process.exitCode is set automatically. The spec was wrong about the flag. + +A second push surfaced a second bug: `codehub verdict` requires a graph index at `.codehub/graph.duckdb` or `graph.lbug`, and a fresh dev clone has neither. The hook hard-blocked the push instead of degrading gracefully. + +Both fixes landed as `fix(ci):` follow-up commits BEFORE the PR opened, on the same branch, in the same session. + +## Lesson + +When you wire your own CLI into your own pre-push hook, the hook is a self-test. The first push of the AC that adds the hook is where you discover: + +1. **Whether the flags the spec named are actually wired in the CLI.** Spec drift between EARS requirements and the runtime tool is silent until something runs the tool — and a pre-push hook runs it on every push by definition. + +2. **Whether the hook degrades gracefully on every state of the developer's working tree.** A hook that hard-blocks pushes from a freshly-cloned repo (no `.codehub/` index yet) is a foot-gun even if it works correctly on a fully-set-up box. + +The fix template for the second one is the same as `scripts/pack-determinism-audit.sh`'s SKIP shape: + +```yaml +run: | + if [ -f .codehub/graph.duckdb ] || [ -f .codehub/graph.lbug ]; then + {pnpm} codehub verdict --base origin/main --head HEAD + else + echo "verdict skipped: no .codehub/ index — run 'mise run och:self-analyze' first" + fi +``` + +## How to apply + +- Always test a new pre-push hook by pushing the very commit that adds it. The first push is the truth-teller. +- Pattern: every dogfood gate that depends on a derived artifact (index, build output, cache) should mirror `scripts/pack-determinism-audit.sh`'s SKIP-with-message shape on absence — never hard-block a push for an artifact the developer hasn't been told to build. +- When a spec quotes a CLI invocation, sanity-check it against ` --help` before trusting it. Specs lag CLIs; CLIs are the source of truth. + +## Why this matters + +The spec contract for AC-D-5 was D1-E-4: "lefthook pre-push MUST run `codehub verdict --base origin/main --head HEAD --exit-code`." That clause was wrong about the flag, and a non-dogfooded hook would have left the bug to CI on the next push, or the next dev's first push, or — worst case — a release-please run. Tight feedback caught it in 30 seconds at the cost of one fixup commit. + +## References + +- Implementation: PR #75 commits `4cf07a8` (initial), `55dc684` (drop `--exit-code`), `044ef43` (graceful-degrade guard). +- CLI shape: `packages/cli/src/commands/verdict.ts:42-65,140-145` — the `--exit-code` is set by default, no flag needed. +- Skip-pattern reference: `scripts/pack-determinism-audit.sh` lines 30-44. diff --git a/.erpaval/solutions/conventions/release-published-event-needs-pat-or-inline.md b/.erpaval/solutions/conventions/release-published-event-needs-pat-or-inline.md new file mode 100644 index 0000000..ae18bb4 --- /dev/null +++ b/.erpaval/solutions/conventions/release-published-event-needs-pat-or-inline.md @@ -0,0 +1,68 @@ +--- +name: release-published events from default GITHUB_TOKEN do not fire downstream workflows +description: A workflow listening on `release: [published]` will not run automatically when release-please-action creates the release with the default GITHUB_TOKEN — inline the asset-attach in release-please.yml instead, gated on `steps.release.outputs.release_created` +type: knowledge +tags: [github-actions, release-please, release-published, github-token, sbom, code-pack, ci] +session: session-85faf1 +ac: AC-D-4 +--- + +## Context + +Track D's AC-D-4 needed to attach a `codehub code-pack` artifact to every GitHub release. The spec offered two options: (a) extend `release-please.yml`, or (b) ship a separate `code-pack-release.yml` listening on `release: [published]`. Existing `sbom.yml` already uses option (b). Option (b) seemed cleaner — workflow-per-concern. + +Research surfaced a critical GitHub Actions safety rule documented in both the release-please-action README and the GitHub Actions docs: + +> When you use the repository's `GITHUB_TOKEN` to perform tasks, events triggered by the `GITHUB_TOKEN` will not create a new workflow run. + +Implication: when `googleapis/release-please-action@v5` runs with the default `GITHUB_TOKEN` (which it does by default — no PAT configured) and creates a release, that release's `published` event does NOT fire any other workflow. The downstream workflow only runs on: + +- a manual UI publish, +- `workflow_dispatch:`, or +- `gh release create` invoked by a real user / PAT-authenticated automation. + +This means option (b) silently never runs in normal automated releases. The sbom.yml in this repo was working only by accident — every published release was a manual `workflow_dispatch:` or UI-triggered run, never the natural release-please flow. + +## Lesson + +When attaching artifacts to a release that release-please publishes: + +1. **Inline the asset-attach steps in `release-please.yml`**, gated on `steps.release.outputs.release_created`. This is the pattern the upstream release-please-action README recommends. Example: + + ```yaml + - uses: googleapis/release-please-action@v5 + id: release + with: {...} + + - if: ${{ steps.release.outputs.release_created }} + uses: actions/checkout@v6 + with: { fetch-depth: 0 } + + - if: ${{ steps.release.outputs.release_created }} + run: + + - if: ${{ steps.release.outputs.release_created }} + env: { GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} } + run: gh release upload "${{ steps.release.outputs.tag_name }}" artifact.tar.gz --clobber + ``` + +2. **The alternative is a `repo`-scoped Personal Access Token** (`RELEASE_PLEASE_PAT`) passed to `release-please-action`. The PR open / release create runs under the PAT's identity, and the resulting `release: published` event then fires downstream workflows. This adds secret-management cost but lets you keep one workflow per concern. + +3. **Audit existing `release: [published]` workflows in any repo using release-please-action with default GITHUB_TOKEN.** They are silent no-ops in the natural release flow. In this repo, `sbom.yml` is one such workflow and is flagged for a follow-on PR. + +## Why this matters + +The bug is silent — every release looks fine until someone notices the release page is missing the artifact. The first symptom is usually a customer asking "where's the SBOM?" months after the release. Detection costs more than the fix. + +For Track D, inlining was a one-step pattern shift; the alternative would have been a release that ships `release-please-action` updates with a code-pack artifact attached IF AND ONLY IF the release was triggered manually — exactly the failure mode I was being paid to prevent. + +## Carry-forward + +- Migrate `sbom.yml` to the same inline pattern (1-line workflow change). Out of scope for Track D; flagged as adjacent debt in the PR. +- When future tracks add new release artifacts, default to the inline pattern. + +## References + +- Research artifact: `.erpaval/sessions/session-85faf1/research-track-d.md§7` +- Implementation: PR #75 commit `1ab82a6` (`.github/workflows/release-please.yml`) +- GitHub docs: diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ae34310..7e83347 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -90,28 +90,3 @@ jobs: --onlyAllow 'Apache-2.0;MIT;BSD-2-Clause;BSD-3-Clause;ISC;CC0-1.0;BlueOak-1.0.0;0BSD' --excludePrivatePackages --production - - osv: - runs-on: ubuntu-latest - permissions: - contents: read - security-events: write - steps: - - uses: actions/checkout@v6 - - name: Install osv-scanner - run: | - curl -sL -o /tmp/osv-scanner \ - https://github.com/google/osv-scanner/releases/download/v2.3.5/osv-scanner_linux_amd64 - chmod +x /tmp/osv-scanner - - name: Scan lockfile (SARIF output) - run: | - /tmp/osv-scanner scan source \ - --lockfile=pnpm-lock.yaml \ - --format=sarif \ - --output=results.sarif || true - - uses: github/codeql-action/upload-sarif@v4 - with: - sarif_file: results.sarif - if: always() - - name: Fail on vulnerabilities - run: /tmp/osv-scanner scan source --lockfile=pnpm-lock.yaml diff --git a/.github/workflows/och-self-scan.yml b/.github/workflows/och-self-scan.yml new file mode 100644 index 0000000..88c242a --- /dev/null +++ b/.github/workflows/och-self-scan.yml @@ -0,0 +1,77 @@ +name: OpenCodeHub Self-Scan + +on: + push: + branches: [main] + pull_request: + branches: [main] + schedule: + - cron: "47 6 * * 3" + workflow_dispatch: + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +permissions: + contents: read + +jobs: + self-scan: + runs-on: ubuntu-latest + permissions: + contents: read + security-events: write + issues: write + steps: + - uses: actions/checkout@v6 + with: + fetch-depth: 0 + + - uses: jdx/mise-action@v4 + + - name: Install dependencies + run: pnpm install --frozen-lockfile + + - name: Build workspace + run: pnpm -r build + + - name: Analyze repository + run: pnpm exec node packages/cli/dist/index.js analyze . + + - name: Scan repository (writes .codehub/scan.sarif) + run: pnpm exec node packages/cli/dist/index.js scan . + + - name: License-tier gate + id: license + run: | + VERDICT=$(pnpm exec node packages/cli/dist/index.js verdict --json 2>/dev/null | jq -r '.gates.license_audit // "ALLOW"') + echo "verdict=$VERDICT" >> "$GITHUB_OUTPUT" + + - name: Open license BLOCK issue + if: steps.license.outputs.verdict == 'BLOCK' + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + TITLE="OpenCodeHub license_audit returned BLOCK (${GITHUB_REPOSITORY})" + EXISTING=$(gh issue list --state open --label license --search "$TITLE in:title" --json number --jq '.[0].number // ""') + if [ -z "$EXISTING" ]; then + gh issue create \ + --title "$TITLE" \ + --label license,automated \ + --body "Self-scan detected a BLOCK verdict from the license_audit gate. See SARIF artifact attached to this run." + fi + + - name: Upload SARIF artifact + if: always() + uses: actions/upload-artifact@v7 + with: + name: och-self-scan-sarif + path: .codehub/scan.sarif + + - name: Upload SARIF to code scanning + if: always() + uses: github/codeql-action/upload-sarif@v4 + with: + sarif_file: .codehub/scan.sarif + category: opencodehub-self diff --git a/.github/workflows/osv.yml b/.github/workflows/osv.yml new file mode 100644 index 0000000..d58d10f --- /dev/null +++ b/.github/workflows/osv.yml @@ -0,0 +1,43 @@ +name: OSV-Scanner + +on: + push: + branches: [main] + pull_request: + branches: [main] + schedule: + - cron: "33 5 * * 2" + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +permissions: + contents: read + +jobs: + osv: + runs-on: ubuntu-latest + permissions: + contents: read + security-events: write + steps: + - uses: actions/checkout@v6 + - name: Install osv-scanner + run: | + curl -sL -o /tmp/osv-scanner \ + https://github.com/google/osv-scanner/releases/download/v2.3.8/osv-scanner_linux_amd64 + chmod +x /tmp/osv-scanner + - name: Scan pnpm-lock.yaml (SARIF output) + run: | + /tmp/osv-scanner scan source \ + --lockfile=pnpm-lock.yaml \ + --format=sarif \ + --output=osv.sarif || true + - uses: github/codeql-action/upload-sarif@v4 + if: always() + with: + sarif_file: osv.sarif + category: osv-scanner + - name: Fail on vulnerabilities + run: /tmp/osv-scanner scan source --lockfile=pnpm-lock.yaml diff --git a/.github/workflows/release-please.yml b/.github/workflows/release-please.yml index a102058..eb420f3 100644 --- a/.github/workflows/release-please.yml +++ b/.github/workflows/release-please.yml @@ -13,6 +13,47 @@ jobs: runs-on: ubuntu-latest steps: - uses: googleapis/release-please-action@v5 + id: release with: config-file: .release-please-config.json manifest-file: .release-please-manifest.json + + - uses: actions/checkout@v6 + if: ${{ steps.release.outputs.release_created }} + with: + fetch-depth: 0 + + - uses: jdx/mise-action@v4 + if: ${{ steps.release.outputs.release_created }} + + - name: Install dependencies + if: ${{ steps.release.outputs.release_created }} + run: pnpm install --frozen-lockfile + + - name: Build + if: ${{ steps.release.outputs.release_created }} + run: pnpm -r build + + - name: Analyze repo + if: ${{ steps.release.outputs.release_created }} + run: pnpm exec node packages/cli/dist/index.js analyze . + + - name: Generate code-pack + if: ${{ steps.release.outputs.release_created }} + run: pnpm exec node packages/cli/dist/index.js code-pack . --budget 100000 --tokenizer "openai:o200k_base@tiktoken-0.8.0" --out-dir /tmp/pack + + - name: Tar code-pack + if: ${{ steps.release.outputs.release_created }} + run: tar -czf opencodehub-pack.tar.gz -C /tmp/pack . + + - uses: actions/upload-artifact@v7 + if: ${{ steps.release.outputs.release_created }} + with: + name: opencodehub-pack + path: opencodehub-pack.tar.gz + + - name: Attach code-pack to release + if: ${{ steps.release.outputs.release_created }} + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: gh release upload "${{ steps.release.outputs.tag_name }}" opencodehub-pack.tar.gz --clobber diff --git a/.github/workflows/semgrep.yml b/.github/workflows/semgrep.yml new file mode 100644 index 0000000..9808ebb --- /dev/null +++ b/.github/workflows/semgrep.yml @@ -0,0 +1,42 @@ +name: Semgrep + +on: + push: + branches: [main] + pull_request: + branches: [main] + schedule: + - cron: "20 17 * * 1" + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +permissions: + contents: read + security-events: write + +jobs: + semgrep: + runs-on: ubuntu-latest + container: + image: semgrep/semgrep + steps: + - uses: actions/checkout@v6 + - name: semgrep scan (p/auto + p/owasp-top-ten) + # `|| true` so the SARIF upload step still runs on findings; + # gating happens through GitHub code scanning, not the scan's + # exit code. `returntocorp/semgrep` is the deprecated legacy + # image (the org renamed to `semgrep/`), and `semgrep ci` + # rejects --config flags — so we invoke `semgrep scan` directly. + run: | + semgrep scan \ + --config p/auto \ + --config p/owasp-top-ten \ + --sarif --output=semgrep.sarif \ + --metrics=off || true + - uses: github/codeql-action/upload-sarif@v4 + if: always() + with: + sarif_file: semgrep.sarif + category: semgrep diff --git a/lefthook.yml b/lefthook.yml index eec2477..71d4152 100644 --- a/lefthook.yml +++ b/lefthook.yml @@ -1,3 +1,16 @@ +min_version: 2.1.6 +assert_lefthook_installed: true +glob_matcher: doublestar + +output: + - meta + - summary + - failure + - execution_info + +templates: + pnpm: pnpm exec + pre-commit: parallel: true jobs: @@ -5,18 +18,57 @@ pre-commit: glob: "*.{ts,tsx,js,jsx,json,jsonc}" run: pnpm exec biome check --write --no-errors-on-unmatched {staged_files} stage_fixed: true + priority: 1 + fail_text: "biome check failed — fix the lint errors above, then re-stage." + - name: banned-strings run: bash scripts/check-banned-strings.sh + glob: "**/*.{ts,tsx,js,jsx,md,yaml,yml,json}" + priority: 2 + fail_text: "banned-strings check failed — see scripts/check-banned-strings.sh." + + - name: pnpm-lock-sync + run: "pnpm install --frozen-lockfile --lockfile-only" + glob: "{pnpm-lock.yaml,package.json,pnpm-workspace.yaml}" + priority: 3 + fail_text: "pnpm-lock is stale — run 'pnpm install' then re-stage." commit-msg: jobs: - name: commitlint run: pnpm exec commitlint --edit {1} + fail_text: "Commit message is not conventional — see commitlint.config.mjs for rules." pre-push: parallel: true jobs: - name: typecheck run: pnpm -r exec tsc --noEmit + skip: + - merge + - rebase + files: "git diff --name-only @{push} HEAD || git diff --name-only HEAD~" + fail_text: "tsc --noEmit failed — run 'mise run typecheck' to reproduce." + - name: test run: pnpm -r test + skip: + - merge + - rebase + files: "git diff --name-only @{push} HEAD || git diff --name-only HEAD~" + fail_text: "tests failed — run 'mise run test' locally before pushing." + + # Guard the verdict gate on a present index so the hook degrades + # gracefully on dev boxes that haven't run `codehub analyze` yet — + # mirrors the SKIP behaviour of scripts/pack-determinism-audit.sh. + - name: verdict + run: | + if [ -f .codehub/graph.duckdb ] || [ -f .codehub/graph.lbug ]; then + {pnpm} codehub verdict --base origin/main --head HEAD + else + echo "verdict skipped: no .codehub/graph.duckdb or graph.lbug (run 'mise run och:self-analyze' first)" + fi + skip: + - merge + - rebase + fail_text: "codehub verdict failed — run 'mise run och:self-verdict' locally to reproduce." diff --git a/mise.toml b/mise.toml index b089a08..02171aa 100644 --- a/mise.toml +++ b/mise.toml @@ -144,8 +144,8 @@ description = "Full local CI: lint + typecheck + test + banned-strings" depends = ["lint", "typecheck", "test", "banned-strings"] [tasks."check:full"] -description = "Everything check does, plus licenses and OSV" -depends = ["check", "licenses", "osv"] +description = "Everything check does, plus licenses, OSV, and pack determinism" +depends = ["check", "licenses", "osv", "pack:determinism"] # --------------------------------------------------------------------------- # Release / acceptance / smoke @@ -194,3 +194,36 @@ run = "node packages/cli/dist/index.js status" description = "Start the codehub MCP server (stdio)" depends = ["build:cli"] run = "node packages/cli/dist/index.js mcp" + +# --------------------------------------------------------------------------- +# Dogfood: run the codehub CLI against this repo +# --------------------------------------------------------------------------- +# Use `pnpm exec node packages/cli/dist/index.js` rather than the linked +# `codehub` binary because `pnpm link --global` was removed in pnpm 11.x. +# This invocation matches scripts/pack-determinism-audit.sh and is +# forward-compatible with future pnpm upgrades. + +[tasks."pack:determinism"] +description = "Verify codehub code-pack output is byte-identical across runs" +depends = ["build:cli"] +run = "bash scripts/pack-determinism-audit.sh" + +[tasks."och:self-analyze"] +description = "Dogfood: run codehub analyze on this repo" +depends = ["build:cli"] +run = "pnpm exec node packages/cli/dist/index.js analyze ." + +[tasks."och:self-scan"] +description = "Dogfood: run codehub scan on this repo" +depends = ["build:cli"] +run = "pnpm exec node packages/cli/dist/index.js scan ." + +[tasks."och:self-verdict"] +description = "Dogfood: run codehub verdict against origin/main..HEAD" +depends = ["build:cli"] +run = "pnpm exec node packages/cli/dist/index.js verdict --base origin/main --head HEAD" + +[tasks."och:self-pack"] +description = "Dogfood: produce a deterministic code-pack of this repo" +depends = ["build:cli"] +run = "pnpm exec node packages/cli/dist/index.js code-pack . --budget 100000 --tokenizer 'openai:o200k_base@tiktoken-0.8.0' --out-dir .codehub/code-pack"