chore: remove dead code left by the v1.0 single-entrypoint refactor - #1676
Open
GautamSharma99 wants to merge 3 commits into
Open
chore: remove dead code left by the v1.0 single-entrypoint refactor#1676GautamSharma99 wants to merge 3 commits into
GautamSharma99 wants to merge 3 commits into
Conversation
checkTriggerAction had no callers. run.ts calls checkContainsTrigger directly, and the `contains_trigger` output the wrapper set is not declared in action.yml, so nothing consumed it either. Removing it leaves @actions/core unused in this module, so that import goes too.
updateTrackingComment had no callers. Updating the tracking comment with a branch link is now handled by updateCommentLink in src/entrypoints/update-comment-link.ts, via checkAndCommitOrDeleteBranch. Its helpers in comments/common.ts are left in place: createJobRunLink and createCommentBody are still used by create-initial.ts, and createBranchLink remains exported and covered by test/comments-common.test.ts.
prepare.ts is an unreferenced duplicate of run.ts's prepare phase. Nothing imports it, action.yml does not invoke it, and no test covers it. The v1.0 refactor (anthropics#898) moved the action to a single run.ts entrypoint and stopped referencing it, but left the file behind. It has already drifted from the code that actually runs. run.ts gained WorkflowValidationSkipError handling and the skipped_due_to_workflow_validation_mismatch output in that same refactor, and propagates GITHUB_TOKEN/GH_TOKEN into the environment; prepare.ts has none of these. So the copy is not merely redundant — it is a stale description of the prepare phase. Keeping it also costs real maintenance: anthropics#1590 applied the workflow_run collaborator-permission change to prepare.ts and run.ts in parallel, so a maintainer paid twice for a change only one of which could ever execute. That is the hazard this removes — a plausible-looking file where a prepare-phase fix can be made with no runtime effect. CLAUDE.md already documents run.ts as the single entrypoint.
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
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.
Fixes #1668 (items 1–3). 167 deletions, no insertions.
Three independent commits, ordered least to most contentious, so any of them can be dropped without touching the others:
49b1663checkTriggerAction+ the now-unused@actions/coreimportd90aa6cupdateTrackingComment(comments/update-with-branch.ts)39aaabfsrc/entrypoints/prepare.tsItem 4 from the issue — the dead
ALLOWED_TOOLS/DISALLOWED_TOOLSexports increate-prompt/index.ts— is not touched. Its trailing comment says it is kept so an H1 report's pointed-to file stays in sync with the live fix, and only maintainers can judge whether that still holds.On the question I raised in #1668
I asked whether
prepare.tswas retained deliberately before sending a deletion. Since I couldn't get an answer first, I went looking for evidence rather than assuming. It points to "left behind", and I'd rather show the work than ask you to take my word:It has already drifted from the code that runs. The v1.0 refactor (#898) both moved the action to a single
run.tsentrypoint and introducedWorkflowValidationSkipErrorhandling.run.tsgot it;prepare.tsnever did:run.tsprepare.tsWorkflowValidationSkipErrorhandlingskipped_due_to_workflow_validation_mismatchoutputGITHUB_TOKEN/GH_TOKENto envSo it isn't a redundant-but-equivalent copy; it is a stale description of the prepare phase. Anyone reading it to understand that phase gets the wrong answer.
Keeping it costs real maintenance. #1590 (
acb0385, 4 Aug 2026) applied theworkflow_runcollaborator-permission change toprepare.tsandrun.tsin parallel — a maintainer paid for the same change twice, and only one copy could ever execute. That is precisely the hazard this removes.If it is being kept on purpose — a downstream consumer, or a documented manual invocation I couldn't find — drop commit
39aaabfand the other two still stand on their own.Verification
Nothing anywhere in the repo — source, tests, workflows, docs,
action.yml— still refers to any removed symbol or file.contains_triggeris included above because it was the only outputcheckTriggerActionproduced, and it is not declared inaction.yml.bun test— 920 pass, 0 failbun run typecheck— cleanbun run format:check— cleanOne thing I deliberately left
Deleting
update-with-branch.tsleavescreateBranchLinkincomments/common.tswith no production caller, thoughtest/comments-common.test.tsstill covers it. Its siblingscreateJobRunLinkandcreateCommentBodyare still used bycreate-initial.ts.I left it rather than expanding scope: removing it means deleting test cases too, and it's a generic link-formatting helper that a maintainer may want to keep alongside its siblings. Happy to remove it and the corresponding tests if you'd prefer — say the word and it's a one-line follow-up.