Skip to content

Commit aaf149b

Browse files
joshunraujoshunrau
authored
Merge pull request DouglasNeuroInformatics#1479 from joshunrau/skill-library
add skill library
2 parents 46d0184 + da467a2 commit aaf149b

49 files changed

Lines changed: 1640 additions & 35 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎.agents/docs/architecture/testing-strategy.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,8 @@ to `apps/gateway` or `packages/react-core` today is collected by nothing, report
1212
passes CI green.
1313

1414
Check the table below before writing a unit test. If the package is not in it, add a
15-
`vitest.config.ts` in the same change — `packages/schemas/vitest.config.ts` is the whole pattern
16-
(`mergeConfig(baseConfig, defineProject({ test: { name, root: import.meta.dirname } }))`).
15+
`vitest.config.ts` in the same change — `.agents/docs/playbooks/add-vitest-project.md` is the order
16+
of operations.
1717

1818
## Tiers
1919

‎.agents/docs/playbooks/add-vitest-project.md‎

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,134 @@
1+
# Add a vitest project
2+
3+
Give a workspace a unit-test tier it does not have yet. What is tested where lives in
4+
`.agents/docs/architecture/testing-strategy.md`; read it first. This file is only the ordering that
5+
matters.
6+
7+
**Plan around this:** the failure this playbook prevents is silent. Root `vitest.config.ts` declares
8+
`projects: ['apps/*/vitest.config.ts', 'packages/*/vitest.config.ts', 'runtime/*/vitest.config.ts']`,
9+
so a workspace with no `vitest.config.ts` of its own contributes no project. A test file added to
10+
`packages/react-core` today is collected by nothing, reported by nothing, and passes CI green — the
11+
run never mentions it, and there is no error to search for.
12+
13+
The opposite mistake is loud. Once a project exists, a `--project` filter that matches no `name` stops
14+
the run dead:
15+
16+
```
17+
Error: No projects matched the filter "react-core".
18+
```
19+
20+
That error means the config is absent or its `name` differs from what you typed. Green with no mention
21+
of your file means the config is absent.
22+
23+
## Steps
24+
25+
1. **Confirm the package really has no project.** `pnpm exec vitest list --filesOnly` prints every
26+
collected file in the repo, each prefixed with its project name; the table in
27+
`.agents/docs/architecture/testing-strategy.md` says the same thing in prose. The package must also
28+
sit directly under one of those three roots — a `vitest.config.ts` in `storybook/` or `testing/`
29+
never runs, pnpm workspace or not (`.agents/docs/workspace-map.md`).
30+
31+
2. **Copy `packages/schemas/vitest.config.ts`.** It is the whole pattern, and every other project is
32+
this file with a different `name`:
33+
34+
```ts
35+
import { defineProject, mergeConfig } from 'vitest/config';
36+
37+
import baseConfig from '../../vitest.config';
38+
39+
export default mergeConfig(
40+
baseConfig,
41+
defineProject({
42+
test: {
43+
name: 'schemas',
44+
root: import.meta.dirname
45+
}
46+
})
47+
);
48+
```
49+
50+
Both halves are load-bearing. `mergeConfig(baseConfig, …)` is what inherits the root `include`
51+
globs and `watch: false`; `root: import.meta.dirname` is what scopes those globs to this package
52+
rather than the repo. Every project file uses `defineProject` — `defineConfig` belongs to the root
53+
config alone.
54+
55+
3. **Name the project after the directory.** The `name` field is the string `--project` takes, and it
56+
is the directory basename everywhere except `runtime/v1`, whose project is `runtime-v1` because
57+
`v1` names nothing on its own. Kebab-case, no `@opendatacapture/` scope.
58+
59+
4. **Choose the environment.** Node is the default and needs no key. A test that renders React needs
60+
`environment: 'happy-dom'`, for which `apps/web/vitest.config.ts` is the model. Carry any alias
61+
the package's source imports through into this file under `resolve.alias` — `vite.config.ts` is
62+
not loaded during tests (`apps/web/AGENTS.md`).
63+
64+
`happy-dom` is pinned in the `catalog:` block of `pnpm-workspace.yaml`, so the devDependency is
65+
`"happy-dom": "catalog:"` — confirm the addition in-conversation per the root rule, then run
66+
`pnpm install`. pnpm links dependencies per package, so an uninstalled catalog entry fails when the
67+
environment loads, not at type-check. The DOM caveats it brings are in `apps/web/AGENTS.md` and
68+
`.agents/docs/architecture/testing-strategy.md`; read them before the first assertion.
69+
70+
5. **Add `vitest.config.ts` to the package's `tsconfig.json` `include` array.** `eslint.config.js`
71+
ignores `vitest.config.ts` globally, so `tsc` is the only thing that ever checks it — and only if
72+
the tsconfig names it. Every package with a `lint` script does this; the exceptions are in
73+
`.agents/docs/architecture/testing-strategy.md`.
74+
75+
If the tests will live in `test/` rather than `src/`, add `test/**/*.ts` to `include` too, as
76+
`packages/runtime-bundler` and `packages/runtime-meta` do — a `test/` directory outside the
77+
tsconfig `include` is type-checked by nothing.
78+
79+
6. **Add the `test` script, and do not rely on it.** `"test": "vitest"` under `packages/` and
80+
`runtime/`, `"test": "env-cmd -f ../../.env vitest"` for an app, matching every project that
81+
already exists. Running it does not work today: `mergeConfig` inherits the root `test.projects`
82+
globs along with everything else, and started from inside the package they re-resolve against that
83+
directory and match nothing —
84+
85+
```
86+
Error: No projects were found. Make sure your configuration is correct. The projects definition: […]
87+
```
88+
89+
Scope a run from the repo root instead: `pnpm exec vitest --project <name>`. Root `pnpm test` runs
90+
the projects directly and never reads this script; `vitest` is a root devDependency, so the package
91+
declares nothing.
92+
93+
7. **Place the test file inside the inherited globs.** Those are
94+
`**/*.{test,spec}.?(c|m)[jt]s?(x)` and `**/*/test.?(c|m)[jt]s?(x)`, both relative to the package
95+
`root`; the house convention is `src/__tests__/<subject>.test.ts`. Only reach for a project-level
96+
`include` when a file must live outside them — `mergeConfig` concatenates arrays, so a project
97+
`include` _adds_ to the root globs rather than replacing them. `apps/api` is the proof: its own
98+
`include` is `['src/**/*.spec.ts', 'test/**/*.test.ts']`, and
99+
`src/auth/__tests__/ability.utils.test.ts` matches neither, yet is collected.
100+
101+
8. **Prove the file is collected, then prove it can fail.** If the package imports
102+
`instrument-library`, `runtime-core` or `runtime/v1`, run `pnpm build` once first — those three
103+
resolve only from `dist`/`lib` (`.agents/docs/workspace-map.md`), and `pnpm test` builds nothing.
104+
105+
```sh
106+
pnpm exec vitest list --filesOnly --project <name>
107+
```
108+
109+
Every line must read `[<name>] <path>`, your file must be among them, and no path may point
110+
outside the package — a path from elsewhere in the repo means `root` is missing from step 2. Then
111+
invert one assertion and watch it go red before restoring it
112+
(`.agents/skills/odc-testing/SKILL.md`).
113+
114+
9. **Record the new tier in every doc that tracks it.** The set that moves together is one row in
115+
`.agents/skills/odc-agent-docs/SKILL.md`. Other files also assert your package has no project —
116+
`packages/instrument-interpreter/AGENTS.md` says it of `react-core` — and none of them contains the
117+
new project name, so search by the package name instead:
118+
`grep -rn '<pkg>' --include=AGENTS.md . ; grep -rn '<pkg>' .agents`.
119+
120+
## Verify
121+
122+
```sh
123+
pnpm exec vitest list --filesOnly --project <name> # your file, prefixed [<name>], nothing from outside
124+
pnpm exec vitest --project <name> # green, with the file count you expect
125+
pnpm lint # from the root; tsc now type-checks vitest.config.ts
126+
pnpm test # from the root: the repo-wide file count rises by yours
127+
```
128+
129+
Run `pnpm lint` from the root, with a clean tree: it rewrites files as it checks them
130+
(`.agents/docs/architecture/testing-strategy.md`), and a package-scoped `lint` skips turbo's `^build`
131+
ordering, failing on unbuilt dependencies rather than on your config. `No projects matched the filter`
132+
from either `vitest` command means the `name` you passed is not the `name` in the config; a root
133+
`pnpm test` whose file count is unchanged means the config is not where the globs look — check that
134+
the package sits directly under `apps/`, `packages/` or `runtime/`.

‎.agents/docs/playbooks/cut-a-release.md‎

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
# Cut a release
2+
3+
A release is one version bump merged to `main`. Everything after the merge is automatic:
4+
`.github/workflows/release.yaml` builds and pushes the container images, publishes the npm packages, and
5+
creates the GitHub release. Which packages are publishable is in `.agents/docs/workspace-map.md`; what CI
6+
gates is in `.agents/docs/architecture/testing-strategy.md`; the `RELEASE_VERSION` build-arg contract is in
7+
`packages/release-info/AGENTS.md`. This file is only the order of operations.
8+
9+
**Plan around this:** the root `package.json` version _is_ the release identity —
10+
`.github/scripts/release.cjs` reads it and it becomes both the container tag and the GitHub tag
11+
`v${version}` — but every npm package publishes at **its own** `package.json` version, and the publish
12+
step skips any version already on the registry (`release.yaml:132-139`):
13+
14+
```
15+
Skipping @opendatacapture/runtime-v1@<version> (already published)
16+
```
17+
18+
So a bump that touches only the root leaves the packages behind and `publish-npm` reports **success while
19+
publishing nothing**. `scripts/increment-version.sh` rewrites the root plus every path
20+
`scripts/list-publishable.sh` returns in one run, and it is the only thing holding those files in
21+
agreement — nothing in CI compares them, and hand edits have moved the root alone before.
22+
23+
## Steps
24+
25+
1. **Get the branch green before you bump.** `pnpm lint`, `pnpm test` and `pnpm test:e2e` from the repo
26+
root — nothing after the PR runs a test against this code
27+
(`.agents/docs/architecture/testing-strategy.md` has the gating). All three need `.env`; root
28+
`pnpm test` is bare vitest with no turbo `^build` behind it, so `pnpm build` must have run once
29+
(`.agents/docs/playbooks/run-locally.md`), and the e2e browsers are in
30+
`.agents/docs/playbooks/add-e2e-test.md`. `pnpm lint` is `tsc && eslint --fix src` per package and
31+
rewrites files as it checks them: read `git status` after it and land any churn as its own commit,
32+
before the bump.
33+
34+
2. **Bump from a branch that already contains `main`.** `git fetch origin && git merge origin/main`.
35+
`increment-version.sh` derives the next version from the root `package.json` in your working tree
36+
alone, so a branch trailing `main` computes a version that is already released — the run then re-pushes
37+
the same image tags, updates the same GitHub release, and prints `Skipping` for every package. Done
38+
when `git merge-base --is-ancestor origin/main HEAD` exits 0.
39+
40+
3. **Run `./scripts/increment-version.sh` from the repo root.** There is no `pnpm` script for it — invoke
41+
the path; prerequisites for anything under `scripts/` are in `.agents/docs/playbooks/run-locally.md`.
42+
Its `select` prompt offers `major`/`minor`/`patch`/`quit`, then a `y/N` confirmation, then it rewrites
43+
the root `package.json` plus every path `scripts/list-publishable.sh` returns. Done when its output
44+
carries one `Updated …` line per file and ends `Done! All packages set to <version>`.
45+
46+
4. **Confirm the lockstep before you commit.**
47+
48+
```sh
49+
node -p "require('./package.json').version" && scripts/list-publishable.sh
50+
```
51+
52+
`list-publishable.sh` never prints the root, so a uniform version column proves nothing on its own —
53+
that is exactly what a root-only bump looks like. Done when the root version on the first line equals
54+
the second tab-separated field of every row below it. No check enforces that equality.
55+
56+
5. **Commit the version files in one commit and open the PR with `main` as its base.** `ci.yaml` fires on
57+
`pull_request` to `main` and on `workflow_dispatch`, never on a push, so a PR based on `dev` or any
58+
other branch runs no lint, no unit tests and no e2e — silently. In-repo work branches on origin and
59+
merges into `main`; `CONTRIBUTING.md` describes a fork path, which addresses outside contributors.
60+
There is no changeset and no changelog file: the generated GitHub release is the whole record.
61+
62+
6. **Land one release at a time.** The workflow's concurrency group is per-workflow-per-ref with
63+
`cancel-in-progress: true` (`release.yaml:9-11`), so a second merge cancels the release in flight. A
64+
`build` that is cancelled or fails leaves `publish-npm` and `release` reported as **skipped** rather
65+
than failed — `.agents/skills/odc-release/SKILL.md` reads that job tree; runs `30389796062` and
66+
`30378779338` show that tree.
67+
68+
7. **Watch the run:** `gh run watch`, or `gh run list --workflow=Release --limit 1` for its id.
69+
70+
| Job | What it does | Skips when |
71+
| ------------- | -------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------ |
72+
| `configure` | derives the build matrix from `docker compose config` through the jq filter described below; runs `release.cjs` for `version` and `should_release` | never |
73+
| `validate` | `pnpm lint` | never |
74+
| `build` | buildx `linux/amd64,linux/arm64` per matrix leg, pushing `latest` and the bare version, with `RELEASE_VERSION` as a build arg | `should_release != 'true'` |
75+
| `publish-npm` | turbo-builds each publishable package and its closure, then publishes each version not already on npm, over OIDC (no `NPM_TOKEN`) | `should_release != 'true'` |
76+
| `release` | creates the GitHub release tagged `v${version}` | any of its `needs` skipped or failed |
77+
78+
**No playground image ships.** The filter keeps only compose services declaring **both** `build` and
79+
`image`, and `playground` declares no `image:` key. `scripts/publish.sh` is not the way to add it back:
80+
it `docker push`es the local `:latest` tag of all four images, builds nothing and pushes no version
81+
tag, so it replaces the three CI-published `latest` tags with whatever is in your daemon. It is wired
82+
to no workflow.
83+
84+
**The `v` belongs to GitHub only.** Image tags are pushed bare (`type=raw,value=${version}`); the
85+
leading `v` appears on the GitHub tag alone, and `RELEASE_VERSION` cannot carry one
86+
(`packages/release-info/AGENTS.md`).
87+
88+
8. **Expect a full build even when nothing changed.** `release.cjs` matches GHCR tags against
89+
`/^v(\d+\.\d+\.\d+(-(alpha|beta)\.\d+)?)$/` while `build` pushes unprefixed tags, so
90+
`extractPackageVersionTag` returns `null` and `should_release` is `'true'` on every push to `main`.
91+
Pushing without a bump re-pushes the same image tags and updates the existing GitHub release in place
92+
rather than skipping. Report that as a finding; a release is not the moment to change the release
93+
script.
94+
95+
9. **Confirm all three artifacts carry the new version** — the images, the npm packages, the GitHub
96+
release. Done when the `## Verify` block below is clean for each.
97+
98+
10. **If npm did not move, cut another patch.** A `Skipping` line for every package means the bump
99+
bypassed `scripts/increment-version.sh`. Run the script properly, commit, merge again — re-running a
100+
version the registry already has publishes nothing, so the fix is always forward.
101+
102+
## Verify
103+
104+
```sh
105+
node -p "require('./package.json').version" && scripts/list-publishable.sh # root first, then that same version on every row
106+
gh run list --workflow=Release --limit 1 # the run for your merge commit
107+
gh run view <run-id> --json jobs -q '.jobs[]|"\(.name)\t\(.conclusion)"' # every build leg, publish-npm and release: success
108+
gh run view <run-id> --log | grep -E 'Publishing|Skipping'
109+
npm view @opendatacapture/runtime-v1 version # the version you just cut
110+
gh release view v<version> # exists, tagged with the leading v
111+
```
112+
113+
`Publishing <name>@<version>` for every package is the first run after a bump. `Skipping <name>@<version>
114+
(already published)` is the **correct** output when re-running a release that already published — that
115+
version guard is what makes a re-run safe — and is a defect only on the first run after a bump.
116+
117+
The images have no read-only command of their own:
118+
`gh api /orgs/DouglasNeuroinformatics/packages/container/open-data-capture-api/versions` answers
119+
`403 … read:packages scope` on a token without that scope, which is the token and not a missing image. The
120+
`build` conclusions in the job list above are the check.
121+
122+
Independent of any one release: each app Dockerfile installs its own global `turbo@<version>` for the
123+
image build, separate from the root `turbo` devDependency — move all four pins together so the images
124+
build on one turbo.

0 commit comments

Comments
 (0)