morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 06:45:22 +00:00

[PR #6182] [MERGED] refactor: consolidate remaining legacy contract ownership #7190

New issue

Closed

opened 2026-11-01 21:46:01 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-11-01 21:46:01 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/6182
Author: @morluto
Created: 10/21/2026
Status: ✅ Merged
Merged: 10/21/2026
Merged by: @morluto

Base: main ← Head: codex/legacy-cleanup-followup-20261022

📝 Commits (3)

1828cf8 refactor: remove redundant contract and syntax definitions
b2dc450 refactor(cli): share registered command traversal
603a7d0 refactor(process): share capture evidence admission

📊 Changes

23 files changed (+281 additions, -186 deletions)

View changed files

📝 scripts/ci/artifacts.d.mts (+1 -1)
📝 src/application/EvidenceInputResolver.ts (+1 -1)
📝 src/application/environment/environmentReplay.test.ts (+4 -4)
📝 src/application/environment/environmentReplay.ts (+0 -2)
📝 src/application/environment/environmentStateAnswer.test.ts (+4 -2)
📝 src/application/environment/environmentStateAnswer.ts (+1 -1)
📝 src/application/process/ProcessCli.ts (+36 -47)
📝 src/application/process/ProcessEvidence.ts (+73 -5)
📝 src/cli/cliResultOutput.ts (+9 -27)
📝 src/cli/omittableDefaults.ts (+3 -17)
➕ src/cli/visitRegisteredCommands.ts (+35 -0)
📝 src/contracts/advertisedInputExamples.test.ts (+1 -1)
➖ src/contracts/evidenceInputContracts.ts (+0 -5)
📝 src/contracts/javascript/applicationWorkflowInputContracts.ts (+1 -1)
📝 src/contracts/native/functionMatchingContracts.ts (+1 -1)
📝 src/contracts/wasm/wasmComparisonContracts.ts (+1 -1)
📝 src/domain/analysisView/analysisView.ts (+4 -2)
📝 src/domain/analysisView/rustBinaryView.ts (+0 -4)
📝 src/domain/webBundleAnalyzerAst.ts (+2 -1)
📝 src/process/ProcessOwnership.ts (+0 -2)

...and 3 more files

📄 Description

Summary

Remove five remaining sources of duplicated ownership after #6181: forwarding-only contracts, CLI registry traversal, process-capture admission, URI scheme syntax, and the CI run type projection. No separate issue: these are source-confirmed maintenance findings from the follow-up audit.

Problem and expected behavior

CLI and MCP separately admitted process-capture Evidence, and two CLI initialization passes independently interpreted Incur's registry. These rules should have one owner while retaining adapter-specific diagnostics and initialization order.

Change and scope

Redirect consumers to canonical schema/type/constant owners and remove unused forwarding exports and the empty Evidence input facade.
Share a CLI-owned registry visitor. Keep schema presentation and result wrapping as separate ordered passes; preserve child/root order and alias/fetch handling.
Admit capture Evidence once in the existing application owner, returning validated facts or tagged failures. Keep CLI/MCP error projections, causes, validation precedence, Evidence links and supported historical provider identity.
Reuse hasScheme for the existing module-specifier scheme arm and express the narrow CI run contract as Pick<CiRun, "id" | "head_sha">.

Research used Context7's official Incur documentation, then checked the actual registry against the pinned Incur 0.6.1 source. Current upstream documentation describes a newer API: official Incur README.

Contract and boundary impact

Semantic owner and earliest changed stage: domain contracts; CLI registration traversal; application process Evidence admission.
Contribution scope fit and linked maintainer decision: ordinary refactoring within existing behavior; new-capability scope decision not applicable.
CLI and MCP/tool-catalog contract: existing schemas, outputs and adapter-specific errors preserved.
Provider/platform compatibility: historical process-provider identity remains supported.
Evidence/provenance: original authenticated records and parsed captures retained, including links and locations.
Authority and cleanup: existing target selection, process execution and resource cleanup unchanged.
Generated metadata/package/install impact: none; no dependencies or generated outputs committed.

Existing process comparison reads captured Evidence; existing capture regression tests launch local Node processes. The refactoring adds no target effect.

Evidence and regression coverage

Two boundary tests add mounted-group root coverage for omittable defaults and alias-invoked output formatting. Existing public CLI/MCP tests exercise current/legacy provider identities, authenticity before provider admission, wrong workflow, malformed captures, source retention and comparison parity. Independent source review found no actionable issues.

Validation performed

npm run test:focused -- <16 affected CLI/MCP/process/filesystem/acceptance files> — 222 passed, 2 Linux-only cases skipped on macOS. Includes real local Node captures and WASM CLI/MCP comparison; runtime/skill build succeeded.
npm run test:local -- <9 affected contract/view/environment/service files> — 58 passed.
npm run test:local -- src/domain/webBundleAnalyzerAst.test.ts src/domain/artifactPathSyntax.test.ts src/browser/WebSourceMapFetcher.test.ts — 7 passed.
npm run check:fast — passed typecheck, lint and module-boundary verification.
npm run knip — passed.
oxfmt --check on every changed source/declaration/test file and git diff --check — passed.

The first integrated typecheck caught Incur's overly narrow registry-key typing; the shared visitor now retains the previous typed WeakMap widening. All final checks above ran after that correction.

Compatibility, safety, and release

No intended public behavior change or migration. Internal source import paths removed only after tracing all consumers. No real Hopper/Ghidra/browser integration run; no Windows/Linux host verification. No performance improvement claimed; task token, CPU and memory costs were not benchmarked.

Author readiness

Human owner: @morluto

I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
I declare this PR ready for initial maintainer review.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/6182 **Author:** [@morluto](https://github.com/morluto) **Created:** 10/21/2026 **Status:** ✅ Merged **Merged:** 10/21/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `codex/legacy-cleanup-followup-20261022` --- ### 📝 Commits (3) - [`1828cf8`](https://github.com/morluto/rea/commit/1828cf87a06a0916fc4f2bd54956051a3ac724bd) refactor: remove redundant contract and syntax definitions - [`b2dc450`](https://github.com/morluto/rea/commit/b2dc45006dde47858e245a0c82d0b60b9b1021f2) refactor(cli): share registered command traversal - [`603a7d0`](https://github.com/morluto/rea/commit/603a7d022c012fb7b6bd1de65b44950720f8b40d) refactor(process): share capture evidence admission ### 📊 Changes **23 files changed** (+281 additions, -186 deletions) <details> <summary>View changed files</summary> 📝 `scripts/ci/artifacts.d.mts` (+1 -1) 📝 `src/application/EvidenceInputResolver.ts` (+1 -1) 📝 `src/application/environment/environmentReplay.test.ts` (+4 -4) 📝 `src/application/environment/environmentReplay.ts` (+0 -2) 📝 `src/application/environment/environmentStateAnswer.test.ts` (+4 -2) 📝 `src/application/environment/environmentStateAnswer.ts` (+1 -1) 📝 `src/application/process/ProcessCli.ts` (+36 -47) 📝 `src/application/process/ProcessEvidence.ts` (+73 -5) 📝 `src/cli/cliResultOutput.ts` (+9 -27) 📝 `src/cli/omittableDefaults.ts` (+3 -17) ➕ `src/cli/visitRegisteredCommands.ts` (+35 -0) 📝 `src/contracts/advertisedInputExamples.test.ts` (+1 -1) ➖ `src/contracts/evidenceInputContracts.ts` (+0 -5) 📝 `src/contracts/javascript/applicationWorkflowInputContracts.ts` (+1 -1) 📝 `src/contracts/native/functionMatchingContracts.ts` (+1 -1) 📝 `src/contracts/wasm/wasmComparisonContracts.ts` (+1 -1) 📝 `src/domain/analysisView/analysisView.ts` (+4 -2) 📝 `src/domain/analysisView/rustBinaryView.ts` (+0 -4) 📝 `src/domain/webBundleAnalyzerAst.ts` (+2 -1) 📝 `src/process/ProcessOwnership.ts` (+0 -2) _...and 3 more files_ </details> ### 📄 Description ## Summary Remove five remaining sources of duplicated ownership after #6181: forwarding-only contracts, CLI registry traversal, process-capture admission, URI scheme syntax, and the CI run type projection. No separate issue: these are source-confirmed maintenance findings from the follow-up audit. ## Problem and expected behavior CLI and MCP separately admitted process-capture Evidence, and two CLI initialization passes independently interpreted Incur's registry. These rules should have one owner while retaining adapter-specific diagnostics and initialization order. ## Change and scope - Redirect consumers to canonical schema/type/constant owners and remove unused forwarding exports and the empty Evidence input facade. - Share a CLI-owned registry visitor. Keep schema presentation and result wrapping as separate ordered passes; preserve child/root order and alias/fetch handling. - Admit capture Evidence once in the existing application owner, returning validated facts or tagged failures. Keep CLI/MCP error projections, causes, validation precedence, Evidence links and supported historical provider identity. - Reuse `hasScheme` for the existing module-specifier scheme arm and express the narrow CI run contract as `Pick<CiRun, "id" | "head_sha">`. Research used Context7's official Incur documentation, then checked the actual registry against the pinned Incur 0.6.1 source. Current upstream documentation describes a newer API: [official Incur README](https://github.com/wevm/incur/blob/main/README.md). ## Contract and boundary impact - Semantic owner and earliest changed stage: domain contracts; CLI registration traversal; application process Evidence admission. - Contribution scope fit and linked maintainer decision: ordinary refactoring within existing behavior; new-capability scope decision not applicable. - CLI and MCP/tool-catalog contract: existing schemas, outputs and adapter-specific errors preserved. - Provider/platform compatibility: historical process-provider identity remains supported. - Evidence/provenance: original authenticated records and parsed captures retained, including links and locations. - Authority and cleanup: existing target selection, process execution and resource cleanup unchanged. - Generated metadata/package/install impact: none; no dependencies or generated outputs committed. Existing process comparison reads captured Evidence; existing capture regression tests launch local Node processes. The refactoring adds no target effect. ## Evidence and regression coverage Two boundary tests add mounted-group root coverage for omittable defaults and alias-invoked output formatting. Existing public CLI/MCP tests exercise current/legacy provider identities, authenticity before provider admission, wrong workflow, malformed captures, source retention and comparison parity. Independent source review found no actionable issues. ## Validation performed - `npm run test:focused -- <16 affected CLI/MCP/process/filesystem/acceptance files>` — 222 passed, 2 Linux-only cases skipped on macOS. Includes real local Node captures and WASM CLI/MCP comparison; runtime/skill build succeeded. - `npm run test:local -- <9 affected contract/view/environment/service files>` — 58 passed. - `npm run test:local -- src/domain/webBundleAnalyzerAst.test.ts src/domain/artifactPathSyntax.test.ts src/browser/WebSourceMapFetcher.test.ts` — 7 passed. - `npm run check:fast` — passed typecheck, lint and module-boundary verification. - `npm run knip` — passed. - `oxfmt --check` on every changed source/declaration/test file and `git diff --check` — passed. The first integrated typecheck caught Incur's overly narrow registry-key typing; the shared visitor now retains the previous typed WeakMap widening. All final checks above ran after that correction. ## Compatibility, safety, and release No intended public behavior change or migration. Internal source import paths removed only after tracing all consumers. No real Hopper/Ghidra/browser integration run; no Windows/Linux host verification. No performance improvement claimed; task token, CPU and memory costs were not benchmarked. ## Author readiness Human owner: @morluto - [ ] I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work. - [ ] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [ ] I declare this PR ready for initial maintainer review. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-11-01 21:46:01 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-11-03 13:39:47 +00:00

[PR #7190] [CLOSED] chore(skills): expose the REA skill to Claude Code sessions #7706

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

morluto/rea#7190

No description provided.

Rows
Columns