feat: enforce runtime State/Response/Event hierarchy at compile time#166
feat: enforce runtime State/Response/Event hierarchy at compile time#166mttrbrts wants to merge 7 commits into
Conversation
7b088f7 to
bb05c3a
Compare
Derive RuntimeState/RuntimeRequest/RuntimeResponse/RuntimeEvent unions from the template's Concerto model (concrete subclasses of the runtime base types) and use them as the bounds/positions in the injected TemplateLogic declarations. A "state" that is a plain concept (not an asset extending State) now binds to `never` and fails to compile (TS2344); likewise an invalid response or emitted event fails in return position (TS2322). Also emit generated model files under `generated/` so the logic's own `./generated/<ns>` imports resolve in the twoslash sandbox - previously they did not, so all model types were `any` and no type checking occurred. compileLogic now surfaces the TS2344 hierarchy violation as a hard error (scoped to the logic entry file) instead of silently ignoring diagnostics. Known limitation: request is not enforceable via the input parameter (TypeScript method parameters are bivariant); some templates still suppress the errors with @ts-expect-error and should be updated separately. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: mttrbrts <code@rbrts.uk>
bb05c3a to
7998d3a
Compare
Coverage Report for CI Build 29432082590Coverage increased (+1.3%) to 54.981%Details
Uncovered Changes
Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
911f238 to
03b2ba7
Compare
Two follow-ups to the runtime hierarchy work: 1. Request enforcement (was unenforceable at compile time). The runtime Request hierarchy cannot be checked by the type system: `request` appears only as the `trigger` parameter, and TypeScript method parameters are bivariant, so the model-derived `never` bound accepts any type (State, Response and Event are caught because they sit in constraint / return positions). compileLogic now asserts the invariant against the model: when the logic defines a `trigger`, the model must declare a concrete subclass of org.accordproject.runtime@0.2.0.Request. Note the base Request/Response/State types are *concrete* (not abstract) in the runtime model, so a base-inclusive query (getRequestTypes) always lists the base and can never be empty. We therefore query with the base excluded via findConcreteSubclassNames(fqn, true) - the same base-excluding semantics used by cicero-core's isStateful() and by the RuntimeRequest union in the compilation context. A "request" declared as a plain concept yields an empty list and is rejected. 2. Remove the now-unused SMART_LEGAL_CONTRACT_BASE64. The runtime TemplateLogic/EngineResponse declarations are emitted directly by TypeScriptCompilationContext (with model-derived bounds), so the bundled base64 copy is dead. Dropped it from declarations.ts and stopped generating it in scripts/updateRuntimeDependencies.js. src/slc/SmartLegalContract.d.ts is kept: src/index.ts still re-exports the public TemplateLogic type from it. dayjs / jsonpath base64 (still consumed by TypeScriptToJavaScriptCompiler) are unchanged. Tests: 2 new TemplateArchiveProcessor cases (valid request compiles; empty request subtypes rejected) that also pin the base-concreteness pitfall. Full suite 74/74 passing, eslint clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Matt Roberts <code@rbrts.uk>
03b2ba7 to
eed77d6
Compare
… nominally at runtime The runtime Request/Response/State bases are concrete (only Obligation is abstract), so a template may legitimately use the bare base type. This reworks the enforcement so bare base types are accepted while a plain concept that does not extend the base is still rejected. Compile time (TypeScriptCompilationContext): - buildRuntimeUnion now *includes* the base type in the State/Response/Event union (getConcreteRuntimeTypes drops the base-exclusion filter, keeping the abstract filter). Using the bare base or any subclass type-checks. Response and Event still reject a plain concept structurally, because the base carries the $timestamp transaction/event discriminant that a concept lacks. State carries only $identifier, so an identified concept is structurally indistinguishable from the base - it can no longer be rejected at compile time. Runtime (TemplateArchiveProcessor): - Removed the model-level request check (it was vacuous once the concrete base is allowed - getRequestTypes always lists the base). - Added assertRuntimeHierarchy: after serialization, assert each payload's $class is, or extends, the runtime base for its role (request/state on input, state/result/events on output). This is nominal, so it accepts the bare base and any subclass and rejects a plain concept - the protection the type system cannot give for request (bivariant parameter) or state (structural $identifier). Events bind to the base Concerto Event (plain events and Obligations both pass). Fixture: the latedelivery archive's own state was a plain `concept LateDeliveryAndPenaltyState identified` - the exact bug pattern, which the new runtime check would (correctly) reject. Changed it to `asset LateDeliveryAndPenaltyState extends State` and updated the generated interface it imports. Tests: StateHierarchy updated for the base-inclusive union (plain-concept state now type-checks; response still rejected via missing $timestamp). Added a runtime test that a Response passed as the request is rejected. Full suite green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Matt Roberts <code@rbrts.uk>
…archy error - Move the runtime base-type FQN constants (State/Request/Response/Event) into src/utils.ts and import them from both TypeScriptCompilationContext and TemplateArchiveProcessor, removing the duplicated declarations. - Broaden the compile-time hierarchy error message to name State, Request, Response and Event (was "State and Obligation" only), and drop the outdated Obligation-centric wording now that events bind to the base Concerto Event. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Matt Roberts <code@rbrts.uk>
|
Suggestion: extract shared "assignable type" logic into
This PR's history makes the case: the semantics changed twice mid-review (base-exclusion → inclusion, model-level → runtime-only), and both call sites needed hand-updating each time. Suggest centralizing in // utils.ts
export function getAssignableConcreteTypes(modelManager: ModelManager, baseFqn: string): ClassDeclaration[] {
let baseType;
try {
baseType = modelManager.getType(baseFqn);
} catch {
return [];
}
return baseType.getAssignableClassDeclarations().filter(decl => !decl.isAbstract());
}
export function isAssignableTo(modelManager: ModelManager, fqn: string, baseFqn: string): boolean {
return getAssignableConcreteTypes(modelManager, baseFqn)
.some(decl => decl.getFullyQualifiedName() === fqn);
}
Guarantees compile-time and runtime checks agree by construction, and gives one place to unit-test the rule directly. |
|
…nit crash Addresses review feedback from @devanshi00: 1. Single source of truth for the runtime type hierarchy. Both the compile-time union builder (TypeScriptCompilationContext.getConcreteRuntimeTypes) and the runtime membership check (TemplateArchiveProcessor.assertRuntimeHierarchy) independently answered "is-a baseFqn" via getAssignableClassDeclarations() with their own try/catch. Extracted getAssignableConcreteTypes() and isAssignableTo() into utils.ts and use them from both sites. (Concerto-core has no equivalent helper - its ModelUtil.isAssignableTo is a private property-assignment check with different semantics.) 2. init() no longer crashes on stateless templates. executeTypeScriptInit() returns an empty placeholder `{ state: {} }` when there is no compiled init; init() then called serializer.fromJSON on that classless {}. Skip serialization of a state with no $class (same guard already used by assertRuntimeHierarchy), in init output, trigger output, and trigger input (a stateless init state chained into trigger). Added a regression test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Matt Roberts <code@rbrts.uk>
|
Thanks @devanshi00, both addressed in 0a431d3: 1. Shared assignable-type logic — extracted 2. |
Reference accordproject/concerto#1281, which proposes adding these class-hierarchy assignability helpers to Concerto's model API. The local implementation stays until that lands. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Matt Roberts <code@rbrts.uk>
There was a problem hiding this comment.
Pull request overview
This PR tightens the contract between Concerto runtime models and TypeScript template logic by ensuring that a template’s declared State/Request/Response/Event types align with the runtime class hierarchy (via model-derived compile-time unions plus nominal $class checks at runtime), and fixes a long-standing issue where generated model imports didn’t resolve in the twoslash sandbox.
Changes:
- Generate model-derived
Runtime*union bounds in the injected TypeScript compilation context (and emit generated models undergenerated/so logic imports resolve). - Remove the bundled SmartLegalContract base64 declarations from the logic compiler path and add runtime
$classhierarchy assertions ininit/trigger. - Add/adjust tests, fixtures, and snapshots to validate hierarchy enforcement and new compilation context layout.
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| test/TemplateArchiveProcessor.test.ts | Adds runtime request-hierarchy regression test and ensures stateless init placeholder doesn’t get serialized. |
| test/StateHierarchy.test.ts | New compiler-focused tests covering compile-time diagnostics for response/event and state behavior. |
| test/archives/latedeliveryandpenalty-typescript/model/model.cto | Fixes the fixture state to extend State and imports State. |
| test/archives/latedeliveryandpenalty-typescript/logic/generated/io.clause.latedeliveryandpenalty@0.1.0.ts | Updates generated interfaces to use IState instead of a concept-shaped state. |
| test/snapshots/TypeScriptCompiler.test.ts.snap | Snapshot updates reflecting injected runtime TemplateLogic JS output. |
| test/snapshots/TypeScriptCompilationContext.test.ts.snap | Snapshot updates for generated/ paths and injected Runtime* declarations. |
| test/snapshots/TemplateMarkInterpreter.test.ts.snap | Snapshot offsets updated due to added injected declarations in the compilation context. |
| src/utils.ts | Introduces runtime base FQNs and reusable model-assignability helpers. |
| src/TypeScriptCompilationContext.ts | Builds model-derived runtime unions and fixes twoslash filesystem layout to match ./generated/<namespace> imports. |
| src/TemplateArchiveProcessor.ts | Removes base64 declaration prepending, hard-fails on TS2344 (logic entry), and adds runtime hierarchy checks; adjusts serialization behavior for stateless placeholders. |
| src/runtime/declarations.ts | Removes unused SMART_LEGAL_CONTRACT_BASE64 export. |
| scripts/updateRuntimeDependencies.js | Stops bundling SmartLegalContract declarations; keeps only dayjs/jsonpath typings packaging. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…chy docs
Addresses Copilot review feedback:
- State validation now skips only the empty `{}` placeholder returned by a
stateless template's init (checked via Object.keys length), instead of any
state lacking $class. A non-empty classless state is validated normally and
rejected. Applied to init output, trigger output and trigger input; added a
regression test.
- Updated stale doc/comments that still described the pre-change behaviour:
the getRuntimeDeclarations docstring and injected comment (no longer a `never`
union for a concrete base) and the compileLogic comments (State/Request/
Response/base Event, not "State / Obligation"). Snapshots refreshed for the
injected-comment text.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Matt Roberts <code@rbrts.uk>
What
Makes the TypeScript compilation of template logic enforce the runtime class hierarchy: a template's declared state / response / emitted event must actually extend the corresponding Concerto runtime base type. Previously a template could declare its "state" as a plain
concept ... identified(rather thanasset ... extends State) and the engine would compile and run it happily.Why
Two root causes made the prior type checking ineffective:
interface IState { $identifier: string }. Under structural typing, any identified concept satisfies it — so the hierarchy was never actually checked../generated/<namespace>, but the compilation context emitted those model files at the root (<namespace>.ts). The imports never resolved in the twoslash sandbox, every model type collapsed toany, andanysatisfies every constraint — so no type checking could fire at all.How
Derive, from the template's own Concerto model, the union of concrete (non-abstract) subclasses of each runtime base type, and use those unions as the bounds/positions in the injected
TemplateLogicdeclarations. When a template declares no valid subtype for a base, its union isnever, so using a non-conforming type fails to type-check. This mirrorscicero-core'sgetStateTypes()/getRequestTypes()/getResponseTypes()/getEmitTypes(), but applies it as a TypeScript bound rather than a per-invocation runtime check — so it costs nothing at runtime.src/TypeScriptCompilationContext.tsbuildRuntimeUnion(baseFqn, aliasPrefix)→{ imports, union }over the concrete subclasses ofbaseFqn, excluding the base type itself, orneverwhen there are none.getRuntimeDeclarations(): the engine now emits the runtime declarations itself (with model-derived bounds) instead of prepending the base64.d.ts:RuntimeState/RuntimeRequest/RuntimeResponse= concrete subclasses of the respectiveorg.accordproject.runtime@0.2.0.*base.RuntimeEvent= concrete subclasses of the baseconcerto@1.0.0.Event(deliberately the base Event, notObligation— plain events are valid emits).TemplateLogicis a realabstract classwith a stubinitbody, so it still emits runtime JS and does not raise TS2391.getCompilationContext(): emits generated model files undergenerated/<namespace>.ts(fixes root cause Release markdown-transform #2).src/TemplateArchiveProcessor.tscompileLogic()no longer prependsSMART_LEGAL_CONTRACT_BASE64(the context supplies those declarations now).test/StateHierarchy.test.ts(new)5 tests: rejects a plain-concept state (TS2344), accepts a state extending
State, backward-compat for a plain-event template, rejects a response not extendingResponse(TS2322), rejects an event not extending baseEvent(TS2322).Testing
generated/prefix + theRuntime*unions; twoTemplateMarkInterpretersnapshots shifted line/offset only).Reviewer notes / known limitations
Update — bare base types allowed; hierarchy enforced nominally at runtime
Request/Response/Stateare concrete (not abstract) in the runtime model — onlyObligationis abstract — so a template may legitimately use the bare base type. The enforcement was reworked so bare base types are accepted while a plain concept that does not extend the base is still rejected:buildRuntimeUnionnow includes the base in the State/Response/Event union (drops the base-exclusion filter). Bare base or any subclass type-checks. Response/Event still reject a plain concept structurally (the base carries the$timestampdiscriminant a concept lacks). State carries only$identifier, so an identified concept is structurally indistinguishable from the base and can no longer be rejected at compile time.assertRuntimeHierarchyintrigger/init: after serialization, assert each payload's$classis, or extends, the runtime base for its role (request/state in, state/result/events out). This is nominal — it accepts the bare base and any subclass and rejects a non-extending concept, giving the protection the type system cannot (request is a bivariant parameter; state is structurally satisfied by any identified concept).latedeliveryarchive's own state was a plainconcept … identified(the bug pattern) — changed toasset … extends State, since the runtime check would otherwise (correctly) reject it.cicero-template-library(e.g. helloworldstate, latedeliveryandpenalty) still carry@ts-expect-error/@ts-ignoreon theirextends TemplateLogic<...>lines that suppress the new diagnostics. These were only added because the runtime types didn't previously resolve; they're now obsolete and should be removed in that repo.SMART_LEGAL_CONTRACT_BASE64(the bundled runtimeTemplateLogicdeclarations) is no longer used for logic compilation — the compilation context emits those declarations with model-derived bounds — so it is dropped fromdeclarations.tsand no longer generated.src/slc/SmartLegalContract.d.tsis kept for the publicTemplateLogictype re-exported bysrc/index.ts.🤖 Generated with Claude Code