feat: complete greenfield phase 0 substrate - #108
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (17)
💤 Files with no reviewable changes (2)
🚧 Files skipped from review as they are similar to previous changes (15)
📝 WalkthroughWalkthroughThe repository now contains four private Phase 0 packages: canonical codec, runtime contracts, conformance evaluation, and authority kernel. Workspace wiring, strict repository validators, fixtures, tests, and Greenfield Phase 0 evidence generation were updated accordingly. ChangesPhase 0 workspace substrate
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant CI
participant RepositoryChecks
participant Packages
participant EvidenceWriter
CI->>RepositoryChecks: run pnpm check
RepositoryChecks->>Packages: typecheck and test workspace packages
Packages-->>RepositoryChecks: validation results
RepositoryChecks-->>CI: check status
CI->>EvidenceWriter: write Phase 0 evidence
EvidenceWriter-->>CI: artifacts/phase-0/evidence.json
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
d68d77b to
12049e0
Compare
12049e0 to
bfe4a6d
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 232523152b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (21)
packages/codec/tsconfig.json (1)
6-6: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
"DOM"lib widens the ambient surface for a pure package.The package only needs
TextEncoder/TextDecoder, which@types/nodeprovides. IncludingDOMmakesfetch,document,localStorage, etc. type-visible, which conflicts with the pure-package/ambient-capability boundary checks this stack introduces. Consider droppingDOMand relying on Node types.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/codec/tsconfig.json` at line 6, Remove "DOM" from the lib array in the codec TypeScript configuration, retaining only the ES2022 library and relying on the existing `@types/node` declarations for TextEncoder and TextDecoder.tests/codec/codec.test.mjs (1)
95-101: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the arity the test name promises.
Nothing enforces "22" — if a constructor is dropped from
tests/fixtures/codec-corpus.json, this loop still passes vacuously. Same applies to the identity loop at Lines 70-71.💚 Proposed change
+ assert.equal(corpus.constructors.length, 22); for (const entry of corpus.constructors)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/codec/codec.test.mjs` around lines 95 - 101, Ensure the formatter test explicitly asserts that corpus.constructors contains exactly 22 entries before iterating, and add the equivalent count assertion for the identity loop’s constructor collection. Keep the existing per-entry assertions unchanged so the tests still validate every supplied canonical form.packages/codec/src/index.ts (2)
97-106: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low valueKey ordering is code-point based, not UTF-16 code-unit based.
RFC 8785 (JCS) sorts object keys by UTF-16 code units. This comparator sorts by code point, so keys containing non-BMP characters order differently from any JCS-compatible implementation. Self-consistent within this package, but cross-runtime golden vectors produced elsewhere would disagree. If the divergence is intentional, a short comment stating that
jig.codec.v1is not JCS would prevent future confusion.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/codec/src/index.ts` around lines 97 - 106, Update compareKeys to compare string characters by UTF-16 code units rather than spreading into Unicode code points, preserving lexicographic ordering and length fallback. If the code-point ordering is intentional, instead document near compareKeys that jig.codec.v1 is not JCS-compatible; otherwise implement the UTF-16 ordering required for cross-runtime compatibility.
502-506: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRe-derive the same-run check from the shared grammar constants.
The inline literal duplicates the
runconstant defined at Line 56; a future change to the run grammar silently desynchronizes this check frompatterns.♻️ Suggested refactor
- const runs = /^(run-[0-9]{12}-[0-9a-f]{16})\/txn\/[0-9]+\/(run-[0-9]{12}-[0-9a-f]{16})\/gen\//.exec(value); + const runs = new RegExp(`^(${run})/txn/${ordinal}/(${run})/gen/`).exec(value);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/codec/src/index.ts` around lines 502 - 506, Update the same-run validation in the ID-TXN/ID-OP branch to derive its regular expression from the shared run grammar constant and existing patterns utilities, rather than duplicating the inline run literal. Preserve the current capture comparison and INVALID_SCOPE behavior, keeping the check synchronized with the run definition.tests/codec/corpus.test.mjs (2)
92-96: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPositional case indices make the tamper matrix brittle.
cases[0],cases[7],cases[1]silently target the wrong case if the corpus is reordered — and a mutation that lands on a case withoutcanonicalBytesSha256/stagedDigestwould make the assertion pass for the wrong reason. Select byid(valid-canonical,digest-bound,malformed) instead.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/codec/corpus.test.mjs` around lines 92 - 96, Select mutation targets by their stable case IDs in the tamper matrix instead of positional indexes: use valid-canonical for canonicalBytesSha256, digest-bound for stagedDigest, and malformed for result.error.code. Preserve the existing mutations while ensuring each field is modified on the intended corpus case.
79-79: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTest depends on cwd and a hardcoded
/tmp.
'tests/codec/golden-consumer.mjs'resolves againstprocess.cwd(), so the test only passes when the runner is invoked from the repository root; andmkdtempSync('/tmp/...')ignoresTMPDIRand is not portable. Both are easy to make self-locating.♻️ Proposed change
-import { join } from 'node:path'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const consumer = fileURLToPath(new URL('./golden-consumer.mjs', import.meta.url));- const result = spawnSync(process.execPath, ['tests/codec/golden-consumer.mjs'], { encoding: 'utf8' }); + const result = spawnSync(process.execPath, [consumer], { encoding: 'utf8' });- const root = mkdtempSync('/tmp/codec-corpus-'); + const root = mkdtempSync(join(tmpdir(), 'codec-corpus-'));- const result = spawnSync(process.execPath, ['tests/codec/golden-consumer.mjs', path], { encoding: 'utf8' }); + const result = spawnSync(process.execPath, [consumer, path], { encoding: 'utf8' });Also applies to: 90-90, 109-109
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/codec/corpus.test.mjs` at line 79, Update the test’s spawnSync calls to resolve golden-consumer.mjs relative to the test file rather than process.cwd(), and replace hardcoded /tmp mkdtempSync prefixes with a portable temporary-directory source such as os.tmpdir(). Apply the same self-locating changes to the occurrences around the referenced lines.tests/codec/golden-consumer.mjs (1)
25-31: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueDispatch on key presence rather than truthiness.
An empty-string
frame(a natural hostile-input case to add later) is falsy and would fall through tovalidateStagedDigest(undefined).Object.hasOwn(entry, 'frame')keeps the dispatch faithful to the fixture shape.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/codec/golden-consumer.mjs` around lines 25 - 31, Update actualCase to dispatch based on whether each fixture key exists, not whether its value is truthy; use own-property checks for frame, generator, identity, and staged so empty-string values still reach their intended handlers and unsupported entries continue throwing.packages/authority-kernel/tsconfig.json (1)
6-6: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDrop
DOMfromlibfor this package.
src/index.tsuses no DOM APIs, and includingDOMmakes ambient browser capabilities (fetch,document, timers) type-visible inside a package that the boundary checker is meant to keep free of unbound ambient-capability reads.["ES2022"]is sufficient here.♻️ Proposed change
- "lib": ["ES2022", "DOM"], + "lib": ["ES2022"],🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/authority-kernel/tsconfig.json` at line 6, Update the TypeScript compiler options in tsconfig.json by removing "DOM" from the lib array, leaving only "ES2022" so browser ambient APIs are not type-visible in this package.tests/runtime-contracts/topology.test.mjs (2)
175-177: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the crossing lookup succeeded before encoding it.
Only
PORT-DECIDE,PORT-SESSION,PORT-WORKSPACE,PORT-VERIFY,PORT-DELIVERY,PORT-LEDGER, andPORT-ARTIFACThave anRT-CONTROLLER-sourced crossing intests/fixtures/runtime-topology.json. Iftests/fixtures/runtime-fakes.jsonever lists any other port,crossingisundefinedand the failure surfaces as an opaque crash insideencodedCrossinginstead of a diagnosable assertion.🛡️ Proposed guard
const crossing = fixture.allowedCrossings.find((entry) => entry.port === port && entry.source === 'RT-CONTROLLER'); + assert.ok(crossing, `no RT-CONTROLLER crossing declared for ${port}`); const serialized = encodedCrossing(crossing);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/runtime-contracts/topology.test.mjs` around lines 175 - 177, In the crossing lookup test, assert that the `crossing` result from `fixture.allowedCrossings.find` is defined before passing it to `encodedCrossing`. Add a clear assertion message identifying the missing port, while preserving the existing encoding and invocation flow for valid crossings.
102-149: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThese hostile-object probes only re-test the
typeof serialized !== 'string'guard.
validateTopologyCrossingrejects any non-string before decoding, so thetoJSONtrap, the getter accessor, and the Proxy trap counters at Lines 120-121 and 149 pass vacuously — no descriptor-vs-getter or ownKeys behavior is actually exercised. If the intent is to prove the parser never invokes user code, drive these through the string frame path (e.g. encode a canonical frame whose payload is the hostile shape, or assert the same invariants insideparsedCrossing); otherwise this collapses to a single non-string rejection case.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/runtime-contracts/topology.test.mjs` around lines 102 - 149, Update the hostile-object probes around validateTopologyCrossing and createScriptedFake().invoke so they exercise the string-frame decoding path rather than only the typeof serialized !== 'string' rejection. Encode canonical frames containing the accessor and Proxy payloads, or move the assertions into parsedCrossing, and preserve checks that getters and Proxy traps remain unused during validation.packages/authority-kernel/src/index.ts (1)
178-187: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueSymbol-keyed extra properties slip past the exact-shape check.
Object.keys(descriptors)enumerates only string keys, so{ ...event, [Symbol('extra')]: 1 }satisfies both the length and membership checks. The returned snapshot is rebuilt fromkeysso nothing leaks, but it does weaken the stated "exact shape" rejection thattests/runtime-contracts/topology.test.mjsasserts for the codec path. UseReflect.ownKeys(value)for the count if strict rejection is intended.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/authority-kernel/src/index.ts` around lines 178 - 187, Update the exact-shape validation around the descriptors check to count all own keys, including symbols, by using Reflect.ownKeys(value) for the property-count comparison. Preserve the existing descriptor and string-key membership checks, and continue returning undefined when extra symbol-keyed properties are present.tests/authority-kernel/authority-kernel.test.mjs (1)
231-232: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the reducer succeeded before spreading
.value.If
reduceAuthorityregresses,transitionbecomesundefined, every{ ...transition, ... }case degrades to a bare object, and all twelvevalidateTransitioncases still returnFC-INPUT— passing for the wrong reason.🛡️ Proposed guard
- const transition = kernel.reduceAuthority(state, event, bindings).value; + const reduced = kernel.reduceAuthority(state, event, bindings); + assert.equal(reduced.ok, true); + const transition = reduced.value;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/authority-kernel/authority-kernel.test.mjs` around lines 231 - 232, Validate the result of kernel.reduceAuthority(state, event, bindings) before accessing its value or constructing transition assertions. Add an explicit success assertion for the reducer result in this test flow, then preserve the existing transition and replayStep handling so failures stop immediately instead of allowing validateTransition cases to pass on an empty object.tests/fixtures/conformance-oracle.json (2)
133-139: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
faults.timeoutandfaults.resumeare unreferenced.Only
before-record/after-record/before-evaluationare consumed by the tests, andrunAttempthas notimeout/resumecrash points. Either exercise them or drop them from the oracle.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/fixtures/conformance-oracle.json` around lines 133 - 139, Remove the unreferenced timeout and resume entries from the faults object in the conformance oracle, since runAttempt has no corresponding crash points and the tests only consume before-record, after-record, and before-evaluation.
186-186: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUnify the route digest key name.
routeElementsBatch1usesdigestwhile batches 2 and 3 userouteDigest, forcing theroute.digest ?? route.routeDigestfallback intests/conformance/conformance.test.mjs(Line 88). Pick one key so the oracle stays mechanically checkable.Also applies to: 277-277
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/fixtures/conformance-oracle.json` at line 186, Unify the route digest property in tests/fixtures/conformance-oracle.json by renaming the batch 1 routeElements entries’ digest key to routeDigest, matching batches 2 and 3. Then update the conformance test’s route digest access to use the single routeDigest key without the digest fallback.packages/conformance/tsconfig.json (1)
3-3: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider a narrow ambient instead of the whole
DOMlib.
packages/runtime-contracts/src/index.tsdeclares minimalTextEncoder/TextDecoderambients rather than pullingDOMin; matching that here keeps the pure package from acquiring the entire browser global surface (and accidentally type-checkingdocument/fetchusage).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/conformance/tsconfig.json` at line 3, Update the conformance tsconfig compilerOptions to remove the broad DOM library and use the package’s narrow TextEncoder/TextDecoder ambient declarations, matching the approach in runtime-contracts/src/index.ts while retaining ES2022 support.tests/conformance/conformance.test.mjs (1)
99-99: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThis assertion is tautological.
Appending
0to a truncated digest only differs from the original when the last character isn't0, which is true by luck for this fixture. It proves nothing about tamper detection — either drop it or recompute a digest over mutated elements (as done on Lines 92-98).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/conformance/conformance.test.mjs` at line 99, The assertion involving oracle.routeElementsBatch1[0].digest is tautological because it relies on the fixture’s final character; remove it or replace it with a meaningful tamper-detection check that mutates route elements and recomputes their digest, following the established approach in the assertions on lines 92-98.packages/conformance/src/index.ts (1)
740-742: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueLoop/local
suiteshadows the module-levelsuite()route-element helper.Purely a readability nit — rename to
suiteIdinsuiteGateandevaluateProviderto keep the helper visible.Also applies to: 752-752
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/conformance/src/index.ts` around lines 740 - 742, Rename the loop-local suite variables to suiteId in suiteGate and evaluateProvider, including their references in seen/expected checks and reason construction, while leaving the module-level suite() route-element helper unchanged.scripts/write-evidence.mjs (1)
14-14: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winUse a byte-stable comparator for digest ordering.
localeCompareis locale- and ICU-sensitive, so key insertion order infixtureDigests— and therefore the serializedevidence.jsonbytes — can differ between machines. For a determinism-oriented evidence artifact, prefer a plain code-unit comparison.♻️ Proposed fix
- for (const entry of readdirSync(current, { withFileTypes: true }).sort((a, b) => a.name.localeCompare(b.name))) { + for (const entry of readdirSync(current, { withFileTypes: true }).sort((a, b) => (a.name < b.name ? -1 : a.name > b.name ? 1 : 0))) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/write-evidence.mjs` at line 14, Update the directory-entry sort in the evidence generation loop to use a locale-independent plain code-unit comparator instead of localeCompare, ensuring fixtureDigests and serialized evidence.json ordering remain byte-stable across machines.scripts/check-runtime-topology.test.mjs (1)
22-41: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider covering the missing-input early return and the manifest gate.
Lines 36-38 (
runtime topology required input is missing, which short-circuits everything downstream) and the manifest checks at Lines 60-70 ofcheck-runtime-topology.mjshave no test. Both are cheap to add with the existingwithCopyhelper.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/check-runtime-topology.test.mjs` around lines 22 - 41, Add tests in scripts/check-runtime-topology.test.mjs using withCopy to cover the missing required input early return in validateRuntimeTopology and the manifest validation checks in check-runtime-topology.mjs. Assert the missing-input case returns the expected error without running downstream validation, and add a copied-manifest case that exercises the manifest gate and verifies its reported failure.scripts/check-runtime-topology.mjs (1)
71-75: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winText-regex capability scan duplicates — and weakens — the AST checker on a blocking path.
check-package-boundaries.mjsreplaced exactly this rule with a TypeScript AST walk (importSpecifiers/unboundIdentifierReads), and its test asserts that comments and string literals are not scanned. Here the raw/\b(fetch|process|require)\b/matches inside comments and strings, so a doc comment mentioningprocessfailspnpm runtime:checkwith a misleading message; conversely/from ['"].../misses bare side-effect imports (import 'node:fs';) and dynamicimport('node:fs').Export the two helpers from
check-package-boundaries.mjsand reuse them here so both gates share one implementation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/check-runtime-topology.mjs` around lines 71 - 75, Replace the raw regex capability scan in check-runtime-topology.mjs with the shared AST-based helpers importSpecifiers and unboundIdentifierReads from check-package-boundaries.mjs. Export those helpers there, then reuse them for runtime topology validation so comments and strings are ignored while side-effect and dynamic imports are detected consistently.scripts/check-package-boundaries.mjs (1)
174-197: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win
exportedNamesmisses several export forms, leaving gaps in the prohibited-surface gate.Not covered:
export default ...,export * from '...'/export * as ns from '...', and binding-pattern declarations (export const { dispatch } = impl;—declaration.nameis not an Identifier, so it is silently skipped). A star re-export of an allowed dependency, or a default export object, bypasses theprohibitedExportNamesregex entirely.Suggest at minimum treating
export defaultandexport *as unresolvable surfaces and rejecting them outright, and recursing intoBindingPatternnames.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/check-package-boundaries.mjs` around lines 174 - 197, Update exportedNames to reject unresolvable export forms by detecting default export declarations and export-all declarations, including namespace re-exports, rather than allowing them through the prohibited-surface check. Extend variable export handling to recursively collect identifiers from BindingPattern names such as object and array destructuring, while preserving existing handling for direct identifiers and named exports.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@package.json`:
- Around line 31-34: Remove the tsc --build tsconfig.json step from the test
script in package.json so test only runs the Node test suite. Keep compilation
handled by the existing typecheck step invoked by check, avoiding duplicate
builds and emitted artifacts.
In `@packages/authority-kernel/src/index.ts`:
- Around line 451-458: Remove the tautological sameBinding(subject, { ...subject
}) condition from the FC-SUBJECT validation in the event-binding gate. Preserve
the parseIdentity, event-prefix, subject.basis, and fence.generation checks
unchanged.
In `@packages/codec/package.json`:
- Around line 6-10: Update the package exports configuration around the
"exports" and "types" fields to use an object export map with a "types"
condition pointing to the declaration file and a default condition pointing to
the JavaScript entry. Remove reliance on the separate top-level "types" field
while preserving the existing entry paths.
In `@packages/codec/src/index.ts`:
- Around line 112-119: Update the surrogate validation loop in the frame
encoding/decoding path to reject a high surrogate when it is the final code
unit, explicitly checking that the next code unit exists before validating its
range. Preserve existing handling for valid pairs and lone low surrogates, and
add coverage for trailing lone surrogates through both encodeFrame and
decodeFrame.
In `@packages/conformance/package.json`:
- Line 10: Update the typecheck script in package.json to use a TypeScript
5.8.2-compatible command instead of tsc --build --noEmit, while preserving the
script’s purpose of validating the package types.
In `@packages/conformance/src/index.ts`:
- Line 66: Deeply freeze all exported authority data: in
packages/conformance/src/index.ts at line 66, freeze the SUITES.slice(0, 32)
result and type REALIZATION_SUITES as readonly SuiteId[]; at line 124, freeze
each ProductRoute, its elements array, and each element before freezing
PRODUCT_ROUTE_ORACLE; in packages/runtime-contracts/src/index.ts at line 97,
freeze every ALLOWED_CROSSINGS entry and its ports/TOPOLOGY members, and make
validateTopologyCrossing return a frozen copy instead of the live match.
In `@README.md`:
- Around line 45-46: Update the README description of the delivery check to
include typecheck, package-boundary validation, runtime-topology validation, and
the full test suite executed by the pnpm check chain, or explicitly label the
current list as non-exhaustive.
In `@scripts/check-active-repository.mjs`:
- Around line 376-517: Update scripts/check-active-repository.mjs lines 376-517
and scripts/check-package-boundaries.mjs lines 308-368 to use the existing
shared readTextOr/readJsonOr helpers for every required text and JSON input
identified by the review, including the guarded package manifests. Ensure
missing or malformed files push validation errors and return undefined instead
of throwing, while preserving existing validation behavior when reads succeed.
In `@scripts/check-package-boundaries.mjs`:
- Around line 321-328: Update the runtime-contracts validatePureSurface call to
provide the ambient capabilities it is allowed to use, including the existing
process export and fetch/require capabilities used by the other pure-package
validations, instead of passing an empty Set. Keep the current rootDir, package
name, export regex, and errors handling unchanged.
In `@scripts/write-evidence.mjs`:
- Line 7: Validate the CLI subject immediately after reading process.argv[2] and
before any path construction or file writes. Accept only the expected phase-0
subject format (for example, a strict allowlist) and reject invalid values so
path segments cannot escape the artifacts directory; apply the same validation
to the related usages around lines 43–44.
- Around line 28-41: Update the test invocation in the evidence-writing flow to
set an appropriate spawn timeout and inspect both test.error and a null
test.status, treating spawn failures, signal termination, and timeouts as failed
runs. Ensure the script exits nonzero and does not produce a successful-looking
artifact when pnpm test does not complete normally, while preserving the
existing exitCode recording for completed runs.
In `@tests/fixtures/codec-corpus.json`:
- Around line 6-13: The requiredClassSet metadata is disconnected from the
corpus case classifications and is not validated. In the codec corpus fixture
and its tests, either rename/assign case class values so every entry in
requiredClassSet is represented and update corpus.test.mjs to assert each
required class appears, or remove requiredClassSet and its self-equality
assertion entirely.
---
Nitpick comments:
In `@packages/authority-kernel/src/index.ts`:
- Around line 178-187: Update the exact-shape validation around the descriptors
check to count all own keys, including symbols, by using Reflect.ownKeys(value)
for the property-count comparison. Preserve the existing descriptor and
string-key membership checks, and continue returning undefined when extra
symbol-keyed properties are present.
In `@packages/authority-kernel/tsconfig.json`:
- Line 6: Update the TypeScript compiler options in tsconfig.json by removing
"DOM" from the lib array, leaving only "ES2022" so browser ambient APIs are not
type-visible in this package.
In `@packages/codec/src/index.ts`:
- Around line 97-106: Update compareKeys to compare string characters by UTF-16
code units rather than spreading into Unicode code points, preserving
lexicographic ordering and length fallback. If the code-point ordering is
intentional, instead document near compareKeys that jig.codec.v1 is not
JCS-compatible; otherwise implement the UTF-16 ordering required for
cross-runtime compatibility.
- Around line 502-506: Update the same-run validation in the ID-TXN/ID-OP branch
to derive its regular expression from the shared run grammar constant and
existing patterns utilities, rather than duplicating the inline run literal.
Preserve the current capture comparison and INVALID_SCOPE behavior, keeping the
check synchronized with the run definition.
In `@packages/codec/tsconfig.json`:
- Line 6: Remove "DOM" from the lib array in the codec TypeScript configuration,
retaining only the ES2022 library and relying on the existing `@types/node`
declarations for TextEncoder and TextDecoder.
In `@packages/conformance/src/index.ts`:
- Around line 740-742: Rename the loop-local suite variables to suiteId in
suiteGate and evaluateProvider, including their references in seen/expected
checks and reason construction, while leaving the module-level suite()
route-element helper unchanged.
In `@packages/conformance/tsconfig.json`:
- Line 3: Update the conformance tsconfig compilerOptions to remove the broad
DOM library and use the package’s narrow TextEncoder/TextDecoder ambient
declarations, matching the approach in runtime-contracts/src/index.ts while
retaining ES2022 support.
In `@scripts/check-package-boundaries.mjs`:
- Around line 174-197: Update exportedNames to reject unresolvable export forms
by detecting default export declarations and export-all declarations, including
namespace re-exports, rather than allowing them through the prohibited-surface
check. Extend variable export handling to recursively collect identifiers from
BindingPattern names such as object and array destructuring, while preserving
existing handling for direct identifiers and named exports.
In `@scripts/check-runtime-topology.mjs`:
- Around line 71-75: Replace the raw regex capability scan in
check-runtime-topology.mjs with the shared AST-based helpers importSpecifiers
and unboundIdentifierReads from check-package-boundaries.mjs. Export those
helpers there, then reuse them for runtime topology validation so comments and
strings are ignored while side-effect and dynamic imports are detected
consistently.
In `@scripts/check-runtime-topology.test.mjs`:
- Around line 22-41: Add tests in scripts/check-runtime-topology.test.mjs using
withCopy to cover the missing required input early return in
validateRuntimeTopology and the manifest validation checks in
check-runtime-topology.mjs. Assert the missing-input case returns the expected
error without running downstream validation, and add a copied-manifest case that
exercises the manifest gate and verifies its reported failure.
In `@scripts/write-evidence.mjs`:
- Line 14: Update the directory-entry sort in the evidence generation loop to
use a locale-independent plain code-unit comparator instead of localeCompare,
ensuring fixtureDigests and serialized evidence.json ordering remain byte-stable
across machines.
In `@tests/authority-kernel/authority-kernel.test.mjs`:
- Around line 231-232: Validate the result of kernel.reduceAuthority(state,
event, bindings) before accessing its value or constructing transition
assertions. Add an explicit success assertion for the reducer result in this
test flow, then preserve the existing transition and replayStep handling so
failures stop immediately instead of allowing validateTransition cases to pass
on an empty object.
In `@tests/codec/codec.test.mjs`:
- Around line 95-101: Ensure the formatter test explicitly asserts that
corpus.constructors contains exactly 22 entries before iterating, and add the
equivalent count assertion for the identity loop’s constructor collection. Keep
the existing per-entry assertions unchanged so the tests still validate every
supplied canonical form.
In `@tests/codec/corpus.test.mjs`:
- Around line 92-96: Select mutation targets by their stable case IDs in the
tamper matrix instead of positional indexes: use valid-canonical for
canonicalBytesSha256, digest-bound for stagedDigest, and malformed for
result.error.code. Preserve the existing mutations while ensuring each field is
modified on the intended corpus case.
- Line 79: Update the test’s spawnSync calls to resolve golden-consumer.mjs
relative to the test file rather than process.cwd(), and replace hardcoded /tmp
mkdtempSync prefixes with a portable temporary-directory source such as
os.tmpdir(). Apply the same self-locating changes to the occurrences around the
referenced lines.
In `@tests/codec/golden-consumer.mjs`:
- Around line 25-31: Update actualCase to dispatch based on whether each fixture
key exists, not whether its value is truthy; use own-property checks for frame,
generator, identity, and staged so empty-string values still reach their
intended handlers and unsupported entries continue throwing.
In `@tests/conformance/conformance.test.mjs`:
- Line 99: The assertion involving oracle.routeElementsBatch1[0].digest is
tautological because it relies on the fixture’s final character; remove it or
replace it with a meaningful tamper-detection check that mutates route elements
and recomputes their digest, following the established approach in the
assertions on lines 92-98.
In `@tests/fixtures/conformance-oracle.json`:
- Around line 133-139: Remove the unreferenced timeout and resume entries from
the faults object in the conformance oracle, since runAttempt has no
corresponding crash points and the tests only consume before-record,
after-record, and before-evaluation.
- Line 186: Unify the route digest property in
tests/fixtures/conformance-oracle.json by renaming the batch 1 routeElements
entries’ digest key to routeDigest, matching batches 2 and 3. Then update the
conformance test’s route digest access to use the single routeDigest key without
the digest fallback.
In `@tests/runtime-contracts/topology.test.mjs`:
- Around line 175-177: In the crossing lookup test, assert that the `crossing`
result from `fixture.allowedCrossings.find` is defined before passing it to
`encodedCrossing`. Add a clear assertion message identifying the missing port,
while preserving the existing encoding and invocation flow for valid crossings.
- Around line 102-149: Update the hostile-object probes around
validateTopologyCrossing and createScriptedFake().invoke so they exercise the
string-frame decoding path rather than only the typeof serialized !== 'string'
rejection. Encode canonical frames containing the accessor and Proxy payloads,
or move the assertions into parsedCrossing, and preserve checks that getters and
Proxy traps remain unused during validation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 69795f10-4203-4b48-aac0-a8a94ebfeb58
⛔ Files ignored due to path filters (2)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamltests/fixtures/workspace/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (58)
.github/workflows/check.ymlAGENTS.mdREADME.mdpackage.jsonpackages/authority-kernel/package.jsonpackages/authority-kernel/src/index.tspackages/authority-kernel/tsconfig.jsonpackages/codec/package.jsonpackages/codec/src/index.tspackages/codec/tsconfig.jsonpackages/conformance/package.jsonpackages/conformance/src/index.tspackages/conformance/tsconfig.jsonpackages/runtime-contracts/package.jsonpackages/runtime-contracts/src/index.tspackages/runtime-contracts/tsconfig.jsonpnpm-workspace.yamlscripts/check-active-repository.mjsscripts/check-active-repository.test.mjsscripts/check-delivery-track.mjsscripts/check-delivery-track.test.mjsscripts/check-package-boundaries.mjsscripts/check-package-boundaries.test.mjsscripts/check-runtime-topology.mjsscripts/check-runtime-topology.test.mjsscripts/run-gf-001-tests.mjsscripts/write-evidence.mjsscripts/write-gf-001-evidence.mjstests/authority-kernel/authority-kernel.test.mjstests/codec/codec.test.mjstests/codec/corpus.test.mjstests/codec/golden-consumer.mjstests/conformance/conformance.test.mjstests/fixtures/authority-oracle.jsontests/fixtures/codec-corpus.jsontests/fixtures/codec-vectors.jsontests/fixtures/conformance-oracle.jsontests/fixtures/runtime-fakes.jsontests/fixtures/runtime-topology.jsontests/fixtures/workspace/.gitignoretests/fixtures/workspace/package.jsontests/fixtures/workspace/packages/pkg-a/package.jsontests/fixtures/workspace/packages/pkg-a/src/index.tstests/fixtures/workspace/packages/pkg-a/tsconfig.jsontests/fixtures/workspace/packages/pkg-b/package.jsontests/fixtures/workspace/packages/pkg-b/src/index.tstests/fixtures/workspace/packages/pkg-b/tsconfig.jsontests/fixtures/workspace/packages/pkg-c/package.jsontests/fixtures/workspace/packages/pkg-c/src/index.tstests/fixtures/workspace/packages/pkg-c/tsconfig.jsontests/fixtures/workspace/pnpm-workspace.yamltests/fixtures/workspace/tsconfig.base.jsontests/fixtures/workspace/tsconfig.jsontests/gf-001/evidence-contract.jsontests/gf-001/evidence.test.mjstests/runtime-contracts/topology.test.mjstests/workspace/workspace-substrate.test.mjstsconfig.json
💤 Files with no reviewable changes (4)
- tests/gf-001/evidence-contract.json
- scripts/run-gf-001-tests.mjs
- tests/gf-001/evidence.test.mjs
- scripts/write-gf-001-evidence.mjs
1 participant
Summary
Reworks Phase 0 under the amended per-story-evidence policy.
checkchain and statically prevents that chain from reaching evidence-writing orartifacts/outputVerification
Clean detached worktree at
2325231:pnpm install --frozen-lockfile --config.confirmModulesPurge=false— passpnpm check— pass; includes lint, formatting, docs links, delivery and structure checks, typecheck, boundaries,runtime:check, and workspace testsgit diff --check— passwrite-evidence.mjsintentionally runspnpm testagain after the gate: it records the observed test exit status in evidence and remains reporter-only, so a failed observed test does not make the evidence writer itself a gate.Rule-16 certification
gf-0NNidentifier remains in trackedscripts/,tests/,packages/, or.github/filenamescheck-delivery-track.mjsand its test, which validate canonical delivery-story datagrep -nE '/[^/*]*GF-0[0-9]{2}[^/]*/'leaves only the four legitimate catalog-data matches (coverage-table rows and doc-content replacement inputs)The exclusions are not blanket file exclusions: all other values still fail the sweep.
Out of scope
scripts/check-delivery-track.mjsScope notes
Summary by CodeRabbit