refactor: rename worker runtime helpers#210
Conversation
|
@kapi-agent review Revision explanation for current head What changed:
Why this closes the prior feedback:
Verification:
|
There was a problem hiding this comment.
kapi-agent review
Verdict: APPROVE
Final approval summary
Review journey
This PR addresses a bounded slice of issue #209 by renaming internal CLI worker/runtime helper filenames away from historical kapi-* names. The semantic scope is narrow: three internal helper module paths, their imports/tests, an architecture guard, and the product-name audit documentation. There is no prior kapi-agent review on this head, so no unresolved ping-pong feedback remains.
What changed
- Renamed:
src/cli/kapi-github-issue-context.ts→src/cli/github-issue-context.tssrc/cli/kapi-worker-events.ts→src/cli/worker-events.tssrc/cli/kapi-worker-runtime.ts→src/cli/worker-runtime.ts
- Updated
src/cli/runctl-cli.tsand direct tests to import the new semantic paths. - Added
test/architecture.test.tscoverage to prevent the old helper filenames from reappearing. - Updated
docs/product-name-audit.mdwith the rename slice and remaining filename exceptions.
Why this is correct
The implementation is primarily a rename/import refactor. The only source-code behavior change is the import path from worker-events.ts to worker-runtime.ts; the helper contents are otherwise preserved. The new architecture guard directly protects the intended workflow contract by checking both absence of the old filenames and presence of the new semantic filenames.
Evidence
- Verifier gate: PASS —
npm ci && npm run verifyexited 0. - Size gate: PASS — 55 changed lines, under the 200-line semantic review threshold.
- Revision-explanation status: not required; found.
- Ilchul review harness gate: PASS — neutral/comment result with 0 blocking findings.
- Inspected diff/files: renamed CLI helper modules,
src/cli/runctl-cli.ts, direct helper tests,test/architecture.test.ts, anddocs/product-name-audit.md.
Remaining risks and approval rationale
Remaining risk is low and limited to import-path churn. The PR intentionally does not complete the broader product-name cleanup, and the residual filenames are documented as follow-up or external-contract exceptions. Approval is justified because the scope is bounded, verification passed, tests cover the intended guard, and no blocking correctness, security, or workflow-contract issue was found.
Blocking issues
None.
Warnings / risks
docs/product-name-audit.mdstill lists broader residualkapisurfaces, including service/factory names and external integration names. This is acceptable for this slice but remains follow-up work.- The architecture guard is filename-specific, not a general product-name detector. That matches the PR’s bounded intent.
Suggestions
- In a future service/factory rename slice, consider adding a similarly targeted architecture guard for the next boundary to avoid accidental reintroduction.
Looks good
- The rename is mechanically narrow and avoids touching serialized event names such as
kapi.worker.*, preserving compatibility contracts. - Tests were updated to import the semantic helper paths.
- The product-name audit clearly documents what was changed and what remains intentionally unchanged.
Verification notes
Verifier gate: PASS — npm ci && npm run verify exited 0.
Size gate: PASS — 55 changed lines, below threshold.
Revision-explanation status: not required; found.
Ilchul review harness: PASS — no blocking findings.
Engine: pi
Summary
kapi-*filenames to semantic runtime names.github-issue-context,worker-events, andworker-runtimemodule paths.Linked issue
Refs #209
Problem
Issue #209 tracks broad removal of historical
kapiproduct-name leakage from internal code identifiers, filenames, exports, helper names, and reusable implementation strings.This slice addresses a bounded filename/import subset: three generic CLI helper modules still used
kapi-*filenames even though they are runtime/worker implementation helpers rather than retained public product surfaces.These names made the generic runtime event and GitHub issue probing helpers look like product-branded compatibility surfaces.
Options considered
src/kapifilenames in one PR — rename service/factory/review/helper filenames together.KapiService,KapiStore, and many tests/imports need a semantic service-boundary rename.Selected approach
Selected option 2.
This PR keeps the slice reviewable and avoids broad mechanical replacement. It renames only helper filenames whose boundary is already semantic (
worker,runtime, GitHub issue context), while preserving intentionally literal external contracts such askapi.worker.*,kapi.pr.*,kapi-agent, andkapi-review.Implementation
src/cli/kapi-github-issue-context.ts→src/cli/github-issue-context.tsrunctl-cli.tsand direct tests to import the semantic path.src/cli/kapi-worker-events.ts→src/cli/worker-events.ts./worker-runtime.js.src/cli/kapi-worker-runtime.ts→src/cli/worker-runtime.tstest/architecture.test.tssrc/cli/kapi-worker-*orsrc/cli/kapi-github-issue-context.tsfilenames reappear.docs/product-name-audit.mdWhy this fixes it
The old helper module paths can no longer be imported or reintroduced silently:
KapiService/ factory) instead of hiding them.This is a partial #209 slice, so the PR uses
Refs #209rather thanCloses #209.QA / Verification
npm test -- test/cli-worker-events.test.ts test/cli-worker-runtime.test.ts test/cli-github-issue-context.test.ts— pass; package script ran the fulltest/*.test.tssuite plus explicit files, 518 tests, 507 pass, 11 skipped.npm test -- test/architecture.test.ts— pass; package script ran the fulltest/*.test.tssuite plus explicit file, 519 tests, 508 pass, 11 skipped.npm run check— pass.npm run check:unused— pass.npm run quality:budgets— pass with existing non-failingcode_smells=52warning.git diff --check— pass.Residual scans:
Changed-line count:
Independent review:
requesting-code-reviewstatic scan — no findings.Anomalies observed
npm test -- <files>runs the full suite because the package script istsx --test test/*.test.ts.src/cli/kapi-review-cli.tsmode changed to executable during verification.git diff --summaryshowedmode change 100644 => 100755.kapisource hits remain.=686`.Risks / Follow-up
src/application/kapi-service.ts,src/adapters/kapi-service-factory.ts, and relatedKapiService/KapiStoreexports still need a later semantic service-boundary slice.src/cli/kapi-review-cli.tsremains intentional for the literal externalkapi-review/kapi-agentintegration.kapi.worker.*,kapi.pr.*, andkapi-autoresearchremain compatibility contracts.kapi-agent review
@kapi-agent review.REQUEST_CHANGES, orCOMMENT, request re-review with a same-comment current-head revision explanation:@kapi-agent reviewRevision explanation for current head <HEAD_SHA>What changedWhy this closes the prior feedbackVerification