Skip to content

refactor: rename adapter workflow helpers#223

Merged
devkade merged 1 commit into
devfrom
refactor/issue-209-adapter-helper-names
May 17, 2026
Merged

refactor: rename adapter workflow helpers#223
devkade merged 1 commit into
devfrom
refactor/issue-209-adapter-helper-names

Conversation

@devkade
Copy link
Copy Markdown
Owner

@devkade devkade commented May 17, 2026

Summary

  • rename product-prefixed private helper names in adapter/domain internals to semantic workflow/state names
  • add an architecture guard for those internal helper names
  • update the product-name audit with the adapter/domain helper slice

Linked issue

Refs #209

Problem

Issue #209 is removing reusable internal kapi implementation leakage while preserving public/persisted Kapi contracts. After the presentation-helper slice, remaining active code still had private helper names such as prepareWritableKapiFile, assertKapiAncestorsAreNotSymlinks, removeKapiOwnedSymlink, assertKapiOwnedBranchName, hasValidKapiBranchPrefix, and formatKapiRules.

Options considered

  1. Rename only adapter helpers.
    • Pros: smallest slice.
    • Cons: leaves domain helper leaks for another tiny PR.
  2. Rename adapter and domain private helpers together.
    • Pros: one bounded semantic cluster; still well under the review size gate.
    • Cons: touches multiple files.
  3. Rename user-facing Kapi messages too.

Selected approach

Selected option 2. This PR changes only internal private helper identifiers and leaves user-facing text, slash commands, tool names, workflow IDs, and external integration names unchanged.

Implementation by file/surface

  • src/adapters/file-store.ts: prepareWritableStateFile, assertStateAncestorsAreNotSymlinks.
  • src/adapters/autoresearch-bridge.ts: removeManagedSymlink, isManagedAutoresearchTarget.
  • src/adapters/worker-substrate.ts: assertWorkflowOwnedBranchName.
  • src/domain/workflow-validation.ts: hasValidWorkflowBranchPrefix.
  • src/domain/run-contract-prompt-renderer.ts: formatWorkflowRules.
  • test/architecture.test.ts: regression guard for the old helper names.
  • docs/product-name-audit.md: records this cleanup slice.

Why this fixes it

The renamed symbols were reusable internal helper concepts, not public compatibility surfaces. Semantic names keep the boundary explicit while preserving Kapi-labeled runtime/user contracts.

QA / Verification

  • npm test -- test/architecture.test.ts — pass; package script runs the full test/*.test.ts suite plus the requested file.
  • 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.

Anomalies observed

  • npm test -- test/architecture.test.ts runs the full suite because the package script expands test/*.test.ts before the extra argument.
  • Verification briefly toggled src/cli/kapi-review-cli.ts executable mode; it was reset before commit.

Risks / Follow-up

  • Risk is low: this is private identifier-only refactoring with passing tests/type checks.
  • Follow-up remains: continue classifying/removing active internal reusable Kapi*/kapi* names while preserving documented public contracts.

kapi-agent review expectations

  • Current-head kapi-agent approval and kapi-agent/review success are required before merge.

@devkade
Copy link
Copy Markdown
Owner Author

devkade commented May 17, 2026

@kapi-agent review

Head SHA: 7b535a9

What changed:

  • Renamed product-prefixed private adapter/domain helper names to semantic workflow/state names.
  • Added an architecture guard for the old helper names.
  • Updated docs/product-name-audit.md for this slice.

Why this closes prior feedback:

Verification:

  • npm test -- test/architecture.test.ts (package script ran full test/*.test.ts suite plus requested file)
  • npm run check
  • npm run check:unused
  • npm run quality:budgets
  • git diff --check

Residual scan:

rg -n 'prepareWritableKapiFile|assertKapiAncestorsAreNotSymlinks|removeKapiOwnedSymlink|isKapiAutoresearchTarget|assertKapiOwnedBranchName|hasValidKapiBranchPrefix|formatKapiRules' src docs
# no matches

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

The PR’s original goal is to continue issue #209 by removing product-prefixed internal helper names while preserving public and persisted Kapi contracts. The semantic scope is narrow: private adapter/domain helper renames plus an architecture regression guard and one audit-doc update. There was no prior kapi-agent feedback cycle for this PR, so no unresolved ping-pong items remain.

What changed

The PR renames internal helpers in:

  • src/adapters/file-store.ts
  • src/adapters/autoresearch-bridge.ts
  • src/adapters/worker-substrate.ts
  • src/domain/workflow-validation.ts
  • src/domain/run-contract-prompt-renderer.ts

It also adds a guard in test/architecture.test.ts and records the cleanup in docs/product-name-audit.md.

Why this is correct

The changed identifiers are private implementation helpers. The diffs preserve call sites and logic while replacing product-prefixed names with semantic workflow/state names. User-facing messages, persisted workflow IDs, tool names, slash commands, and external kapi-agent/kapi-review contracts remain unchanged, which matches the stated migration boundary.

Evidence

  • Verifier gate: PASS.
  • Size gate: PASS, 67 changed lines under the semantic review threshold.
  • Revision-explanation status: not required; explanation present in PR body.
  • Ilchul review harness: PASS as gate evidence, neutral/comment with no blocking findings.
  • Verified evidence includes npm ci && npm run verify passing, including tests, type/check gates, unused checks, and quality budgets.
  • Inspected changed source, test, and audit-doc paths.

Remaining risks and approval rationale

Remaining risk is low and limited to missed internal naming cases outside this slice. The added architecture guard reduces regression risk for the renamed helper set. Approval is justified because the PR is bounded, behavior-preserving, verified, and does not alter public or persisted contracts.

Blocking issues

None.

Warnings / risks

  • test/architecture.test.ts guards a specific set of old helper names. Future product-name leakage outside that explicit regex will still need separate audit coverage.
  • Local review workspace showed an uncommitted executable-bit change on src/cli/kapi-review-cli.ts, but it is not part of the PR head commit inspected here.

Suggestions

  • As follow-up slices continue, consider keeping the audit guard list close to the latest residual scan so it remains easy to tell which product-name hits are intentionally public/persisted versus internal leakage.

Looks good

  • The refactor is private identifier-only and preserves runtime behavior.
  • Public Kapi contract text and external integration names are intentionally retained.
  • The architecture test creates a concrete regression guard for the renamed adapter/domain helper surface.
  • docs/product-name-audit.md is updated in the same change, keeping documentation aligned with implementation.

Verification notes

  • Verifier gate status: PASS — npm ci && npm run verify exited 0.
  • Size gate status: PASS — 67 changed lines, below 200.
  • Revision-explanation status: not required now; found in PR body.
  • Ilchul harness gate: PASS — no blocking findings.
  • PR evidence also reports targeted QA commands and git diff --check passing.

Engine: pi

@devkade devkade merged commit dc00430 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