From 9553c910a7171363b6a93e08fa70ae30bf408cf1 Mon Sep 17 00:00:00 2001 From: Five <232717103+mascot-five@users.noreply.github.com> Date: Fri, 24 Jul 2026 01:33:08 -0500 Subject: [PATCH 1/7] fix(NavigationNavbar): widen NavigationItem.title to React.ReactNode NavigationNavbar renders `title` into MUI's `ListItemText` `primary` slot, which is typed `React.ReactNode`. Declaring `title` as `string` meant composed labels - a label plus a trailing external-link glyph, a badge, a count - rendered correctly at runtime but did not type-check, forcing consumers into an `as unknown as string` assertion. The widening is source-compatible: `string` is assignable to `ReactNode`, so no existing consumer breaks. Sub-items share the same type and gain the same latitude. Adds a type-level regression guard. jest transforms with @swc/jest, which strips types without checking them, so the test shells out to `tsc` over a fixture that exercises a plain string title, a composed ReactNode title, and a nested sub-item title. Reverting the widening makes it fail. Verified through the built artifact, not just source: `dist/index.d.ts` and `dist/index.d.mts` both emit `title: React__default.ReactNode`, and a clean consumer installing the packed tarball type-checks both title forms, where the same consumer against published v0.21.42 errors with "Type 'Element' is not assignable to type 'string'". Fixes #1746 Signed-off-by: Five <232717103+mascot-five@users.noreply.github.com> --- .../fixtures/navigationItemTitle.tsx | 54 +++++++++++++++++++ .../tsconfig.navigationItemTitle.json | 7 +++ .../navigationItemTitleTypes.test.ts | 52 ++++++++++++++++++ .../NavigationNavbar/navigationNavbar.tsx | 9 +++- 4 files changed, 121 insertions(+), 1 deletion(-) create mode 100644 src/__testing__/fixtures/navigationItemTitle.tsx create mode 100644 src/__testing__/fixtures/tsconfig.navigationItemTitle.json create mode 100644 src/__testing__/navigationItemTitleTypes.test.ts diff --git a/src/__testing__/fixtures/navigationItemTitle.tsx b/src/__testing__/fixtures/navigationItemTitle.tsx new file mode 100644 index 000000000..fa9378ba7 --- /dev/null +++ b/src/__testing__/fixtures/navigationItemTitle.tsx @@ -0,0 +1,54 @@ +// Type-level fixture for `NavigationItem['title']`. It is compiled by +// `navigationItemTitleTypes.test.ts` via `tsc --noEmit`, never executed, and is +// deliberately outside jest's `*.test.*` glob so jest does not collect it. +// +// `NavigationNavbar` renders `title` into MUI's `ListItemText` `primary` slot, +// which is typed `React.ReactNode`. Declaring `title` as `string` therefore +// rejected composed labels that already rendered correctly - see +// https://github.com/layer5io/sistent/issues/1746. +import OpenInNewIcon from '@mui/icons-material/OpenInNew'; +import type { NavigationItem } from '../../custom/NavigationNavbar'; + +// A plain string still compiles - the widening is source-compatible. +export const plainStringTitle: NavigationItem = { + id: 'settings', + title: 'Settings', + onClick: () => {} +}; + +// A composed ReactNode title compiles: the exact shape that previously forced +// consumers into an `as unknown as string` assertion. +export const composedNodeTitle: NavigationItem = { + id: 'api-docs', + title: ( + + API Docs + + + ), + onClick: () => {} +}; + +// Sub-items share the type, so they gain the same latitude. +export const composedSubItemTitle: NavigationItem = { + id: 'docs', + title: 'Docs', + onClick: () => {}, + subItems: [ + { + id: 'docs-api', + title: API, + onClick: () => {} + } + ] +}; + +// Self-check: if the compiler ever stops type-checking this fixture, the +// suppression below goes unused and tsc reports TS2578 here, failing the test +// rather than letting it pass vacuously. A plain object is not a `ReactNode`. +export const rejectsNonNode: NavigationItem = { + id: 'bad', + // @ts-expect-error - a plain object is not assignable to React.ReactNode + title: { label: 'not a node' }, + onClick: () => {} +}; diff --git a/src/__testing__/fixtures/tsconfig.navigationItemTitle.json b/src/__testing__/fixtures/tsconfig.navigationItemTitle.json new file mode 100644 index 000000000..e84a46afd --- /dev/null +++ b/src/__testing__/fixtures/tsconfig.navigationItemTitle.json @@ -0,0 +1,7 @@ +{ + "extends": "../../../tsconfig.json", + "compilerOptions": { + "noEmit": true + }, + "include": ["./navigationItemTitle.tsx"] +} diff --git a/src/__testing__/navigationItemTitleTypes.test.ts b/src/__testing__/navigationItemTitleTypes.test.ts new file mode 100644 index 000000000..c6935daa4 --- /dev/null +++ b/src/__testing__/navigationItemTitleTypes.test.ts @@ -0,0 +1,52 @@ +import { execFileSync } from 'child_process'; +import path from 'path'; + +// Type-level regression guard for https://github.com/layer5io/sistent/issues/1746. +// +// `NavigationNavbar` renders `NavigationItem['title']` into MUI's `ListItemText` +// `primary` slot, which is typed `React.ReactNode`. While `title` was declared +// `string`, composed labels rendered correctly but did not type-check, and +// consumers reached for `as unknown as string`. +// +// jest transforms with @swc/jest, which strips types without checking them, so a +// plain type-only assertion file would pass no matter what `title` is declared +// as. This test therefore shells out to `tsc` over a fixture that exercises both +// a plain string and a composed `ReactNode` title. +// +// Diagnostics are filtered to the fixture itself: type-checking it pulls in the +// component's transitive dependencies, which carry pre-existing errors unrelated +// to this contract (see CLAUDE.md, "Repo state that looks broken but is +// pre-existing"). The fixture's own `@ts-expect-error` self-check fails loudly if +// the compiler ever stops checking it, so filtering cannot make this vacuous. + +const FIXTURE = 'src/__testing__/fixtures/navigationItemTitle.tsx'; +const PROJECT = 'src/__testing__/fixtures/tsconfig.navigationItemTitle.json'; +const repoRoot = path.resolve(__dirname, '..', '..'); + +const typeCheckFixture = (): string[] => { + let output: string; + try { + output = execFileSync('npx', ['tsc', '-p', PROJECT], { + cwd: repoRoot, + encoding: 'utf8', + stdio: ['ignore', 'pipe', 'pipe'] + }); + } catch (error) { + // tsc exits non-zero whenever any file in the program has an error, including + // the pre-existing ones outside the fixture. Read its stdout rather than + // treating the exit code as the verdict. + const spawned = error as { stdout?: string; stderr?: string; message?: string }; + output = spawned.stdout ?? spawned.stderr ?? spawned.message ?? ''; + } + + return output + .split('\n') + .map((line) => line.trim()) + .filter((line) => line.startsWith(FIXTURE)); +}; + +describe('NavigationItem title type contract', () => { + it('accepts both a plain string and a composed ReactNode title', () => { + expect(typeCheckFixture()).toEqual([]); + }, 180_000); +}); diff --git a/src/custom/NavigationNavbar/navigationNavbar.tsx b/src/custom/NavigationNavbar/navigationNavbar.tsx index 2bb024c4a..6d59537d4 100644 --- a/src/custom/NavigationNavbar/navigationNavbar.tsx +++ b/src/custom/NavigationNavbar/navigationNavbar.tsx @@ -9,7 +9,14 @@ import { IconWrapper, MenuItemList, MenuItemSubList, MenuListStyle, SubIconWrapp export type NavigationItem = { id: string; - title: string; + /** + * Label rendered into `ListItemText`'s `primary` slot. + * + * Accepts any `React.ReactNode`, not just a string, so composed labels - + * a label plus a trailing external-link glyph, a `` badge, a count - + * type-check as well as they already render. + */ + title: React.ReactNode; icon?: React.ReactNode; /** * Legacy boolean permission flag. From 24e341027319ddf87c85df101d8415720ff5a7e9 Mon Sep 17 00:00:00 2001 From: Ahmed Jamil <197998703+CodeAhmedJamil@users.noreply.github.com> Date: Fri, 24 Jul 2026 03:12:27 -0500 Subject: [PATCH 2/7] no-mistakes(review): make navigationItemTitle tsc guard fail closed Signed-off-by: Ahmed Jamil <197998703+CodeAhmedJamil@users.noreply.github.com> --- .../navigationItemTitleTypes.test.ts | 86 +++++++++++++++---- 1 file changed, 67 insertions(+), 19 deletions(-) diff --git a/src/__testing__/navigationItemTitleTypes.test.ts b/src/__testing__/navigationItemTitleTypes.test.ts index c6935daa4..2ad235630 100644 --- a/src/__testing__/navigationItemTitleTypes.test.ts +++ b/src/__testing__/navigationItemTitleTypes.test.ts @@ -1,4 +1,4 @@ -import { execFileSync } from 'child_process'; +import { spawnSync } from 'child_process'; import path from 'path'; // Type-level regression guard for https://github.com/layer5io/sistent/issues/1746. @@ -18,35 +18,83 @@ import path from 'path'; // to this contract (see CLAUDE.md, "Repo state that looks broken but is // pre-existing"). The fixture's own `@ts-expect-error` self-check fails loudly if // the compiler ever stops checking it, so filtering cannot make this vacuous. +// +// Everything below the filter exists so that the guard fails closed. Filtering +// is the one place a compiler check can quietly turn into a no-op: every outcome +// that is not "tsc ran, compiled the fixture, and reported on it" has to be made +// to fail rather than collapse into an empty, green list of diagnostics. const FIXTURE = 'src/__testing__/fixtures/navigationItemTitle.tsx'; const PROJECT = 'src/__testing__/fixtures/tsconfig.navigationItemTitle.json'; const repoRoot = path.resolve(__dirname, '..', '..'); -const typeCheckFixture = (): string[] => { - let output: string; - try { - output = execFileSync('npx', ['tsc', '-p', PROJECT], { +// `(,): error TS….` The file part is what scopes a diagnostic, +// so it is parsed rather than prefix-matched: tsc prints paths relative to its +// cwd only while the fixture stays under it, and a bare `startsWith` would read +// any other emission as "the fixture is clean". +const FILE_SCOPED_DIAGNOSTIC = /^(.+?)\(\d+,\d+\): error TS\d+/; + +// `error TS…` with no file part is a config-level failure - TS18003 "No inputs +// were found in config file", TS5083 "Cannot read file" - and means the fixture +// was never compiled at all. The fixture filter drops these on the floor, and +// the fixture's `@ts-expect-error` self-check cannot see them either, because +// neither fires unless the compiler had the fixture in its program. +const CONFIG_SCOPED_DIAGNOSTIC = /^error TS\d+/; + +type Diagnostics = { fixture: string[]; config: string[] }; + +const typeCheckFixture = (): Diagnostics => { + // Spawned through `process.execPath` rather than `npx`: no shell, no PATH + // lookup, and no `npx` vs `npx.cmd` divergence on Windows, where an ENOENT + // would otherwise be caught and laundered into the diagnostic output. + const tsc = spawnSync( + process.execPath, + [require.resolve('typescript/bin/tsc'), '-p', PROJECT, '--pretty', 'false'], + { cwd: repoRoot, encoding: 'utf8', - stdio: ['ignore', 'pipe', 'pipe'] - }); - } catch (error) { - // tsc exits non-zero whenever any file in the program has an error, including - // the pre-existing ones outside the fixture. Read its stdout rather than - // treating the exit code as the verdict. - const spawned = error as { stdout?: string; stderr?: string; message?: string }; - output = spawned.stdout ?? spawned.stderr ?? spawned.message ?? ''; + // tsc reports on every file in the program, and the fixture's transitive + // dependencies are a large share of `src/`. The 1 MB default would kill + // the child mid-stream and truncate stdout - and the fixture is a root + // file, so its diagnostics are emitted last and lost first. + maxBuffer: 32 * 1024 * 1024 + } + ); + + // A compiler that never ran is not a compiler that found nothing. + if (tsc.error) throw tsc.error; + if (tsc.status === null) { + throw new Error(`tsc was killed by ${tsc.signal ?? 'an unknown signal'} before reporting`); } - return output - .split('\n') - .map((line) => line.trim()) - .filter((line) => line.startsWith(FIXTURE)); + // tsc exits non-zero whenever any file in the program has an error, including + // the pre-existing ones outside the fixture, so the exit code is not the + // verdict here - the diagnostics are. + const lines = `${tsc.stdout}\n${tsc.stderr}`.split('\n').map((line) => line.trim()); + + const belongsToFixture = (line: string): boolean => { + const file = FILE_SCOPED_DIAGNOSTIC.exec(line)?.[1]; + return file !== undefined && file.replace(/\\/g, '/').endsWith(FIXTURE); + }; + + return { + fixture: lines.filter(belongsToFixture), + config: lines.filter((line) => CONFIG_SCOPED_DIAGNOSTIC.test(line)) + }; }; describe('NavigationItem title type contract', () => { - it('accepts both a plain string and a composed ReactNode title', () => { - expect(typeCheckFixture()).toEqual([]); + let diagnostics: Diagnostics; + + beforeAll(() => { + diagnostics = typeCheckFixture(); }, 180_000); + + it('compiles the fixture at all', () => { + expect(diagnostics.config).toEqual([]); + }); + + it('accepts both a plain string and a composed ReactNode title', () => { + expect(diagnostics.fixture).toEqual([]); + }); }); From 84263f2df81111cbefbf15367e47c36e9131fc15 Mon Sep 17 00:00:00 2001 From: Ahmed Jamil <197998703+CodeAhmedJamil@users.noreply.github.com> Date: Fri, 24 Jul 2026 03:32:53 -0500 Subject: [PATCH 3/7] no-mistakes(document): document tsc-in-jest type guard pattern in AGENTS.md Signed-off-by: Ahmed Jamil <197998703+CodeAhmedJamil@users.noreply.github.com> --- AGENTS.md | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/AGENTS.md b/AGENTS.md index 7d51e61f9..88ee37b6d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -37,7 +37,7 @@ Two complementary guards, and you need both: scans `src/` for every load-time form (`import from`, `export ... from`, bare `import`, `require`) of an enforced optional peer, and fails `jest` if one appears. `await import()` is deliberately not matched - deferring resolution is the fix, not the defect. That file is also the source of truth for -*which* optional peers are enforced: `react` / `react-dom` are marked optional in `package.json` but +_which_ optional peers are enforced: `react` / `react-dom` are marked optional in `package.json` but exempted there on the record, because every component imports React at module scope and always will. **2. Built artifact, by hand.** The CI guard reads source, so it cannot speak for what the bundler @@ -84,6 +84,12 @@ missing `@types/jest` wiring for `src/__testing__`). Neither is a CI gate - CI r `.github/workflows/node-checks.yml` (lint + build) and `jest`. Do not assume you caused these; do not mass-reformat to "fix" them. +A type contract can still be gated, just not by a type-only assertion file: jest transforms with +`@swc/jest`, which strips types without checking them, so such a file passes no matter what it +asserts. Shell out to `tsc` over a scoped fixture and filter the diagnostics to that fixture - +[`src/__testing__/navigationItemTitleTypes.test.ts`](src/__testing__/navigationItemTitleTypes.test.ts) +is the worked pattern, including the checks that keep the filter from turning the guard vacuous. + ## Maintaining this file Keep this file for knowledge useful to almost every future agent session in this project. From 7bbd797386d3c51beeede97a25f846eafbc34a08 Mon Sep 17 00:00:00 2001 From: Ahmed Jamil <197998703+CodeAhmedJamil@users.noreply.github.com> Date: Fri, 24 Jul 2026 03:47:10 -0500 Subject: [PATCH 4/7] docs(agents): record DCO sign-off requirement and repair recipe The DCO app blocked #1748: the two follow-up commits on the branch carried no `Signed-off-by` trailer while the primary commit did. Nothing in the tree surfaces that requirement - DCO is a GitHub App with no workflow file under `.github/`, and `.husky/commit-msg` was deliberately emptied in `612608be`, so there is no local gate either. Records the three non-obvious parts: the trailer must match the commit's own author identity, follow-up commits are the usual offender, and the repair rebase must be scoped to your own commits so a co-author's commit does not pick up a second sign-off. Points at CONTRIBUTING.md for the policy itself. Signed-off-by: Ahmed Jamil <197998703+CodeAhmedJamil@users.noreply.github.com> --- AGENTS.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index 88ee37b6d..2d28672b7 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -90,6 +90,21 @@ asserts. Shell out to `tsc` over a scoped fixture and filter the diagnostics to [`src/__testing__/navigationItemTitleTypes.test.ts`](src/__testing__/navigationItemTitleTypes.test.ts) is the worked pattern, including the checks that keep the filter from turning the guard vacuous. +## Every commit needs a sign-off matching its own author + +DCO is a blocking required check, enforced by the [probot/dco](https://github.com/probot/dco) GitHub +App - there is no workflow file for it under `.github/`, so it is invisible from the tree until a PR +fails. Policy is in [`CONTRIBUTING.md`](CONTRIBUTING.md#commit-signing); the sharp edges are: + +- The trailer must match that commit's **author** identity, so `git commit -s` under a + `user.email` that differs from the author still fails. `.husky/commit-msg` is deliberately empty + (commit `612608be`) - nothing catches this locally. +- The usual offender is a follow-up commit (review fix, doc update, rebase fixup), not the first one. + Sign every commit you add, including the ones an automated pass makes for you. +- To repair, rebase only your own commits: `git rebase --signoff `, then + force-push. Rebasing the whole branch grafts your sign-off onto a co-author's commit as a second + trailer. + ## Maintaining this file Keep this file for knowledge useful to almost every future agent session in this project. From b9161afb7b7246c712699e267308f75a61fdcb50 Mon Sep 17 00:00:00 2001 From: Five <232717103+mascot-five@users.noreply.github.com> Date: Fri, 24 Jul 2026 04:01:32 -0500 Subject: [PATCH 5/7] test(NavigationNavbar): prove the fixture compiled before trusting its diagnostics Addresses two review findings on the type guard added in this branch. An empty diagnostic list only means nothing was wrong with what tsc compiled; it says nothing about *what* it compiled. A project whose `include` resolves to some other existing file emits no diagnostic at all, so both the fixture-scoped and config-scoped lists came back empty and the guard passed green without ever looking at the fixture - reproduced by pointing the fixture tsconfig at another file, which passed in 1.6s. Run tsc with `--listFiles` and assert the fixture is in the emitted program, making its presence a checked fact rather than an assumption. The two diagnostic assertions are ordered after it, since they are only meaningful once the fixture is known to have been compiled. Also bound the compiler call with a 150s child timeout, below the 180s `beforeAll` budget. `spawnSync` blocks the jest worker outright and jest's own timeout cannot preempt a synchronous child, so a wedged tsc would have held the worker until the entire run was killed. Node signals the child on timeout, which surfaces through the existing `status === null` throw. Verified: the guard now fails when the fixture is absent from the program, and still fails when the widening is reverted. Signed-off-by: Five <232717103+mascot-five@users.noreply.github.com> --- .../navigationItemTitleTypes.test.ts | 43 +++++++++++++++---- 1 file changed, 34 insertions(+), 9 deletions(-) diff --git a/src/__testing__/navigationItemTitleTypes.test.ts b/src/__testing__/navigationItemTitleTypes.test.ts index 2ad235630..416d298b9 100644 --- a/src/__testing__/navigationItemTitleTypes.test.ts +++ b/src/__testing__/navigationItemTitleTypes.test.ts @@ -41,7 +41,21 @@ const FILE_SCOPED_DIAGNOSTIC = /^(.+?)\(\d+,\d+\): error TS\d+/; // neither fires unless the compiler had the fixture in its program. const CONFIG_SCOPED_DIAGNOSTIC = /^error TS\d+/; -type Diagnostics = { fixture: string[]; config: string[] }; +// Absence of diagnostics only means "nothing was wrong with what tsc compiled" - +// it says nothing about *what* tsc compiled. A project whose `include` resolves +// to some other existing file emits no diagnostic at all, so both lists above +// come back empty and the guard passes without ever having looked at the +// fixture. `--listFiles` closes that by making the program's contents an +// assertable fact instead of an assumption. +const asPosix = (value: string): string => value.replace(/\\/g, '/'); + +type Diagnostics = { fixture: string[]; config: string[]; compiledFixture: boolean }; + +// Below the 180s `beforeAll` budget. `spawnSync` blocks the jest worker outright, +// and jest's own timeout cannot preempt a synchronous child, so a wedged tsc +// would otherwise hold the worker until the whole run is killed. Node's timeout +// signals the child, which surfaces as the `status === null` throw below. +const TSC_TIMEOUT_MS = 150_000; const typeCheckFixture = (): Diagnostics => { // Spawned through `process.execPath` rather than `npx`: no shell, no PATH @@ -49,14 +63,15 @@ const typeCheckFixture = (): Diagnostics => { // would otherwise be caught and laundered into the diagnostic output. const tsc = spawnSync( process.execPath, - [require.resolve('typescript/bin/tsc'), '-p', PROJECT, '--pretty', 'false'], + [require.resolve('typescript/bin/tsc'), '-p', PROJECT, '--pretty', 'false', '--listFiles'], { cwd: repoRoot, encoding: 'utf8', - // tsc reports on every file in the program, and the fixture's transitive - // dependencies are a large share of `src/`. The 1 MB default would kill - // the child mid-stream and truncate stdout - and the fixture is a root - // file, so its diagnostics are emitted last and lost first. + timeout: TSC_TIMEOUT_MS, + // tsc reports on every file in the program, and `--listFiles` prints each + // one, so this runs to several MB. The 1 MB default would kill the child + // mid-stream and truncate stdout - and the fixture is a root file, so its + // diagnostics are emitted last and lost first. maxBuffer: 32 * 1024 * 1024 } ); @@ -74,12 +89,16 @@ const typeCheckFixture = (): Diagnostics => { const belongsToFixture = (line: string): boolean => { const file = FILE_SCOPED_DIAGNOSTIC.exec(line)?.[1]; - return file !== undefined && file.replace(/\\/g, '/').endsWith(FIXTURE); + return file !== undefined && asPosix(file).endsWith(FIXTURE); }; return { fixture: lines.filter(belongsToFixture), - config: lines.filter((line) => CONFIG_SCOPED_DIAGNOSTIC.test(line)) + config: lines.filter((line) => CONFIG_SCOPED_DIAGNOSTIC.test(line)), + // `--listFiles` prints one absolute path per program file, diagnostics aside. + compiledFixture: lines.some( + (line) => !FILE_SCOPED_DIAGNOSTIC.test(line) && asPosix(line).endsWith(FIXTURE) + ) }; }; @@ -90,7 +109,13 @@ describe('NavigationItem title type contract', () => { diagnostics = typeCheckFixture(); }, 180_000); - it('compiles the fixture at all', () => { + // Ordered deliberately: the two assertions below are only meaningful once the + // fixture is known to have been compiled, so prove that first. + it('compiles the fixture', () => { + expect(diagnostics.compiledFixture).toBe(true); + }); + + it('reports no config-level failure that would skip the fixture', () => { expect(diagnostics.config).toEqual([]); }); From d852654256b92611755c3190dc3333c060611ea6 Mon Sep 17 00:00:00 2001 From: Ahmed Jamil <197998703+CodeAhmedJamil@users.noreply.github.com> Date: Fri, 24 Jul 2026 04:22:57 -0500 Subject: [PATCH 6/7] no-mistakes(document): sync AGENTS.md tsc-guard recipe and stale prettier count Signed-off-by: Ahmed Jamil <197998703+CodeAhmedJamil@users.noreply.github.com> --- AGENTS.md | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 2d28672b7..8aa315213 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -79,15 +79,16 @@ of examples there, e.g. `FeedbackButton`, `NavigationItem`). Verify by building ## Repo state that looks broken but is pre-existing -`prettier --check` fails on ~82 files and `tsc --noEmit` reports errors across `src/` (including -missing `@types/jest` wiring for `src/__testing__`). Neither is a CI gate - CI runs +`prettier --check` fails on dozens of files and `tsc --noEmit` reports errors across `src/` +(including missing `@types/jest` wiring for `src/__testing__`). Neither is a CI gate - CI runs `.github/workflows/node-checks.yml` (lint + build) and `jest`. Do not assume you caused these; do not mass-reformat to "fix" them. A type contract can still be gated, just not by a type-only assertion file: jest transforms with `@swc/jest`, which strips types without checking them, so such a file passes no matter what it -asserts. Shell out to `tsc` over a scoped fixture and filter the diagnostics to that fixture - -[`src/__testing__/navigationItemTitleTypes.test.ts`](src/__testing__/navigationItemTitleTypes.test.ts) +asserts. Shell out to `tsc` over a scoped fixture, assert the fixture is in the compiled program +(`--listFiles`) before trusting an empty diagnostic list, then filter the diagnostics to that +fixture - [`src/__testing__/navigationItemTitleTypes.test.ts`](src/__testing__/navigationItemTitleTypes.test.ts) is the worked pattern, including the checks that keep the filter from turning the guard vacuous. ## Every commit needs a sign-off matching its own author From 788c2b222a100ccc9b3bd0f0d01d1000ce934611 Mon Sep 17 00:00:00 2001 From: Ahmed Jamil <197998703+CodeAhmedJamil@users.noreply.github.com> Date: Fri, 24 Jul 2026 09:30:37 -0500 Subject: [PATCH 7/7] no-mistakes(document): move DCO sign-off mechanics to CONTRIBUTING.md, pointer in AGENTS.md Signed-off-by: Ahmed Jamil <197998703+CodeAhmedJamil@users.noreply.github.com> --- AGENTS.md | 19 +++++++------------ CONTRIBUTING.md | 32 +++++++++++++++++++++++++++----- 2 files changed, 34 insertions(+), 17 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 8aa315213..219530ba6 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -93,18 +93,13 @@ is the worked pattern, including the checks that keep the filter from turning th ## Every commit needs a sign-off matching its own author -DCO is a blocking required check, enforced by the [probot/dco](https://github.com/probot/dco) GitHub -App - there is no workflow file for it under `.github/`, so it is invisible from the tree until a PR -fails. Policy is in [`CONTRIBUTING.md`](CONTRIBUTING.md#commit-signing); the sharp edges are: - -- The trailer must match that commit's **author** identity, so `git commit -s` under a - `user.email` that differs from the author still fails. `.husky/commit-msg` is deliberately empty - (commit `612608be`) - nothing catches this locally. -- The usual offender is a follow-up commit (review fix, doc update, rebase fixup), not the first one. - Sign every commit you add, including the ones an automated pass makes for you. -- To repair, rebase only your own commits: `git rebase --signoff `, then - force-push. Rebasing the whole branch grafts your sign-off onto a co-author's commit as a second - trailer. +DCO is a blocking required check with nothing enforcing it locally, and the trailer must match that +commit's own author identity. The rule, the enforcement, and the repair recipe live in +[`CONTRIBUTING.md`](CONTRIBUTING.md#commit-signing) - read it before rewriting history. + +The agent-specific trap: the usual offender is a follow-up commit (review fix, doc update, rebase +fixup), not the first one. Sign every commit you add, including the ones an automated pass makes +for you. ## Maintaining this file diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 51ca9ed51..03f6898e8 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -40,6 +40,11 @@ address (sorry, no pseudonyms or anonymous contributions). An example of signing $ commit -s -m “my commit message w/signoff” ``` +The trailer must match that commit's **own author identity**. `-s` copies your current +`user.name` / `user.email` into the trailer, so a commit authored under a different identity - a +repo-local override, a second machine, a co-author's commit you replayed - still fails the check +even though a `Signed-off-by:` line is present. + To ensure all your commits are signed, you may choose to add this alias to your global `.gitconfig`: _~/.gitconfig_ @@ -55,6 +60,23 @@ Or you may configure your IDE, for example, Visual Studio Code to automatically +### Repairing a failed DCO check + +DCO is a blocking required check, enforced by the [probot/dco](https://github.com/probot/dco) +GitHub App rather than by a workflow - there is no file for it under `.github/`, so it is invisible +from the repository tree until a pull request fails. `.husky/commit-msg` is deliberately empty +(commit `612608be`), so nothing catches a missing or mismatched sign-off locally either. + +To repair, rebase only the commits you authored, then force-push: + +``` +git rebase --signoff +git push --force-with-lease +``` + +Pick that starting point deliberately. `--signoff` adds your trailer to every commit it replays, so +rebasing the whole branch grafts your sign-off onto a co-author's commit as a second trailer. + ## Documentation Contribution Flow Please contribute! Layer5 documentation uses Jekyll and GitHub Pages to host docs sites. Learn more about [Layer5's documentation framework](https://docs.google.com/document/d/17guuaxb0xsfutBCzyj2CT6OZiFnMu9w4PzoILXhRXSo/edit?usp=sharing). The process of contributing follows this flow: @@ -155,10 +177,10 @@ make lint `src/custom/ResponsiveDataTable.tsx` wraps `@mui/x-data-grid` (via `@layer5/mui-datatables`) with responsive column visibility and a standardised row action menu. Key exports: -| Export | Description | -|---|---| -| `ResponsiveDataTable` | The main table component | -| `TableAction` | Type for items in the per-row action menu | +| Export | Description | +| ----------------------- | ---------------------------------------------- | +| `ResponsiveDataTable` | The main table component | +| `TableAction` | Type for items in the per-row action menu | | `getCopyDeepLinkAction` | Helper that builds a "Copy link" `TableAction` | **`getCopyDeepLinkAction`** @@ -171,7 +193,7 @@ import { getCopyDeepLinkAction, TableAction } from '@sistent/sistent'; const rowActions: TableAction[] = [ getCopyDeepLinkAction(() => copyRowDeepLink(row.id)), // or, with a custom title: - getCopyDeepLinkAction(() => copyRowDeepLink(row.id), t('Copy link')), + getCopyDeepLinkAction(() => copyRowDeepLink(row.id), t('Copy link')) ]; ```