Surface static package dependents after publish - #430
Conversation
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
📝 WalkthroughWalkthroughThis PR adds static package dependents metadata to publish responses. It introduces a DB query layer to identify packages with statically bundled references to a published package, a summary builder that groups and ranks those dependents, integrates the summary into the publish-external-push capability handler, extends repo checks and manifest schema for ChangesStatic Dependents Reporting
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
🔎 Preview deployed: https://kody-pr-430.kentcdodds.workers.dev Worker: Mocks:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (6)
docs/use/packages.md (1)
75-78: 💤 Low valueLGTM — accurately documents static snapshot semantics and the
static_dependentspayload.The snapshot/dynamic distinction is clearly framed and matches the runtime behavior. One tiny copy nit (skip if you don't care): "when the metadata is available" on line 228-229 is a bit hand-wavy — the contributing doc's "when the published commit is available" is sharper since that's the actual precondition in
getPublishStaticDependents.Also applies to: 228-241
🤖 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 `@docs/use/packages.md` around lines 75 - 78, Update the wording in the docs to replace the vague phrase "when the metadata is available" with the precise precondition "when the published commit is available" to match the implementation in getPublishStaticDependents; find the occurrences around the paragraph describing static snapshot semantics (lines referenced in the review) and change the phrasing so the docs state that static dependents are applied when the published commit is available, ensuring consistency with the getPublishStaticDependents behavior.packages/worker/src/package-runtime/static-package-dependents.ts (2)
7-8: 💤 Low valueOptional: export both default limits for symmetry.
defaultStaticDependentPackageLimitis exported butdefaultStaticDependentArtifactsPerPackageLimitis module-private. Callers that want to overridepackageLimitwhile keeping the artifacts default consistent (or surface defaults in docs/tests) currently can't reference the latter without duplicating the literal. Cheap to expose.♻️ Proposed change
export const defaultStaticDependentPackageLimit = 10 -const defaultStaticDependentArtifactsPerPackageLimit = 5 +export const defaultStaticDependentArtifactsPerPackageLimit = 5🤖 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/worker/src/package-runtime/static-package-dependents.ts` around lines 7 - 8, The second default constant defaultStaticDependentArtifactsPerPackageLimit is module-private but should be exported for symmetry and reuse; update its declaration to export it (export const defaultStaticDependentArtifactsPerPackageLimit = 5) so callers/tests/docs can reference the artifacts-per-package default just like defaultStaticDependentPackageLimit and avoid duplicating the literal; ensure any imports elsewhere use the exported name from static-package-dependents.
138-157: 💤 Low valueMinor:
entrypoints_truncatedsemantics are slightly broader than the schema docstring suggests.The flag fires when
matchingArtifactCount > min(entrypoints.length, artifactsPerPackageLimit). Becauseentrypointsis deduplicated byentryPoint, two artifacts that share anentryPointbut differ inartifactKind/artifactNamecollapse to one entry, which can flipentrypoints_truncatedtotrueeven when every retrieved row is represented. The correspondingstaticDependentItemSchema.entrypoints_truncateddescription inpublish-external-push.ts("True when the dependent has more matching entrypoints than returned") implies a stricter "more entrypoints than returned" semantic. Either is defensible, but agents reading the schema may interpret it more narrowly than the implementation.Two low-cost options:
- Tighten the wording to e.g. "True when the dependent has more matching artifacts than the entrypoints returned".
- Or rank/dedupe
entrypointsagainstmatchingArtifactCountin a way that aligns with the docstring.🤖 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/worker/src/package-runtime/static-package-dependents.ts` around lines 138 - 157, The current entrypoints_truncated logic uses matchingArtifactCount > Math.min(item.entrypoints.length, artifactsPerPackageLimit) which mixes artifact-level counts with deduped entrypoint rows; update the logic and docs to be consistent: either (A) change the boolean computation in static-package-dependents.ts (the entrypoints_truncated field) to compare matchingArtifactCount against artifactsPerPackageLimit (e.g. matchingArtifactCount > artifactsPerPackageLimit) so it reflects "more matching artifacts than returned", or (B) leave the code but update the staticDependentItemSchema docstring in publish-external-push.ts to explicitly say "True when the dependent has more matching artifacts than the entrypoints returned (artifacts may collapse to fewer entrypoints)"; pick one approach and apply it to the variables entrypoints_truncated, matchingArtifactCount, artifactsPerPackageLimit, and staticDependentItemSchema so semantics and docs align.packages/worker/src/repo/published-bundle-artifacts-repo.ts (1)
112-248: ⚡ Quick winSQL is correct; flag for operational follow-up at scale.
Both queries correctly bound results (
package_rank/artifact_rank) and stale-rank packages first, and the bind order matches the?positions. One operational note: the dependent-side filter isjson_extract(dependency.value, '$.sourceId') = ?, which D1/SQLite cannot satisfy with a normal column index onpublished_bundle_artifacts. Each publish for a user will effectively scan that user's artifacts and expanddependencies_jsonviajson_each. That's fine today but is worth keeping in mind as bundle counts grow.Possible follow-ups (no action needed in this PR):
- Add a lightweight
published_bundle_artifact_dependenciesassociation table (artifact_id, dependency_source_id, dependency_published_commit) populated alongside the existingdependencies_jsonupserts, with an index on(user_id, dependency_source_id). Then the count/list queries can target that table directly and stay sub-linear in artifact count.- Or, add a generated/expression index (where supported) on the JSON path, though D1's support for that is more limited.
- Add a query timing log or Sentry breadcrumb around
getStaticPackageDependentsSummaryso growth in this hot path on publish surfaces in observability.🤖 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/worker/src/repo/published-bundle-artifacts-repo.ts` around lines 112 - 248, The queries in countStaticDependentBundleArtifactPackages and listStaticDependentBundleArtifactRows currently filter on json_extract(dependency.value, '$.sourceId') = ? which forces per-user scans and json_each expansion as bundle counts grow; to address this operationally, add a lightweight published_bundle_artifact_dependencies association table (artifact_id, dependency_source_id, dependency_published_commit, user_id) populated during the same upsert that writes dependencies_json (or maintain it in the publish flow), index (user_id, dependency_source_id), and update getStaticPackageDependentsSummary to query that table instead of json_each; alternatively, add a generated/expression index on the JSON path where supported and add timing/log breadcrumbs around getStaticPackageDependentsSummary to surface regressions.packages/worker/src/mcp/capabilities/packages/publish-external-push.node.test.ts (1)
75-237: 💤 Low valueLGTM — mock + tests cover the new contract well, including
currentDependencyCommitplumbing.Asserting that
getStaticPackageDependentsSummarywas called withcurrentDependencyCommit: 'commit-new'is exactly the right hook to lock down the publish-time wiring. TheoutputTypeDefinitionsubstring checks also nicely guard the schema surface.Optional, low-priority: the
'No published bundle artifacts currently declare a static dependency on this package.'literal is duplicated acrossbeforeEach, the basicalready_publishedtest, and the retry test. Pulling it into a sharedconst(or importing it from the runtime module if exported) would prevent drift if the copy is tweaked.🤖 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/worker/src/mcp/capabilities/packages/publish-external-push.node.test.ts` around lines 75 - 237, The duplicate recommendation string is repeated in the test setup and two tests; introduce a single shared constant (e.g., const NO_STATIC_DEPENDENTS_MSG = 'No published bundle artifacts currently declare a static dependency on this package.') at the top of the test file and replace the three literal occurrences (the initial mockModule.getStaticPackageDependentsSummary.mockResolvedValue call in the beforeEach, the expected static_dependents.recommended_next_action in the 'returns already_published' test, and any other repeated assertions) with that constant, or alternatively import the canonical value from the runtime module if it is exported; update all references so the tests assert against NO_STATIC_DEPENDENTS_MSG instead of the hard-coded string.packages/worker/src/mcp/capabilities/packages/publish-external-push.ts (1)
266-317: 💤 Low valueOptional: collapse the three
static_dependentsaugmentation sites into one helper.The early
already_published, the post-publishFromExternalRefalready_published, and thepublishedbranches all repeat the same wiring (db: ctx.env.APP_DB, userId: user.userId, sourceId: source.id, then callgetPublishStaticDependents, then spread). A tiny helper makes the handler easier to read and keeps the three branches from drifting apart over time.♻️ Sketch
const augmentWithStaticDependents = async < T extends { published_commit: string | null }, >( result: T, ): Promise<T & { static_dependents: StaticPackageDependentsSummary }> => ({ ...result, static_dependents: await getPublishStaticDependents({ db: ctx.env.APP_DB, userId: user.userId, sourceId: source.id, publishedCommit: result.published_commit, }), })Then each branch becomes
return await augmentWithStaticDependents(...).🤖 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/worker/src/mcp/capabilities/packages/publish-external-push.ts` around lines 266 - 317, Extract the repeated augmentation logic that calls getPublishStaticDependents into a small helper (e.g. augmentWithStaticDependents) and replace the three places that manually spread static_dependents (the early already_published branch, the result.status === 'already_published' branch, and the published branch after repoSessionRpc(...).publishFromExternalRef) with a single call to that helper; the helper should accept the result object (typed to have published_commit) and return the result plus static_dependents using ctx.env.APP_DB, user.userId and source.id so callers simply do "return await augmentWithStaticDependents(resultOrInlineObject)".
🤖 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.
Nitpick comments:
In `@docs/use/packages.md`:
- Around line 75-78: Update the wording in the docs to replace the vague phrase
"when the metadata is available" with the precise precondition "when the
published commit is available" to match the implementation in
getPublishStaticDependents; find the occurrences around the paragraph describing
static snapshot semantics (lines referenced in the review) and change the
phrasing so the docs state that static dependents are applied when the published
commit is available, ensuring consistency with the getPublishStaticDependents
behavior.
In
`@packages/worker/src/mcp/capabilities/packages/publish-external-push.node.test.ts`:
- Around line 75-237: The duplicate recommendation string is repeated in the
test setup and two tests; introduce a single shared constant (e.g., const
NO_STATIC_DEPENDENTS_MSG = 'No published bundle artifacts currently declare a
static dependency on this package.') at the top of the test file and replace the
three literal occurrences (the initial
mockModule.getStaticPackageDependentsSummary.mockResolvedValue call in the
beforeEach, the expected static_dependents.recommended_next_action in the
'returns already_published' test, and any other repeated assertions) with that
constant, or alternatively import the canonical value from the runtime module if
it is exported; update all references so the tests assert against
NO_STATIC_DEPENDENTS_MSG instead of the hard-coded string.
In `@packages/worker/src/mcp/capabilities/packages/publish-external-push.ts`:
- Around line 266-317: Extract the repeated augmentation logic that calls
getPublishStaticDependents into a small helper (e.g.
augmentWithStaticDependents) and replace the three places that manually spread
static_dependents (the early already_published branch, the result.status ===
'already_published' branch, and the published branch after
repoSessionRpc(...).publishFromExternalRef) with a single call to that helper;
the helper should accept the result object (typed to have published_commit) and
return the result plus static_dependents using ctx.env.APP_DB, user.userId and
source.id so callers simply do "return await
augmentWithStaticDependents(resultOrInlineObject)".
In `@packages/worker/src/package-runtime/static-package-dependents.ts`:
- Around line 7-8: The second default constant
defaultStaticDependentArtifactsPerPackageLimit is module-private but should be
exported for symmetry and reuse; update its declaration to export it (export
const defaultStaticDependentArtifactsPerPackageLimit = 5) so callers/tests/docs
can reference the artifacts-per-package default just like
defaultStaticDependentPackageLimit and avoid duplicating the literal; ensure any
imports elsewhere use the exported name from static-package-dependents.
- Around line 138-157: The current entrypoints_truncated logic uses
matchingArtifactCount > Math.min(item.entrypoints.length,
artifactsPerPackageLimit) which mixes artifact-level counts with deduped
entrypoint rows; update the logic and docs to be consistent: either (A) change
the boolean computation in static-package-dependents.ts (the
entrypoints_truncated field) to compare matchingArtifactCount against
artifactsPerPackageLimit (e.g. matchingArtifactCount > artifactsPerPackageLimit)
so it reflects "more matching artifacts than returned", or (B) leave the code
but update the staticDependentItemSchema docstring in publish-external-push.ts
to explicitly say "True when the dependent has more matching artifacts than the
entrypoints returned (artifacts may collapse to fewer entrypoints)"; pick one
approach and apply it to the variables entrypoints_truncated,
matchingArtifactCount, artifactsPerPackageLimit, and staticDependentItemSchema
so semantics and docs align.
In `@packages/worker/src/repo/published-bundle-artifacts-repo.ts`:
- Around line 112-248: The queries in countStaticDependentBundleArtifactPackages
and listStaticDependentBundleArtifactRows currently filter on
json_extract(dependency.value, '$.sourceId') = ? which forces per-user scans and
json_each expansion as bundle counts grow; to address this operationally, add a
lightweight published_bundle_artifact_dependencies association table
(artifact_id, dependency_source_id, dependency_published_commit, user_id)
populated during the same upsert that writes dependencies_json (or maintain it
in the publish flow), index (user_id, dependency_source_id), and update
getStaticPackageDependentsSummary to query that table instead of json_each;
alternatively, add a generated/expression index on the JSON path where supported
and add timing/log breadcrumbs around getStaticPackageDependentsSummary to
surface regressions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b91b09e9-afca-40b8-aa37-f80f1beadbc0
📒 Files selected for processing (8)
docs/contributing/packages-and-manifests.mddocs/use/packages.mdpackages/worker/src/mcp/capabilities/packages/publish-external-push.node.test.tspackages/worker/src/mcp/capabilities/packages/publish-external-push.tspackages/worker/src/package-runtime/static-package-dependents.node.test.tspackages/worker/src/package-runtime/static-package-dependents.tspackages/worker/src/repo/published-bundle-artifacts-repo.node.test.tspackages/worker/src/repo/published-bundle-artifacts-repo.ts
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
| published_commit: string | null | ||
| packageStale: boolean | ||
| matchingArtifactCount: number | ||
| artifactRows: Array<StaticDependentBundleArtifactRow> |
There was a problem hiding this comment.
Accumulator artifactRows array populated but never read
Low Severity
The artifactRows field on StaticDependentPackageAccumulator is declared, initialized as an empty array, and receives a .push() for every row — but is never read anywhere in the summary-building logic. This is dead code that needlessly allocates and grows an array per dependent package.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit ab0d77c. Configure here.
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 4a7fff0. Configure here.
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
There was a problem hiding this comment.
🧹 Nitpick comments (5)
packages/worker/src/repo/published-bundle-artifacts-repo.workers.test.ts (3)
175-189: ⚡ Quick winAdd a comment explaining the duplicate entrypoint.
This artifact intentionally reuses entrypoint
'src/current-0.ts'to test that multiple artifacts can share the same entrypoint and are counted correctly. Adding a brief comment would make this test case more explicit.📝 Suggested comment
+ // Duplicate entrypoint to verify multiple artifacts can share the same entrypoint await insertArtifact({ userId, sourceId: sourceB,🤖 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/worker/src/repo/published-bundle-artifacts-repo.workers.test.ts` around lines 175 - 189, Add a brief inline comment above the insertArtifact call explaining that the entryPoint 'src/current-0.ts' is intentionally reused (duplicate entrypoint) to test that multiple artifacts can share the same entrypoint and are counted correctly; reference the insertArtifact invocation and artifactName 'a-current-0-importable' so reviewers know this is a deliberate test case rather than a copy-paste mistake.
258-259: 💤 Low valueDocument the entrypoint truncation limit.
The assertion expects exactly 5 entrypoints and verifies truncation behavior, but the limit of 5 is not documented. Consider adding a comment explaining this is testing the per-package entrypoint truncation limit.
📝 Suggested comment
+ // Verify entrypoint truncation at limit of 5 per package expect(summary.items[0]?.entrypoints).toHaveLength(5) expect(summary.items[0]?.entrypoints).not.toContain('src/stale.ts')🤖 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/worker/src/repo/published-bundle-artifacts-repo.workers.test.ts` around lines 258 - 259, The test assertions on summary.items[0]?.entrypoints assume a per-package entrypoint truncation limit of 5 but don't document it; update the test (in published-bundle-artifacts-repo.workers.test.ts) by adding a brief inline comment above the assertions referencing the truncation limit being tested (e.g., "tests per-package entrypoint truncation limit = 5") and why we check not-to-contain 'src/stale.ts', so future readers understand that expect(summary.items[0]?.entrypoints).toHaveLength(5) is intentionally enforcing the truncation behavior for the entrypoints collection.
242-267: ⚡ Quick winVerify that package D is excluded from results.
The test setup creates an obsolete artifact for package D (with
commit-d-obsoletewhile the package hascommit-d-current), which should be filtered out. The assertions validateitems[0]anditems[1]but don't explicitly confirm that package D is absent or thatitems.length === 2.✅ Proposed assertion to verify exclusion
expect(summary.items[1]).toEqual( expect.objectContaining({ name: `@kentcdodds/package-c-${unique}`, stale: true, bundled_dependency_commit: null, }), ) + // Verify package D with obsolete artifact is excluded + expect(summary.items).toHaveLength(2) + expect(summary.items.every(item => !item.name.includes('package-d'))).toBe(true) })🤖 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/worker/src/repo/published-bundle-artifacts-repo.workers.test.ts` around lines 242 - 267, Add explicit assertions to confirm package D is excluded by checking summary.items length equals 2 and that neither item has the package D name; use the existing summary and unique variables (e.g., assert summary.items.length === 2 and that summary.items.every(i => i.name !== `@kentcdodds/package-d-${unique}`)) so the test verifies the obsolete artifact for package D is filtered out.docs/contributing/packages-and-manifests.md (2)
73-84: ⚡ Quick winWell-documented static import contract.
The bundled snapshot semantics and manifest contract requirements are thoroughly explained. The distinction between type-only imports and bundled literal dynamic imports is clear.
Optional: Consider adding a brief example for literal dynamic imports
The explanation of literal dynamic
import("kody:@...")being bundled is accurate but abstract. A quick example might help readers:declaration files such as `.d.ts` are treated as type-only. Literal dynamic `import("kody:@...")` expressions are bundled snapshots too, so they must be - declared. + declared. For example, `const mod = await import("kody:`@scope/my-package`")` + is bundled and requires `"@scope/my-package"` in `kody.dependencies`.This is purely optional—the current explanation is sufficient.
🤖 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 `@docs/contributing/packages-and-manifests.md` around lines 73 - 84, The docs currently explain static `kody:@...` imports and the manifest contract but a short concrete example showing a literal dynamic import pattern and how to declare it would improve clarity; add a one-line example illustrating import("kody:`@scope/my-package`") being treated as a bundled snapshot and show the corresponding package.json entry under package.json#kody.dependencies (e.g., including "@scope/my-package"), and note that type-only imports and `.d.ts` files are excluded so they need not be listed.
236-237: ⚡ Quick winClarify what "bounded" means for the static_dependents summary.
The documentation mentions a "bounded summary" but doesn't specify the bounds (e.g., maximum number of dependents shown, truncation rules). Adding this detail would help readers understand what to expect in the response.
Suggested clarification
If the bounds are defined in the implementation, consider adding them here:
-with `static_dependents`, a bounded summary of direct saved packages whose +with `static_dependents`, a summary of up to [N] direct saved packages whoseOr reference where the bounds are enforced if they're documented elsewhere.
🤖 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 `@docs/contributing/packages-and-manifests.md` around lines 236 - 237, The phrase "bounded summary" in the static_dependents description is ambiguous; update the docs for static_dependents to explicitly state the bounds (e.g., maximum number of dependents returned, truncation/order rules, and whether counts are approximate) and either list the numeric limits and truncation behavior directly or add a clear cross-reference to the implementation/function that enforces them (e.g., mention the function or config that caps results for static_dependents). Ensure you reference the symbol static_dependents in the text so readers can find the related behavior and include where to look (implementation file or section) if the limits are maintained elsewhere.
🤖 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.
Nitpick comments:
In `@docs/contributing/packages-and-manifests.md`:
- Around line 73-84: The docs currently explain static `kody:@...` imports and
the manifest contract but a short concrete example showing a literal dynamic
import pattern and how to declare it would improve clarity; add a one-line
example illustrating import("kody:`@scope/my-package`") being treated as a bundled
snapshot and show the corresponding package.json entry under
package.json#kody.dependencies (e.g., including "@scope/my-package"), and note
that type-only imports and `.d.ts` files are excluded so they need not be
listed.
- Around line 236-237: The phrase "bounded summary" in the static_dependents
description is ambiguous; update the docs for static_dependents to explicitly
state the bounds (e.g., maximum number of dependents returned, truncation/order
rules, and whether counts are approximate) and either list the numeric limits
and truncation behavior directly or add a clear cross-reference to the
implementation/function that enforces them (e.g., mention the function or config
that caps results for static_dependents). Ensure you reference the symbol
static_dependents in the text so readers can find the related behavior and
include where to look (implementation file or section) if the limits are
maintained elsewhere.
In `@packages/worker/src/repo/published-bundle-artifacts-repo.workers.test.ts`:
- Around line 175-189: Add a brief inline comment above the insertArtifact call
explaining that the entryPoint 'src/current-0.ts' is intentionally reused
(duplicate entrypoint) to test that multiple artifacts can share the same
entrypoint and are counted correctly; reference the insertArtifact invocation
and artifactName 'a-current-0-importable' so reviewers know this is a deliberate
test case rather than a copy-paste mistake.
- Around line 258-259: The test assertions on summary.items[0]?.entrypoints
assume a per-package entrypoint truncation limit of 5 but don't document it;
update the test (in published-bundle-artifacts-repo.workers.test.ts) by adding a
brief inline comment above the assertions referencing the truncation limit being
tested (e.g., "tests per-package entrypoint truncation limit = 5") and why we
check not-to-contain 'src/stale.ts', so future readers understand that
expect(summary.items[0]?.entrypoints).toHaveLength(5) is intentionally enforcing
the truncation behavior for the entrypoints collection.
- Around line 242-267: Add explicit assertions to confirm package D is excluded
by checking summary.items length equals 2 and that neither item has the package
D name; use the existing summary and unique variables (e.g., assert
summary.items.length === 2 and that summary.items.every(i => i.name !==
`@kentcdodds/package-d-${unique}`)) so the test verifies the obsolete artifact
for package D is filtered out.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 33218d2d-818e-4526-81f7-387e26a34fd0
📒 Files selected for processing (15)
docs/contributing/packages-and-manifests.mddocs/use/packages.mdpackages/worker/src/package-registry/types.tspackages/worker/src/package-runtime/import-specifiers.node.test.tspackages/worker/src/package-runtime/import-specifiers.tspackages/worker/src/package-runtime/module-graph.tspackages/worker/src/package-runtime/published-bundle-artifacts.tspackages/worker/src/package-runtime/static-kody-imports.tspackages/worker/src/package-runtime/static-package-dependents.node.test.tspackages/worker/src/package-runtime/static-package-dependents.tspackages/worker/src/repo/checks.node.test.tspackages/worker/src/repo/checks.tspackages/worker/src/repo/published-bundle-artifacts-repo.node.test.tspackages/worker/src/repo/published-bundle-artifacts-repo.tspackages/worker/src/repo/published-bundle-artifacts-repo.workers.test.ts
✅ Files skipped from review due to trivial changes (2)
- packages/worker/src/package-runtime/import-specifiers.node.test.ts
- docs/use/packages.md
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/worker/src/repo/published-bundle-artifacts-repo.ts
- packages/worker/src/package-runtime/static-package-dependents.ts
- packages/worker/src/repo/published-bundle-artifacts-repo.node.test.ts
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
2 participants
Summary
package_publish_external_pushpublished/already_published responses.kody:@...imports must matchpackage.json#kody.dependenciesdeclarations.kody:@imports so they do not count as static dependencies or get bundled; declaration files are treated as type-only; literal dynamicimport("kody:@...")is bundled and must be declared.kody:@this-package/...exports, so dependent entrypoints are not over-reported and dead files do not break unrelated entrypoints.kody:@bundled snapshot semantics and dependent republish guidance.Validation
npx vitest run --project node-unit --project workers-unit packages/worker/src/package-runtime/import-specifiers.node.test.ts packages/worker/src/package-runtime/module-graph.node.test.ts packages/worker/src/repo/checks.node.test.ts packages/worker/src/package-runtime/published-bundle-artifacts.node.test.ts packages/worker/src/repo/published-bundle-artifacts-repo.workers.test.ts packages/worker/src/package-runtime/static-package-dependents.node.test.ts packages/worker/src/mcp/capabilities/packages/publish-external-push.node.test.ts(62 tests passed)npm run typechecknpm run validateSummary by CodeRabbit
Documentation
New Features
Tests