refactor: rename workflow registry entry type#215
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
The original goal was to close a bounded piece of issue #209 by removing the product-prefixed internal KapiRegistryEntry type name. The semantic scope is narrow: a type/interface rename across domain, registry store, CLI runtime/event/report code, presentation formatters, tests, and the product-name audit doc. There is no prior kapi-agent review/fix ping-pong on this PR, so no previous blocking feedback needed closure.
What changed
The domain export was renamed from KapiRegistryEntry to WorkflowRegistryEntry in src/domain/registry.ts, and internal type imports/usages were updated in:
src/adapters/registry-store.tssrc/cli/runctl-cli.tssrc/cli/worker-events.tssrc/cli/worker-runtime.tssrc/presentation/runctl-formatters.ts- related tests
docs/product-name-audit.md
Why this is correct
The change is type-name-only and preserves the serialized registry fields, runtime workflow IDs, event names, CLI output strings, registry paths, and persisted compatibility contracts. I verified there are no remaining KapiRegistryEntry references in src, test, or docs, and the package is private, so the intentional exported type rename does not create a public package compatibility concern beyond the documented internal cleanup.
Evidence
Verifier gate: PASS. npm ci && npm run verify exited 0.
Size gate: PASS. 132 changed lines, under the configured 200-line review threshold.
Revision-explanation status: not required for this first kapi-agent review; explanation found in the PR body.
Ilchul review harness gate: PASS. Harness returned neutral/comment with zero blocking findings.
Inspected files included src/domain/registry.ts, src/adapters/registry-store.ts, src/cli/runctl-cli.ts, src/cli/worker-events.ts, src/cli/worker-runtime.ts, src/presentation/runctl-formatters.ts, tests, and docs/product-name-audit.md.
Remaining risks and approval rationale
Remaining risk is limited to intentional internal TypeScript import churn for any out-of-tree consumers, but package.json marks the package private and the PR explicitly documents that risk. Approval is justified because the behavioral contract is unchanged, verification passed, the rename is complete, and documentation was updated alongside the implementation.
Blocking issues
None.
Warnings / risks
src/domain/registry.ts: removing the old exported type name is a breaking TypeScript import change for any untracked external consumers. This appears acceptable because the package is private and the rename is intentional for the internal naming cleanup.- Verification workspace showed local mode-only dirtiness on
src/cli/kapi-review-cli.ts, but that file is not part of the PR diff atHEAD^..HEAD; the PR body also notes this kind of command-induced mode churn was reset before staging.
Suggestions
- For future residual naming cleanup slices, keep the same pattern: one semantic cluster per PR, explicitly call out persisted/external literals that must not change, and keep the audit doc current.
Looks good
- The persisted registry object shape is unchanged; only TypeScript type names moved.
- The rename is consistently applied across domain, adapter, CLI, presentation, and tests.
- The product-name audit now accurately records this slice and leaves remaining service/store identifiers as follow-up work.
- Existing tests cover the touched registry store, runtime, event, formatter, and CLI surfaces.
Verification notes
Verifier gate: PASS — npm ci && npm run verify exited 0.
Size gate: PASS — 132 changed lines, below threshold.
Revision-explanation status: not required now; found in PR body.
PR/CI/local evidence reviewed: full verifier output, Ilchul harness result with zero blocking findings, git diff HEAD^..HEAD --stat, targeted diff inspection, and rg -n "KapiRegistryEntry|WorkflowRegistryEntry" src test docs package.json.
Engine: pi
There was a problem hiding this comment.
kapi-agent review
Verdict: APPROVE
Final approval summary
Review journey
The original goal was to close a bounded slice of issue #209 by renaming the internal product-prefixed registry entry type. The semantic scope is narrow and mechanical: KapiRegistryEntry became WorkflowRegistryEntry across the domain type export, registry store, CLI runtime/report code, worker helpers, presentation formatters, tests, and the product-name audit doc. A prior kapi-agent approval already exists for this same current head, and no unresolved blocking feedback remains.
What changed
The PR renames the registry entry model in:
src/domain/registry.tssrc/adapters/registry-store.tssrc/cli/runctl-cli.tssrc/cli/worker-events.tssrc/cli/worker-runtime.tssrc/presentation/runctl-formatters.ts- related tests
docs/product-name-audit.md
Why this is correct
The change is type-name-only. It does not alter serialized registry JSON fields, workflow IDs, event names, persisted paths, CLI behavior, or runtime compatibility contracts. The remaining Kapi literals are documented as external integration, persisted compatibility, or follow-up service/store naming surfaces.
Evidence
Verifier gate: PASS. npm ci && npm run verify completed successfully.
Size gate: PASS. 132 changed lines, below the configured 200-line threshold.
Revision-explanation status: not required now, and an explanation was found.
Ilchul review harness gate: PASS, neutral/comment result with zero blocking findings.
Inspected files include the domain registry type, registry store, runtime CLI usage sites, worker event/runtime helpers, presentation formatters, tests, and audit doc references.
Remaining risks and approval rationale
Remaining risk is limited to out-of-tree TypeScript consumers importing the old exported type name, but the PR explains this package is private and the rename is intentional for the internal naming cleanup. Approval is justified because all gates pass, the semantic scope is bounded, tests/typechecks pass, and I found no blocking correctness, safety, or contract issue.
Blocking issues
None.
Warnings / risks
src/domain/registry.ts: Removing the exportedKapiRegistryEntryname is a compile-time breaking change for any external TypeScript consumer. This appears acceptable given the stated private/internal package context, but it is the main compatibility consideration.
Suggestions
- If this package later becomes public, consider a temporary deprecated type alias for exported type renames like this. Not needed for this private cleanup slice.
Looks good
- The rename is consistently applied across source, tests, and docs.
- Persisted/runtime contracts remain stable: registry fields, workflow IDs, event names, and external
kapi-agentliterals are not changed. - The audit doc clearly distinguishes completed cleanup from remaining intentional or follow-up product-name surfaces.
Verification notes
Verifier gate: PASS — npm ci && npm run verify exited 0.
Size gate: PASS — 132 changed lines, below threshold.
Revision-explanation status: not required now; found.
Ilchul review harness: PASS — neutral/comment with zero blocking findings.
Additional inspection: searched for KapiRegistryEntry and reviewed the current head diff for the renamed type and affected call sites.
Engine: pi
Summary
KapiRegistryEntrytoWorkflowRegistryEntryacross domain, adapter, CLI, presentation, and tests.kapi-agentintegration literals.Linked issue
Refs #209
Problem
Issue #209 is removing reusable internal
kapiidentifiers after the Ilchul/runtime naming direction changed. The workflow registry entry type still usedKapiRegistryEntrythroughout the source tree even though it models a generic workflow runtime registry record, not a Kapi product boundary.That product-prefixed type leaked into the registry store, runtime observation, worker events, CLI report/doctor code, presentation formatters, and test fixtures.
Options considered
KapiRegistryEntryexported from the domain and import it asWorkflowRegistryEntryin callers.WorkflowRegistryEntryand update the adapter, CLI, presentation, and tests.Kapi*surface while preserving runtime behavior.KapiStore/ service surfaces in the same PR.Selected approach
Selected option 2.
The slice is still mechanical and reviewable while removing a complete semantic cluster. It deliberately avoids changing serialized data, runtime registry paths,
kapi.worker.*/kapi.pr.*event contracts, and larger service/store names that should remain separate follow-up slices.Implementation
src/domain/registry.tsKapiRegistryEntrytoWorkflowRegistryEntry.RegistryListResult.entriesto use the generic type.src/adapters/registry-store.tsWorkflowRegistryEntry.src/cli/runctl-cli.ts,src/cli/worker-events.ts,src/cli/worker-runtime.tsWorkflowRegistryEntry.src/presentation/runctl-formatters.tsWorkflowRegistryEntry.test/cli-args.test.ts,test/cli-worker-events.test.ts,test/cli-worker-runtime.test.ts,test/registry-store.test.ts,test/runctl-formatters.test.tsdocs/product-name-audit.mdWhy this fixes it
KapiRegistryEntryno longer appears insrc,test, or docs. The registry record model now uses a semantic workflow boundary name while preserving the persisted object fields and runtime behavior. This removes one of the larger residual internalKapi*identifier clusters from #209 without mixing in broader service/store refactors.Residual scan for this slice:
Changed-line size: 132 total changed lines (
67 insertions + 65 deletions).QA / Verification
npm test -- test/registry-store.test.ts test/cli-worker-events.test.ts test/cli-worker-runtime.test.ts test/runctl-formatters.test.ts test/cli-args.test.ts— pass; package script ran the fulltest/*.test.tssuite plus repeated args: 521 tests, 510 pass, 11 skipped.npm run check— pass.npm run check:unused— pass.npm run quality:budgets— pass with existing non-failing warning:code_smells=52.git diff --check— pass.rg -n 'KapiRegistryEntry' src test docs— no matches.Manual smoke, if applicable:
Anomalies observed
npm test -- test/registry-store.test.ts test/cli-worker-events.test.ts test/cli-worker-runtime.test.ts test/runctl-formatters.test.ts test/cli-args.test.tsinvokedtsx --test test/*.test.ts ...; 521 tests ran.src/cli/kapi-review-cli.tsexecutable bit locally.git diff --summary -- src/cli/kapi-review-cli.tsshowed mode-only churn.100644before staging.quality:budgetsstill warns oncode_smells=52.npm run quality:budgetsoutput.Risks / Follow-up
KapiService,KapiStore/FileKapiStore, and intentional external/persisted literals.KapiRegistryEntrywould need to update toWorkflowRegistryEntry; this package is currently private and the rename is intentional for the internal naming cleanup.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