Skip to content

chore: replace tsup auto-discovery with explicit entry list#179

Merged
derekmisler merged 3 commits into
docker:mainfrom
docker-agent:chore/remove-library-tsup-entries
May 6, 2026
Merged

chore: replace tsup auto-discovery with explicit entry list#179
derekmisler merged 3 commits into
docker:mainfrom
docker-agent:chore/remove-library-tsup-entries

Conversation

@docker-agent
Copy link
Copy Markdown
Contributor

Summary

tsup.config.ts previously auto-discovered every src/<name>/index.ts as a build entry, emitting 8 dist/*.js bundles. Four of those (add-reaction, check-org-membership, get-pr-meta, post-comment) are pure library helpers — no action.yml anywhere in the repo references their bundles as an entrypoint.

This PR replaces the glob-based auto-discovery with an explicit entry map containing only the four bundles that back actual GitHub Action entrypoints:

Bundle Action
dist/credentials.js .github/actions/setup-credentials/action.yml
dist/mention-reply.js .github/actions/mention-reply/action.yml
dist/security.js root action.yml
dist/signed-commit.js root action.yml

The four library sub-modules stay in src/ unchanged — they're still imported by src/mention-reply/index.ts and their unit tests still run. They just don't get their own top-level bundle anymore (follow-on to #178).

Verification

pnpm build   # emits exactly the 4 listed bundles ✅
pnpm test    # 100/100 unit tests pass ✅
biome ci .   # clean ✅
tsc --noEmit # clean ✅

The four library sub-modules (add-reaction, check-org-membership,
get-pr-meta, post-comment) have no standalone action.yml and are only
imported by mention-reply. Stop emitting dist bundles for them.

Only the four dist files that back actual actions are now built:
  dist/credentials.js
  dist/mention-reply.js
  dist/security.js
  dist/signed-commit.js

Assisted-By: docker-agent
@docker-agent docker-agent requested a review from a team as a code owner May 6, 2026 18:23
@docker-agent docker-agent requested a review from derekmisler May 6, 2026 18:23
Copy link
Copy Markdown
Contributor

@docker-agent docker-agent Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assessment: 🟢 APPROVE

Summary: This PR replaces tsup's auto-discovery glob with an explicit 4-entry map containing only the bundles that back actual GitHub Action entrypoints (credentials, mention-reply, security, signed-commit). The four removed entries (add-reaction, check-org-membership, get-pr-meta, post-comment) are confirmed internal library helpers — a full repo-wide grep found zero references to their standalone dist/*.js paths in any workflow, action YAML, or script. With splitting: false and noExternal: [/.*/], these modules are already fully inlined into dist/mention-reply.js at build time, so removing their top-level bundles has no runtime impact.

All build settings (createRequire banner, platform: 'node', target: 'node24', noExternal, sourcemap: false, clean: true) are preserved unchanged.

Note: AGENTS.md still documents the old auto-discovery convention ("to add a new sub-action helper, just create src/<name>/index.ts"). Future contributors will need to also update tsup.config.ts — a follow-up update to that doc would prevent confusion.

Copy link
Copy Markdown
Contributor Author

@docker-agent docker-agent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test review

Comment thread tsup.config.ts Outdated
// Library sub-modules (add-reaction, check-org-membership, get-pr-meta,
// post-comment) are imported by mention-reply but have no standalone action,
// so they don't get their own top-level dist bundle.
const src = (name: string) => resolve(import.meta.dirname, 'src', name, 'index.ts');
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[MEDIUM] Test comment

Test.

Copy link
Copy Markdown
Contributor Author

@docker-agent docker-agent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test

Comment thread tsup.config.ts
.map((d) => [d.name, resolve(srcDir, d.name, 'index.ts')]),
);
// Explicit entry list: only modules that back an actual action.yml entrypoint.
// Library sub-modules (add-reaction, check-org-membership, get-pr-meta,
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test

Copy link
Copy Markdown
Contributor Author

@docker-agent docker-agent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test

Comment thread tsup.config.ts
);
// Explicit entry list: only modules that back an actual action.yml entrypoint.
// Library sub-modules (add-reaction, check-org-membership, get-pr-meta,
// post-comment) are imported by mention-reply but have no standalone action,
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test

Copy link
Copy Markdown
Contributor Author

@docker-agent docker-agent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test

Comment thread tsup.config.ts
// Explicit entry list: only modules that back an actual action.yml entrypoint.
// Library sub-modules (add-reaction, check-org-membership, get-pr-meta,
// post-comment) are imported by mention-reply but have no standalone action,
// so they don't get their own top-level dist bundle.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test

Copy link
Copy Markdown
Contributor Author

@docker-agent docker-agent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test

Comment thread tsup.config.ts
const src = (name: string) => resolve(import.meta.dirname, 'src', name, 'index.ts');
const entry = {
credentials: src('credentials'),
'mention-reply': src('mention-reply'),
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test

Copy link
Copy Markdown
Contributor Author

@docker-agent docker-agent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test

Comment thread tsup.config.ts
const entry = {
credentials: src('credentials'),
'mention-reply': src('mention-reply'),
security: src('security'),
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test

Copy link
Copy Markdown
Contributor Author

@docker-agent docker-agent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test

Comment thread tsup.config.ts
'mention-reply': src('mention-reply'),
security: src('security'),
'signed-commit': src('signed-commit'),
};
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test

Auto-discovery was replaced with an explicit entry map in tsup.config.ts.
Document that new action helpers require updating both src/ and the entry
map.

Assisted-By: docker-agent
Copy link
Copy Markdown
Contributor Author

@docker-agent docker-agent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test

Comment thread tsup.config.ts
.filter((d) => d.isDirectory() && existsSync(resolve(srcDir, d.name, 'index.ts')))
.map((d) => [d.name, resolve(srcDir, d.name, 'index.ts')]),
);
// Explicit entry list: only modules that back an actual action.yml entrypoint.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test line 4 with commit_id

Copy link
Copy Markdown
Contributor Author

@docker-agent docker-agent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test

Comment thread tsup.config.ts
// post-comment) are imported by mention-reply but have no standalone action,
// so they don't get their own top-level dist bundle.
const src = (name: string) => resolve(import.meta.dirname, 'src', name, 'index.ts');
const entry = {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test

Copy link
Copy Markdown
Contributor Author

@docker-agent docker-agent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test

Comment thread tsup.config.ts
credentials: src('credentials'),
'mention-reply': src('mention-reply'),
security: src('security'),
'signed-commit': src('signed-commit'),
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test

Copy link
Copy Markdown
Contributor Author

@docker-agent docker-agent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test

Comment thread tsup.config.ts
// Explicit entry list: only modules that back an actual action.yml entrypoint.
// Library sub-modules (add-reaction, check-org-membership, get-pr-meta,
// post-comment) are imported by mention-reply but have no standalone action,
// so they don't get their own top-level dist bundle.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test

Copy link
Copy Markdown
Contributor Author

@docker-agent docker-agent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assessment: 🟡 NEEDS ATTENTION

Comment thread tsup.config.ts
.map((d) => [d.name, resolve(srcDir, d.name, 'index.ts')]),
);
// Explicit entry list: only modules that back an actual action.yml entrypoint.
// Library sub-modules (add-reaction, check-org-membership, get-pr-meta,
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[MEDIUM] AGENTS.md still documents auto-discovery after this PR removes it

AGENTS.md line 111 currently states:

"tsup.config.ts auto-discovers entries — to add a new sub-action helper, just create src/<name>/index.ts."

This is now factually incorrect. After this PR the build system uses an explicit allowlist. A developer following the AGENTS.md instructions will create src/<new-module>/index.ts, run pnpm build, and see no dist/<new-module>.js produced — with no error explaining why.

AGENTS.md should be updated to:

  1. Remove the "auto-discovers entries" sentence from the TypeScript / src rules section
  2. Add: "To add a new action entrypoint, create src/<name>/index.ts and add the corresponding entry to the explicit map in tsup.config.ts."
  3. Update the repo-layout comments for add-reaction, check-org-membership, get-pr-meta, post-comment — they currently say # Entry → bundled to dist/<name>.js but they are no longer standalone dist bundles.

Comment thread tsup.config.ts
// Explicit entry list: only modules that back an actual action.yml entrypoint.
// Library sub-modules (add-reaction, check-org-membership, get-pr-meta,
// post-comment) are imported by mention-reply but have no standalone action,
// so they don't get their own top-level dist bundle.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[MEDIUM] src() helper loses the existsSync guard from the old auto-discovery code

The old auto-discovery used existsSync to filter only directories that actually had an index.ts. The new explicit src() helper is a bare resolve() call with no existence check:

const src = (name: string) => resolve(import.meta.dirname, 'src', name, 'index.ts');

All four current entries (credentials, mention-reply, security, signed-commit) exist today, so there is no immediate breakage. However, the guard rail has been removed: if a future edit misspells a name or deletes a module without updating this map, tsup may silently produce an incomplete dist/ that gets committed and shipped to action consumers with no build-time signal.

Consider adding a simple guard to fail loudly:

import { existsSync } from 'node:fs';
const src = (name: string) => {
  const p = resolve(import.meta.dirname, 'src', name, 'index.ts');
  if (!existsSync(p)) throw new Error(`tsup entry not found: ${p}`);
  return p;
};

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in cda493d — added existsSync back to the import and expanded src() to throw with a clear message if the file is missing:

import { existsSync } from 'node:fs';
// ...
const src = (name: string) => {
  const p = resolve(import.meta.dirname, 'src', name, 'index.ts');
  if (!existsSync(p)) throw new Error(`tsup entry not found: ${p}`);
  return p;
};

@derekmisler derekmisler merged commit c8feaa7 into docker:main May 6, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants