refactor: extract authored artifact metadata helpers

[devkade]

Summary

• Extracted authored artifact metadata synchronization helpers from KapiService into src/application/authored-artifact-metadata.ts.
• Kept KapiService orchestration unchanged while delegating authored metadata header synchronization and decision-report ordering to the helper module.
• Added focused helper tests covering synced artifact names, context/interview/decision-report header replacement, decision-report ordering, and no-op behavior for artifacts without Kapi metadata headers.

Linked issue

Closes #136

Problem

src/application/kapi-service.ts owned small authored artifact metadata utilities inline, adding unrelated helper behavior to an already broad service class/file. The behavior is still needed for terminal workflow completion so context/interview/decision-report metadata stays current and decision evidence logs do not precede the primary decision report heading.

Options considered

1. Leave helpers inline.
   • Pros: lowest risk.
   • Cons: does not address the requested extraction; keeps formatting/sync policy in the orchestration service.
2. Split larger portions of KapiService.
   • Pros: could reduce the service size more broadly.
   • Cons: out of scope for this child slice and would reduce reviewability.
3. Extract only the authored artifact metadata constants/functions to a small application helper.
   • Pros: behavior-preserving, narrow, reviewable, and directly matches the issue scope.
   • Cons: leaves broader KapiService refactors for separate slices.

Selected approach

Option 3: move AUTHORED_METADATA_ARTIFACTS, authoredArtifactMetadataHeader, syncAuthoredArtifactMetadata, and normalizeDecisionReportOrder into a dedicated application helper module, then import the needed surface from KapiService.

Implementation by file/surface

• src/application/authored-artifact-metadata.ts
  • New helper module exporting the authored artifact list and metadata sync helpers.
  • Preserves the existing metadata header format and decision-report ordering logic.
• src/application/kapi-service.ts
  • Imports AUTHORED_METADATA_ARTIFACTS and syncAuthoredArtifactMetadata.
  • Removes the inline helper definitions.
• test/authored-artifact-metadata.test.ts
  • Adds focused unit coverage for synced artifact names, context header replacement, interview header replacement, decision-report stale header replacement plus ordering, and user-authored no-op behavior.

Why this fixes it

The authored artifact metadata behavior now lives behind a small application helper module instead of being embedded in KapiService. KapiService still iterates the same artifact names and writes synced content only when the helper returns changed content, so context/interview/decision-report metadata synchronization and decision-report primary heading ordering are preserved without changing artifact names, storage layout, or .kapi compatibility.

QA / Verification

• npm test -- test/workflow-service.test.ts test/artifact-overwrite.test.ts test/artifact-metadata.test.ts test/authored-artifact-metadata.test.ts — pass (407 tests passed, 11 skipped; package script runs test/*.test.ts plus provided args)
• npm test -- test/authored-artifact-metadata.test.ts test/artifact-overwrite.test.ts — pass (408 tests passed, 11 skipped; package script runs test/*.test.ts plus provided args)
• npm run check — pass
• npm run check:unused — pass
• npm run quality:budgets — pass with existing non-failing code_smells budget warning (50 vs target <=20)
• git diff --check — pass

Anomalies observed

• tsx emits Node [DEP0205] module.register() deprecation warnings during tests; tests still passed.
• Supervisor review found the first helper test slice did not directly cover interview.md metadata sync or stale decision-report.md header replacement; this head adds those focused assertions.
• Verification temporarily flipped src/cli/kapi-review-cli.ts to executable mode; the mode-only churn was reset and is not included in this PR.

Risks / Follow-up

• Risk is low: the helper code is a direct extraction plus focused tests.
• The metadata-header matcher remains intentionally format-sensitive to Kapi-generated authored artifact headers; future header format changes should update this helper and tests together.

kapi-agent review expectations and current-head merge gate

• Review should focus on the extraction boundary and preservation of authored artifact metadata behavior.
• This PR intentionally avoids broader KapiService splitting, artifact/storage changes, compatibility changes, and destructive cleanup/deletion.
• Previous kapi-agent approval was for stale head 6f6c5d43a7a8b69575eba8ed551a24b421f985b0; current head is fa2b53762ce74b91041d7ced9f51f324a3a4584a, so current-head review/check must run again before merge readiness.

Revision explanation

Revision explanation for current head 2c8372b2da7a1266656ee71d119771eeec52e1e3:

What changed:

• Extracted authored artifact metadata/header synchronization helpers into src/application/authored-artifact-metadata.ts.
• Preserved KapiService behavior by delegating to the extracted helper.
• Added focused tests for metadata headers and decision-report ordering.

Why this closes the prior feedback:

• The latest review reported no code-level blockers and only required a current-head revision explanation.
• This PR body now carries the explanation so the next review/check can evaluate it before the review run starts.

Verification:

• npx tsx --test test/authored-artifact-metadata.test.ts test/artifact-overwrite.test.ts — pass
• npm run check — pass
• npm run check:unused — pass
• git diff --cached --check — pass before commit

[kapi-agent]

kapi-agent review

Verdict: APPROVE

Final approval summary

Review journey

The original goal was to refactor authored artifact metadata synchronization out of KapiService into a smaller helper module. The semantic scope is narrow: preserve existing metadata header synchronization and decision-report ordering behavior while moving the helper logic and adding focused tests. There was no prior kapi-agent review on this PR, so no fix ping-pong was required.

What changed

• Added src/application/authored-artifact-metadata.ts with:
  • authored artifact constants
  • metadata header generation
  • metadata sync logic
  • decision-report ordering normalization
• Updated src/application/kapi-service.ts to import and use the extracted helper surface.
• Added test/authored-artifact-metadata.test.ts covering the extracted behavior.

Why this is correct

The service still iterates the same authored artifact names and still only writes when synced content differs. The extracted logic matches the prior inline behavior and keeps decision evidence logs after the primary # Decision Report heading when applicable. The tests directly exercise the helper’s public behavior, reducing regression risk from the extraction.

Evidence

• Verifier gate: PASS — npm ci && npm run verify exited 0.
• Size gate: PASS — 99 changed lines, under the 200-line limit.
• Ilchul review harness gate: PASS — neutral/comment result, 0 blocking findings.
• Revision explanation status: not required, found.
• Inspected files:
  • src/application/authored-artifact-metadata.ts
  • src/application/kapi-service.ts
  • test/authored-artifact-metadata.test.ts

Remaining risks and approval rationale

Remaining risk is low and limited to the regex-based metadata-header contract, which was preserved from the existing implementation and now has focused tests. Approval is justified because all gates pass, the PR is small, the behavior-preserving extraction is well-scoped, and no blocking correctness, safety, or contract issues were found.

Blocking issues

None.

Warnings / risks

• src/application/authored-artifact-metadata.ts: the metadata header matcher remains format-sensitive. That appears intentional and preserves the existing artifact contract, but future header format changes should update this helper and tests together.

Suggestions

• Consider adding a short comment above the header regex if future contributors may not know it is intentionally matching only Kapi-generated authored artifact headers.

Looks good

• The extraction boundary is clean: KapiService keeps orchestration, while metadata formatting/sync details move to a focused application helper.
• Tests cover the important behavior: artifact list, header replacement, decision-report heading ordering, and no-op behavior for user-authored files without Kapi metadata.
• No storage layout, artifact names, or workflow compatibility changes are introduced.

Verification notes

• Verifier gate status: PASS — npm ci && npm run verify exited 0.
• Size gate status: PASS — 99 changed lines < 200.
• Revision-explanation status: not required; PR body includes a clear explanation.
• Local inspection matched the PR’s stated behavior and did not reveal blocking issues.

---

Engine: pi

[kapi-agent]

kapi-agent review

Verdict: REQUEST_CHANGES

Blocking issues

• Revision explanation gate failed. This PR was updated after a prior kapi-agent approval, but there is no PR body update or author comment explaining what changed and why. Please add a brief revision response before requesting re-review, specifically covering:
  • what changed since the prior kapi-agent approval,
  • why the change was needed,
  • what verification was rerun.

Warnings / risks

• No code-level blocking risks found in the provided diff. The extraction in src/application/authored-artifact-metadata.ts appears behavior-preserving relative to the removed inline helpers in src/application/kapi-service.ts.

Suggestions

• Consider keeping the revision response concise but explicit, so reviewers can distinguish semantic changes from rerun/no-op synchronization updates.

Looks good

• The helper extraction is narrow and maintainable.
• KapiService keeps orchestration responsibility while delegating authored metadata synchronization.
• Focused tests in test/authored-artifact-metadata.test.ts cover artifact list exposure, metadata header replacement, decision-report ordering, and no-op behavior for non-Kapi-authored content.

Verification notes

• Verification gate status: FAIL due to revision explanation gate failure.
• Size gate status: PASS — 109 changed lines, below the 200-line limit.
• Revision-explanation status: FAIL — required now, not found.
• Verifier evidence: PASS — npm ci && npm run verify exited 0.
• Ilchul review harness: PASS as gate evidence — conclusion neutral, verdict COMMENT, blocking findings 0.
• Inspected changed surfaces: src/application/authored-artifact-metadata.ts, src/application/kapi-service.ts, and test/authored-artifact-metadata.test.ts.

---

Engine: pi

[devkade]

@kapi-agent review

Revision explanation for current head fa2b53762ce74b91041d7ced9f51f324a3a4584a:

What changed:

• Added direct interview.md authored metadata sync coverage in test/authored-artifact-metadata.test.ts.
• Changed the decision-report helper test to start from a stale generated metadata header, proving both stale header replacement and primary-heading ordering in the same assertion.
• Reset unrelated mode-only churn on src/cli/kapi-review-cli.ts; current PR files are only the extracted helper, the KapiService import/removal, and the helper tests.

Why this closes the prior feedback:

• A supervisor subagent review found the first head did not directly prove interview.md header sync or stale decision-report.md header replacement. The current head covers context, interview, and decision-report sync explicitly while preserving the original extraction boundary.

Verification:

• npm test -- test/workflow-service.test.ts test/artifact-overwrite.test.ts test/artifact-metadata.test.ts test/authored-artifact-metadata.test.ts — pass (407 pass, 11 skipped; package script runs all test/*.test.ts plus provided args)
• npm test -- test/authored-artifact-metadata.test.ts test/artifact-overwrite.test.ts — pass (408 pass, 11 skipped; package script runs all test/*.test.ts plus provided args)
• npm run check — pass
• npm run check:unused — pass
• npm run quality:budgets — pass with existing non-failing code_smells warning
• git diff --check — pass

[devkade]

@kapi-agent review

Revision explanation for current head fa2b53762ce74b91041d7ced9f51f324a3a4584a:

What changed:

• Added direct interview.md authored metadata sync coverage in test/authored-artifact-metadata.test.ts.
• Changed the decision-report helper test to start from a stale generated metadata header, proving both stale header replacement and primary-heading ordering in the same assertion.
• Reset unrelated mode-only churn on src/cli/kapi-review-cli.ts; current PR files are only the extracted helper, the KapiService import/removal, and the helper tests.

Why this closes the prior feedback:

• The current-head kapi-agent review requested a revision explanation after the post-approval update. This comment is posted after that review and explains the exact semantic delta from the prior approved head.
• The added tests close the supervisor-found coverage gap for direct interview.md sync and stale decision-report.md sync while preserving the original helper extraction boundary.

Verification:

• npm test -- test/workflow-service.test.ts test/artifact-overwrite.test.ts test/artifact-metadata.test.ts test/authored-artifact-metadata.test.ts — pass (407 pass, 11 skipped; package script runs all test/*.test.ts plus provided args)
• npm test -- test/authored-artifact-metadata.test.ts test/artifact-overwrite.test.ts — pass (408 pass, 11 skipped; package script runs all test/*.test.ts plus provided args)
• npm run check — pass
• npm run check:unused — pass
• npm run quality:budgets — pass with existing non-failing code_smells warning
• git diff --check — pass

[devkade]

@kapi-agent review

Revision explanation for current head fa2b53762ce74b91041d7ced9f51f324a3a4584a:

What changed:

• Moved authored artifact metadata/header synchronization helpers out of KapiService into src/application/authored-artifact-metadata.ts.
• Kept KapiService behavior-preserving by importing the extracted helper surface.
• Added focused tests for metadata header replacement, decision-report ordering, artifact list exposure, and non-Kapi-authored no-op behavior.

Why this closes the prior feedback:

• The latest kapi-agent review found no code-level blockers and only requested a current-head revision explanation.
• This comment is posted after the failing review and names the exact current head SHA so the revision gate can evaluate the latest state.

Verification:

• npx tsx --test test/authored-artifact-metadata.test.ts test/artifact-overwrite.test.ts — pass
• npm run check — pass
• npm run check:unused — pass
• git diff --cached --check — pass before commit

[kapi-agent]

kapi-agent review

Verdict: APPROVE

Final approval summary

Review journey

The original goal was to extract authored artifact metadata synchronization helpers out of KapiService without changing runtime behavior. The semantic scope is narrow: move metadata header generation/sync and decision-report ordering into a dedicated helper, keep KapiService orchestration intact, and add focused tests. A prior kapi-agent approval became stale after a new head, then kapi-agent requested a revision explanation. The author supplied current-head revision explanations covering what changed, why, and verification.

What changed

• Added src/application/authored-artifact-metadata.ts.
• Updated src/application/kapi-service.ts to import AUTHORED_METADATA_ARTIFACTS and syncAuthoredArtifactMetadata.
• Added test/authored-artifact-metadata.test.ts covering artifact list exposure, context/interview metadata replacement, decision-report stale header replacement plus ordering, and no-op behavior for non-Kapi-authored content.

Why this is correct

The helper code is a direct extraction of the existing inline logic. KapiService still uses the same authored artifact names and synchronization path, so storage layout, artifact names, and terminal workflow behavior remain unchanged. The new tests cover the previously under-specified interview and stale decision-report cases.

Evidence

• Verifier gate: PASS.
• Size gate: PASS, 109 changed lines under the 200-line limit.
• Revision explanation status: found and no longer blocking.
• Ilchul review harness gate: PASS, neutral/comment with zero blocking findings.
• Verified command evidence: npm ci && npm run verify exited 0.
• Inspected changed files: src/application/authored-artifact-metadata.ts, src/application/kapi-service.ts, test/authored-artifact-metadata.test.ts.

Remaining risks and approval rationale

Remaining risk is low and limited to the existing format-sensitive metadata header regex. That risk is not introduced by this PR; it is preserved behavior and now has focused test coverage. Approval is justified because all gates pass, the PR is small, the prior revision-explanation blocker is closed, and no blocking correctness, security, or workflow-contract issue remains.

Blocking issues

• None.

Warnings / risks

• src/application/authored-artifact-metadata.ts: the metadata matcher remains intentionally format-sensitive. Future header format changes should update this helper and its tests together.

Suggestions

• None blocking. If this helper grows later, consider keeping its API limited to artifact metadata synchronization rather than broader artifact formatting responsibilities.

Looks good

• The extraction reduces KapiService helper clutter while preserving orchestration behavior.
• Tests directly cover the authored artifact list and sync behavior for all three managed files.
• The decision-report ordering test covers the important case where evidence decision logs must not precede the primary # Decision Report heading.

Verification notes

• Verifier gate status: PASS — npm ci && npm run verify exited 0.
• Size gate status: PASS — 109 changed lines, below the 200-line gate.
• Revision-explanation status: found; prior blocker is resolved.
• Ilchul harness: PASS with no blocking findings.
• Local inspection found no additional blocking issues in the changed files.

---

Engine: pi

[kapi-agent]

kapi-agent review

Verdict: APPROVE

Final approval summary

Review journey

The original goal was to extract authored artifact metadata synchronization out of KapiService into a small helper without changing runtime behavior. The semantic scope is narrow: preserve metadata header replacement for context.md, interview.md, and decision-report.md, plus decision-report heading ordering. A prior approval became stale after later revisions; kapi-agent then requested a current-head revision explanation. The PR body now includes that explanation.

What changed

• Added src/application/authored-artifact-metadata.ts.
• Updated src/application/kapi-service.ts to import AUTHORED_METADATA_ARTIFACTS and syncAuthoredArtifactMetadata.
• Added test/authored-artifact-metadata.test.ts covering artifact list exposure, context/interview metadata replacement, decision-report header replacement and ordering, and no-op behavior for non-Kapi-authored content.

Why this is correct

The extracted helper preserves the previous inline logic and keeps KapiService focused on orchestration. The terminal artifact sync contract remains unchanged: only recognized Kapi metadata headers are rewritten, and decision evidence logs are normalized to follow the primary # Decision Report heading.

Evidence

• Verifier gate: PASS — npm ci && npm run verify exited 0.
• Size gate: PASS — 109 changed lines, below the 200-line limit.
• Revision explanation status: PASS — required now false, found true.
• Ilchul harness gate: PASS — neutral/comment result with 0 blocking findings.
• Inspected files: src/application/authored-artifact-metadata.ts, src/application/kapi-service.ts, and test/authored-artifact-metadata.test.ts.

Remaining risks and approval rationale

No blocking risks remain. The main residual risk is format sensitivity in the metadata header regex, but that is pre-existing behavior and now has focused tests. Approval is justified because the change is small, behavior-preserving, tested, and all configured gates pass.

Blocking issues

• None.

Warnings / risks

• src/application/authored-artifact-metadata.ts: the metadata matcher remains intentionally strict about the generated header shape. Future header format changes should update this helper and tests together.

Suggestions

• Consider adding a short inline comment near headerPattern if future maintainers are likely to wonder why the matcher only recognizes Kapi-generated authored artifact headers.

Looks good

• The extraction boundary is clean and keeps runtime behavior centralized in an application helper.
• KapiService no longer owns low-level metadata formatting logic.
• Tests cover the previously risky cases: interview.md sync and stale decision-report.md header replacement with ordering normalization.

Verification notes

• Verifier gate status: PASS — npm ci && npm run verify exited 0.
• Size gate status: PASS — 109 changed lines < 200.
• Revision-explanation status: PASS — explanation present in the PR body.
• Ilchul review harness: PASS — no blocking findings.

---

Engine: pi

[devkade]

@kapi-agent review

Revision explanation for current head 7170838:

What changed:

• Changed .github/workflows/kapi-agent-formal-approval-gate.yml so the formal approval gate no longer runs on early pull_request events.
• The gate now runs after kapi-agent/review completes via check_run and still re-evaluates on pull_request_review submitted/edited/dismissed events.
• Added PR-number extraction for both event payload shapes and a short poll loop before failing when kapi-agent/review is missing or pending.

Why this closes the blocker:

• PR refactor: extract authored artifact metadata helpers #150 was blocked by stale failed require formal kapi-agent approval runs created before kapi-agent/review existed on the current head.
• Moving the primary trigger to check_run makes the formal gate a final verdict after review/check evidence exists, rather than a race-prone early failure.
• Keeping pull_request_review preserves re-checks when formal reviews are submitted, edited, or dismissed.

Verification rerun:

• YAML parse passed with Ruby YAML.load_file.
• Extracted GitHub Script passed node --check when wrapped as an async function.
• git diff --check passed.
• npm run verify passed.

[kapi-agent]

kapi-agent review

Verdict: APPROVE

Final approval summary

Review journey

The original goal was to extract authored artifact metadata synchronization out of KapiService into a focused helper module. The semantic scope is narrow: preserve existing metadata header synchronization for context.md, interview.md, and decision-report.md, while keeping decision evidence logs ordered after the primary decision-report heading. Prior review feedback only required a current-head revision explanation after stale approvals; that explanation is now present.

What changed

• Added src/application/authored-artifact-metadata.ts.
• Updated src/application/kapi-service.ts to import AUTHORED_METADATA_ARTIFACTS and syncAuthoredArtifactMetadata.
• Added test/authored-artifact-metadata.test.ts covering artifact list exposure, context/interview metadata replacement, decision-report header replacement plus ordering, and no-op behavior for non-Kapi-authored artifacts.

Why this is correct

The extracted helper preserves the removed inline behavior from KapiService: same artifact names, same metadata header format, same generated-header matching, and same decision-report ordering normalization. KapiService remains responsible for orchestration while delegating the formatting/sync policy to the new helper.

Evidence

• Verifier gate: PASS — npm ci && npm run verify exited 0.
• Size gate: PASS — 109 changed lines, under the 200-line limit.
• Ilchul harness gate: PASS — neutral/comment result with 0 blocking findings.
• Revision explanation status: found and not currently required as a blocker.
• Inspected files: src/application/authored-artifact-metadata.ts, src/application/kapi-service.ts, test/authored-artifact-metadata.test.ts.

Remaining risks and approval rationale

Remaining risk is low and limited to the intentionally format-sensitive metadata-header regex. That risk already existed in the inline implementation and is now covered by focused helper tests. Approval is justified because all gates pass, the PR is small, the prior revision-explanation issue is resolved, and no blocking correctness, safety, or workflow-contract issues remain.

Blocking issues

• None.

Warnings / risks

• src/application/authored-artifact-metadata.ts: the metadata-header matcher remains format-sensitive. Future changes to generated header shape must update this helper and its tests together.

Suggestions

• None blocking. Keeping this helper small and avoiding broader KapiService extraction in this PR was the right reviewable boundary.

Looks good

• The extraction is behavior-preserving and keeps orchestration separate from authored artifact metadata formatting.
• Tests cover the important contract points, including interview.md and stale decision-report.md header replacement.
• The PR avoids unrelated mode or CLI churn.

Verification notes

• Verifier gate: PASS — npm ci && npm run verify exited 0.
• Size gate: PASS — 109 changed lines < 200.
• Revision-explanation status: found; current policy says not required now.
• Ilchul review harness: PASS with 0 blocking findings.
• Local inspection confirmed only the helper extraction, import replacement, and focused tests are in scope.

---

Engine: pi

---

kapi-agent request lifecycle:

• Status: COMPLETED
• Head: 2c8372b2da7a
• Formal review event: APPROVE
• Review API status: 200
• Required check API status: 201

[kapi-agent]

kapi-agent review

Verdict: APPROVE

Final approval summary

Review journey

The original goal was to extract authored artifact metadata synchronization out of KapiService into a focused helper module. The semantic scope is narrow: preserve existing metadata header synchronization for context.md, interview.md, and decision-report.md, while keeping decision evidence logs ordered after the primary decision-report heading. Prior review feedback only required a current-head revision explanation after stale approvals; that explanation is now present.

What changed

• Added src/application/authored-artifact-metadata.ts.
• Updated src/application/kapi-service.ts to import AUTHORED_METADATA_ARTIFACTS and syncAuthoredArtifactMetadata.
• Added test/authored-artifact-metadata.test.ts covering artifact list exposure, context/interview metadata replacement, decision-report header replacement plus ordering, and no-op behavior for non-Kapi-authored artifacts.

Why this is correct

The extracted helper preserves the removed inline behavior from KapiService: same artifact names, same metadata header format, same generated-header matching, and same decision-report ordering normalization. KapiService remains responsible for orchestration while delegating the formatting/sync policy to the new helper.

Evidence

• Verifier gate: PASS — npm ci && npm run verify exited 0.
• Size gate: PASS — 109 changed lines, under the 200-line limit.
• Ilchul harness gate: PASS — neutral/comment result with 0 blocking findings.
• Revision explanation status: found and not currently required as a blocker.
• Inspected files: src/application/authored-artifact-metadata.ts, src/application/kapi-service.ts, test/authored-artifact-metadata.test.ts.

Remaining risks and approval rationale

Remaining risk is low and limited to the intentionally format-sensitive metadata-header regex. That risk already existed in the inline implementation and is now covered by focused helper tests. Approval is justified because all gates pass, the PR is small, the prior revision-explanation issue is resolved, and no blocking correctness, safety, or workflow-contract issues remain.

Blocking issues

• None.

Warnings / risks

• src/application/authored-artifact-metadata.ts: the metadata-header matcher remains format-sensitive. Future changes to generated header shape must update this helper and its tests together.

Suggestions

• None blocking. Keeping this helper small and avoiding broader KapiService extraction in this PR was the right reviewable boundary.

Looks good

• The extraction is behavior-preserving and keeps orchestration separate from authored artifact metadata formatting.
• Tests cover the important contract points, including interview.md and stale decision-report.md header replacement.
• The PR avoids unrelated mode or CLI churn.

Verification notes

• Verifier gate: PASS — npm ci && npm run verify exited 0.
• Size gate: PASS — 109 changed lines < 200.
• Revision-explanation status: found; current policy says not required now.
• Ilchul review harness: PASS with 0 blocking findings.
• Local inspection confirmed only the helper extraction, import replacement, and focused tests are in scope.

---

Engine: pi