refactor(sync): unify durable-_meta IRI term classifier (#1940) - #1944
Merged
Conversation
Extract the two duplicated durable-`_meta` IRI term predicates into one
shared module, packages/agent/src/sync/iri-term.ts, imported by both
callers. Behavior-preserving: the intentional, load-bearing divergence on
empty/non-string input is kept as two named exports over one private core
predicate, not collapsed to a single function or a defaulted flag.
- isIriTerm (responder read, graph-plan.ts): the bare core predicate,
LENIENT — isIriTerm('') stays true; no typeof/length guard added, so the
protocol responder read/paging path is byte-identical.
- isIriMetaSubject (requester ingest, durable-integrity.ts): STRICT,
fail-closed — isIriMetaSubject('') stays false and a non-string subject is
false rather than throwing, preserving the #1921 peer-ingest hardening.
durable-integrity.ts and graph-plan.ts drop their local copies and import
the shared exports; all 6 call sites are unchanged. Adds
test/iri-term.test.ts covering IRI / blank-node / literal / empty-string /
non-string inputs for both exports and asserting the divergence explicitly
(and registers it in vitest.unit.config.ts).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address the bot's 🟡 on iri-term.ts:58 — the strict ingest helper is documented as the fail-closed boundary for raw peer-ingest values (including non-string), but its param type was `string`, forcing untyped/unknown-valued ingest callers (and the test) to cast. Widen ONLY isIriMetaSubject to `(term: unknown): boolean`; the existing `typeof term === 'string' && term.length > 0` guard narrows before isIriTermCore, so tsc stays clean and runtime is byte-identical (the 4 durable-integrity call sites pass a `string` `quad.subject` and are unaffected). isIriTerm stays `(term: string): boolean` — the lenient responder helper runs on trusted store rows and calls core directly (widening would break the guardless startsWith). The string-vs-unknown split now honestly reflects trusted-input vs fail-closed-untrusted-input in the type system. Drops the `as unknown as string` cast from the non-string test case (it now delegates directly) and keeps the non-string→false assertion. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
otReviewAgent
left a comment
otReviewAgent
left a comment
There was a problem hiding this comment.
Review Agent completed this review and found no issues.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
_metaIRI term classifier existed as two near-identical copies:isIriMetaSubjectinpackages/agent/src/sync/durable-integrity.ts(requester ingest) andisIriTerminpackages/agent/src/sync/responder/graph-plan.ts(responder read). This unifies them into one shared module,packages/agent/src/sync/iri-term.ts, imported by both.!term.startsWith('_:') && !term.startsWith('"')) but keep a deliberate, load-bearing divergence on empty/non-string input, so they are exported as two named functions, not collapsed into one or a defaulted flag:isIriTerm(responder read) is the bare core — lenient:isIriTerm('')staystrue, and notypeof/length guard is added, so the protocol responder read/paging path is byte-identical.isIriMetaSubject(requester ingest) istypeof term === 'string' && term.length > 0 && <core>— strict / fail-closed:isIriMetaSubject('')staysfalseand a non-string subject isfalserather than throwing, preserving the Enforce IRI-only durable-meta subjects at unverified peer-ingest (root fix behind #1788 fail-loud) #1921 peer-ingest hardening.durable-integrity.ts(4) andgraph-plan.ts(2) are unchanged — only the definition moved.Related
Diagrams
Files changed
packages/agent/src/sync/iri-term.tsisIriTermCore+ two named exports (isIriTermlenient,isIriMetaSubjectstrict), jsdoc documenting the intentional empty/non-string divergence.packages/agent/src/sync/durable-integrity.tsisIriMetaSubject; import it from./iri-term.js. 4 call sites unchanged.packages/agent/src/sync/responder/graph-plan.tsisIriTerm; import it from../iri-term.js. 2 call sites unchanged (protocol read path).packages/agent/test/iri-term.test.ts_:) / literal (") / empty-string / non-string inputs for both exports; asserts the divergence explicitly (isIriMetaSubject('') === false,isIriTerm('') === true).packages/agent/vitest.unit.config.tstest/**/*.test.tsglob already auto-discovers it).Test plan
tsc --noEmitclean for@origintrail-official/dkg-agent.pnpm exec vitest run --config vitest.unit.config.tsover durable / sync-responder / sync-verify / sync-control suites + the newiri-termtest — 24 files, 308 tests green.test/iri-term.test.ts— 8 tests green; locks the shared behavior and the intentional empty/non-string divergence.