Skip to content

refactor: remove workflow service compatibility alias#221

Merged
devkade merged 1 commit into
devfrom
refactor/issue-209-workflow-service-tests-final
May 17, 2026
Merged

refactor: remove workflow service compatibility alias#221
devkade merged 1 commit into
devfrom
refactor/issue-209-workflow-service-tests-final

Conversation

@devkade
Copy link
Copy Markdown
Owner

@devkade devkade commented May 17, 2026

Summary

  • Migrates the two remaining large active test clusters from the temporary KapiService compatibility export to the semantic WorkflowService class.
  • Removes the temporary export { WorkflowService as KapiService } alias from src/application/workflow-service.ts.
  • Updates docs/product-name-audit.md so the service/store surface records WorkflowService as the active internal contract.

Linked issue

Refs #209

Problem

Issue #209 is normalizing reusable internal identifiers away from product names while preserving intentional external compatibility surfaces. Earlier slices renamed the source service to WorkflowService and migrated most tests, but two large active test clusters still imported and constructed KapiService through a temporary alias.

That left the final reusable service compatibility alias in active source even though the semantic class name already existed.

Options considered

  1. Migrate one large test cluster only
    • Pros: smallest possible follow-up PR.
    • Cons: leaves the temporary alias in source and requires another near-identical slice.
  2. Migrate both remaining active test clusters and remove the alias
    • Pros: completes the active KapiService service-surface cleanup while staying well below the size gate.
    • Cons: removes the named alias in one step, so out-of-tree imports of an internal temporary export would need to update.
  3. Keep the alias after migrating tests
    • Pros: maximum compatibility.
    • Cons: preserves the reusable product-name identifier this issue is explicitly retiring.

Selected approach

Selected option 2.

The final active consumers were only two test clusters, and the total diff is small enough for one reviewable PR. Public serialized kapi-* workflow IDs, slash-command contracts, and kapi-agent review integration literals remain unchanged.

Implementation

  • test/service-store.test.ts
    • Replaced KapiService import, constructor calls, type annotations, and ReturnType references with WorkflowService.
  • test/autoresearch-validation.test.ts
    • Replaced KapiService import, constructor calls, and helper type annotations with WorkflowService.
  • src/application/workflow-service.ts
    • Removed the temporary WorkflowService as KapiService export alias.
  • docs/product-name-audit.md
    • Updated the service/store classification and guard notes to reflect that active service tests now use WorkflowService and the temporary alias is gone.

Why this fixes it

The active service/store surface no longer exposes or consumes the product-named KapiService identifier. Tests now exercise the same class through its semantic export, and removing the alias prevents new reusable internal code from depending on the retired compatibility name.

QA / Verification

  • npm test -- test/service-store.test.ts test/autoresearch-validation.test.ts — pass; package script runs the full suite shape, 521 tests, 510 pass, 11 skipped.
  • npm run check — pass.
  • npm run check:unused — pass.
  • npm run quality:budgets — pass with existing non-failing code_smells=52 warning.
  • git diff --check — pass.
  • Static added-line scan for hardcoded secrets, shell injection, dynamic eval/exec, pickle, and SQL formatting — no matches.
  • Residual active-path scan: git grep -n "KapiService" -- ':!node_modules' ':!references' ':!.kapi' ':!.hermes/plans' — only docs explaining removal and the existing createLocalKapiService negative architecture assertion remain.
  • Independent reviewer subagent — pass; no security concerns or logic errors.

Anomalies observed

Symptom Evidence Impact Action
Targeted-looking npm test -- ... still ran the full suite package script is tsx --test test/*.test.ts; output showed 521 tests Slower than narrow targeting, but broad confidence Reported honestly in QA
Test execution flipped src/cli/kapi-review-cli.ts executable mode git diff --summary showed 100644 => 100755 Unrelated mode-only churn risk Restored with chmod 644 before commit

Risks / Follow-up

  • Out-of-tree imports of the temporary internal KapiService alias would need to update to WorkflowService; no active in-repo consumers remain.
  • This PR intentionally does not rename serialized /kapi-* workflow IDs, storage compatibility references, or external kapi-agent review literals.

kapi-agent review

  • Expected reviewer-only bot behavior: automatic formal review after PR open; if absent after a short wait, request with @kapi-agent review.
  • Merge gate: latest kapi-agent approval/check must apply to the current head before merge.
  • Current head: 63f6b5a.

@devkade
Copy link
Copy Markdown
Owner Author

devkade commented May 17, 2026

@kapi-agent review

Revision explanation for current head 63f6b5aa0672d608d42cb7ae96a4c4bf23650824:

What changed:

  • Migrated the final two active KapiService test clusters to WorkflowService.
  • Removed the temporary WorkflowService as KapiService source export alias.
  • Updated docs/product-name-audit.md to record the active service/store surface as semantic WorkflowService only.

Why this closes the prior feedback:

  • There is no prior kapi-agent feedback on this PR; this is the final bounded follow-up from PR test: migrate workflow service imports #220.
  • Active in-repo consumers no longer import or instantiate KapiService, so the compatibility alias can be removed safely.
  • Serialized /kapi-* workflow IDs and external kapi-agent literals are intentionally unchanged.

Verification:

  • npm test -- test/service-store.test.ts test/autoresearch-validation.test.ts — pass; package script ran full suite shape, 521 tests, 510 pass, 11 skipped.
  • npm run check — pass.
  • npm run check:unused — pass.
  • npm run quality:budgets — pass with existing non-failing code_smells=52 warning.
  • git diff --check — pass.
  • Static added-line scan — no secrets/shell/eval/pickle/sql matches.
  • Independent reviewer subagent — pass.

Copy link
Copy Markdown

@kapi-agent kapi-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.

kapi-agent review

Verdict: APPROVE

Final approval summary

Review journey

This PR completes the issue #209 cleanup for the workflow service naming surface. The semantic scope is narrow: migrate the last active in-repo test consumers from the temporary KapiService compatibility alias to WorkflowService, remove that alias from src/application/workflow-service.ts, and update the product-name audit documentation.

There was no prior kapi-agent review on this PR, so no unresolved ping-pong feedback remains.

What changed

  • src/application/workflow-service.ts
    • Removed export { WorkflowService as KapiService };.
  • test/service-store.test.ts
    • Replaced KapiService imports/usages/type references with WorkflowService.
  • test/autoresearch-validation.test.ts
    • Replaced KapiService imports/usages/type references with WorkflowService.
  • docs/product-name-audit.md
    • Updated the audit to state that active service/store tests now use WorkflowService and the temporary service alias is gone.

Why this is correct

The implementation already exposes WorkflowService as the primary semantic class. The PR removes only the product-named compatibility re-export after migrating the remaining in-repo active consumers. A residual scan shows no active src/test dependency on KapiService; remaining occurrences are documentation context and a negative architecture assertion for createLocalKapiService.

This preserves external/persisted Kapi contracts such as workflow IDs and review integration names while removing an internal reusable-code alias, which matches the stated product-name audit policy.

Evidence

  • Verifier gate: PASS — npm ci && npm run verify exited 0.
  • Size gate: PASS — 102 changed lines, below the 200-line review threshold.
  • Revision-explanation status: not required; explanation found.
  • Ilchul review harness gate: PASS — neutral/comment, no blocking findings.
  • Inspected files:
    • src/application/workflow-service.ts
    • test/service-store.test.ts
    • test/autoresearch-validation.test.ts
    • docs/product-name-audit.md
  • Additional local inspection:
    • Confirmed WorkflowService remains exported.
    • Confirmed the KapiService alias is absent from the service module.
    • Confirmed git grep finds no active source/test KapiService consumers beyond docs and the existing negative architecture assertion.

Remaining risks and approval rationale

Remaining risk is limited to out-of-tree consumers importing the temporary internal KapiService alias. The PR body calls this out, and the alias appears intentionally internal/temporary rather than a public contract. Approval is justified because the change is small, semantically coherent, documented, verified by the full project gate, and does not alter persisted workflow IDs or external Kapi integration literals.

Blocking issues

None.

Warnings / risks

  • src/application/workflow-service.ts: removing KapiService can break out-of-tree imports if anyone depended on the temporary internal alias. This is acceptable given the PR’s stated cleanup goal, but it is the main compatibility risk.
  • Verification appears to leave a local mode-only change on src/cli/kapi-review-cli.ts during test execution. This is not part of the committed PR diff, but the test-suite side effect is worth tracking separately if it recurs in CI/workspaces.

Suggestions

  • Consider adding or keeping an architecture guard that directly prevents reintroducing export { WorkflowService as KapiService } if this naming cleanup is important long-term.

Looks good

  • The source change is minimal and targeted.
  • Test changes are mechanical renames from KapiService to WorkflowService.
  • Documentation was updated alongside the implementation, keeping the product-name audit aligned with the current code.
  • External/persisted kapi-* contracts are intentionally left unchanged.

Verification notes

  • Verifier gate status: PASS — npm ci && npm run verify exited 0.
  • Size gate status: PASS — 102 changed lines < 200.
  • Revision-explanation status: not required; found.
  • Ilchul harness status: PASS — no blocking findings.
  • PR evidence includes full test/check/unused/budget verification and residual KapiService scanning.

Engine: pi

@devkade devkade merged commit 6be0055 into dev May 17, 2026
2 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.

1 participant