chore: sync main into testnet-canary after 10.0.10 - #1986
Conversation
release: promote testnet-canary to main for 10.0.10
chore(release): bump version set to 10.0.10
| // unbounded growth (e.g. an accidental dump of full OpenAPI schema in-line). | ||
| const lines = skillContent.split('\n').length; | ||
| expect(lines).toBeLessThan(950); | ||
| expect(lines).toBeLessThan(1000); |
There was a problem hiding this comment.
🟡 Issue: Raising the skill size budget normalizes the sprawl instead of decomposing it
What's wrong
This change makes the size guard less effective exactly when the canonical skill is already close to the 1k-line boundary. The comments justify accumulated growth, but the cleaner move is to split the content model so future feature documentation has a natural home. Otherwise every new API surface will pressure this same file and the test will keep becoming a budget waiver instead of a maintainability guard.
Example
A future endpoint addition can add another ~25 lines before this guard trips, keeping the main dkg-node skill as the dumping ground instead of forcing a split into focused referenced material.
Suggested direction
Treat the failing 950-line budget as a design signal. Keep dkg-node/SKILL.md as the concise routing/overview artifact and move expanded contracts or workflows into focused referenced docs, rather than ratcheting the cap upward.
For Agents
Look at packages/cli/skills/dkg-node/SKILL.md and the skill-serving routes. Preserve the newly documented resident-author behavior, but extract long secondary sections into a dedicated referenced skill/reference endpoint or trim duplicated API teaching so the primary skill remains comfortably below the prior budget. Keep a line-budget test that fails before the doc approaches 1k lines again.
| inFlightSubstrateFanOutCount: () => 0, | ||
| router: { closePooling: vi.fn(async () => {}) }, | ||
| node: { stop: stopNode }, | ||
| finalizationRuntime: new FinalizationRuntime(), |
There was a problem hiding this comment.
🟡 Issue: Repeated hand-built agent fixtures keep spreading private lifecycle dependencies
What's wrong
Adding finalizationRuntime to every hand-rolled any agent fixture deepens an existing test-maintenance trap: these tests bypass the constructor, then mirror whatever private fields stop() happens to touch. That makes lifecycle refactors noisy and encourages future changes to patch scattered object literals rather than improve the fixture boundary.
Example
The next dependency read by agent.stop() will require editing each object literal again. Missing one fixture will fail for fixture-shape reasons rather than because the shutdown scenario changed.
Suggested direction
Collapse the duplicated Object.create setup into a focused fixture factory for the shutdown surface. The tests should describe which collaborator each scenario varies, not restate the full private agent shape every time.
For Agents
In packages/agent/test/outbox-shutdown-lifecycle.test.ts, extract a local makeStartedAgentForStop(overrides) helper that installs the default shutdown collaborators, including finalizationRuntime, and lets each test override only the collaborator it asserts. Preserve the four shutdown scenarios and prove the helper still supports the timeout override and warning assertions.
| expect(peerStoreReads).toBe(3); | ||
| expect(syncFromPeerDetailed.calls.at(-1)).toEqual([remotePeer.toString(), ['runtime-contextGraph']]); | ||
| expect(syncSharedMemoryFromPeerDetailed.calls.at(-1)).toEqual([remotePeer.toString(), ['runtime-contextGraph']]); | ||
| expect(syncFromPeerDetailed.calls.at(-1)).toEqual([ |
There was a problem hiding this comment.
🟡 Issue: The test now codifies optional positional-argument plumbing
What's wrong
The changed expectation locks the test to incidental call mechanics rather than the behavior this scenario cares about. It also exposes a broader readability smell: the sync method API has accumulated optional positional parameters, so simple calls now need placeholder undefineds to reach later arguments.
Example
Adding a new optional parameter before options would force this assertion to change even if syncContextGraphFromConnectedPeers still sends the same peer and context graph IDs.
Suggested direction
Prefer a semantic assertion over an exact tuple full of placeholders, and consider moving the detailed sync API toward a named options object so callers do not have to encode undefined slots.
Confidence note
This is anchored in a test-only hunk, so the risk is maintainability friction rather than runtime behavior.
For Agents
In packages/agent/test/agent.part-16.test.ts, avoid asserting the entire private positional tuple. Either assert the semantic prefix and the specific option slot that matters, or introduce a small named-options helper around the detailed sync calls and test that boundary. Preserve the result aggregation assertions.
| "version": "10.0.9", | ||
| "version": "10.0.10", | ||
| "type": "module", | ||
| "engines": { |
There was a problem hiding this comment.
🟡 Issue: New Node runtime contract is not locked by tests
What's wrong
The PR makes the supported Node range an operator-facing packaging contract, but there is no automated check that the two published packages keep declaring the same unflagged-node:sqlite range. That leaves a high-risk release detail verified only by manual review.
Example
A packaging edit could remove or mistype one range, for example leaving @origintrail-official/dkg-agent without >=22.13.0 <23.0.0 || >=23.4.0, while the current package-root tests still pass. Consumers embedding the agent directly would then lose the EBADENGINE signal and only discover the unsupported runtime when node:sqlite fails at daemon start.
Suggested direction
Cover the new manifest contract with a static test so CI fails if either published package drops or diverges from the documented Node range.
Confidence note
I did not find an existing manifest/package-root assertion for this new runtime contract; this is a static review of the PR diff and nearby package-root checks.
For Agents
Add a small manifest-contract test near the package-root/export checks, or a shared release/package metadata test, that reads packages/agent/package.json and packages/cli/package.json and asserts both published packages expose the exact supported engines.node range required by unflagged node:sqlite. Keep the existing export checks intact.
The 10.0.10 sync (#1986) was squash-merged, which flattened it into a single commit parented on 8334c0a — a pre-release commit on main. The content came across correctly (main and testnet-canary trees were already byte-identical), but git no longer saw main's release commits as ancestors of testnet-canary, so `git rev-list testnet-canary..main` reported four phantom commits: f61f335 Merge pull request #1984 (release/10.0.10-bump) f516586 Merge pull request #1983 (release/10.0.10) e89f8f6 fix(epcis): fail fast on invalid payloads 0112e78 Fix release CI test expectations This is a real merge commit, so the histories join and those stop being reported as missing. It changes no files. Squash is right for feature PRs; for a branch-sync PR it defeats the purpose, because the ancestry is the payload. Future main -> testnet-canary syncs should use 'Create a merge commit'. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
3 participants
Summary
Re-equalizes
testnet-canarywithmainafter the 10.0.10 release. Fast-forward only —testnet-canaryis fully contained inmain(0 commits ahead, merge-base == canary tip), so there is nothing to resolve.mainis 4 commits ahead, 29 files, +140 / −32.What canary is missing
0112e78a2Fix release CI test expectationsrelease/10.0.10, not canarye89f8f6ebfix(epcis): fail fast on invalid payloadsrelease/10.0.10, not canaryf51658645Merge #1983 (testnet-canary→mainpromotion)f61f33508Merge #1984 (version bump to 10.0.10)The first two are the important ones. They were pushed to the frozen
release/10.0.10branch during the promotion PR rather than to canary, so canary has been missing them since — including a real product change inpackages/epcis/src/validation.ts, not just test expectations. Without this sync the next canary cycle would silently regress both.The other two bring canary's version set to 10.0.10 (it currently still declares 10.0.9) along with the 10.0.10
CHANGELOG.mdentry, theengines.nodedeclarations on@origintrail-official/dkgand@origintrail-official/dkg-agent, and theRELEASE_PROCESS.md§6c/§10 rewrite.Risk
Low. Every commit here is already on
mainand has been through full CI on #1983 and #1984, plus a published release. No code is being introduced that has not already shipped tomainnetas 10.0.10.Note on CI
ci.yml:14/:16filterpushandpull_requestto[main, v10-rc, release/rc.12, rc17-vm-wip], so a PR targetingtestnet-canarydoes not trigger the vitest lane. That is expected here and not a coverage gap — this exact tree is what CI validated on #1984 and what was published as 10.0.10. Closing that filter gap is tracked separately as part of the canary-cycle cleanup.Merge
Prefer a merge commit (or fast-forward) over squash, so canary keeps the same per-PR history
mainhas.🤖 Generated with Claude Code