morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 07:47:29 +00:00

[PR #5261] [CLOSED] fix(javascript): preserve caller-selected input paths in Evidence #6844

New issue

Closed

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

bsakaguchi commented

2026-11-01 21:43:27 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/5261
Author: @kwakhyun
Created: 10/19/2026
Status: ❌ Closed

Base: main ← Head: codex/fix-js-input-path-evidence

📝 Commits (1)

3792ea5 fix(javascript): preserve caller-selected input paths in Evidence

📊 Changes

18 files changed (+246 additions, -15 deletions)

View changed files

📝 docs/javascript-artifact-reconstruction.md (+7 -0)
➕ scripts/lib/javascript-application-paths.d.mts (+8 -0)
➕ scripts/lib/javascript-application-paths.mjs (+25 -0)
📝 scripts/verify-claude-client.mjs (+2 -1)
📝 scripts/verify-commandcode-client.mjs (+2 -1)
📝 scripts/verify-copilot-client.mjs (+2 -1)
📝 scripts/verify-deepseek-client.mjs (+2 -1)
📝 scripts/verify-gemini-client.mjs (+2 -1)
📝 scripts/verify-grok-client.mjs (+5 -4)
📝 scripts/verify-native-agent-client.mjs (+2 -1)
📝 scripts/verify-omp-client.mjs (+2 -1)
📝 scripts/verify-opencode-client.mjs (+2 -1)
📝 scripts/verify-qwen-client.mjs (+2 -1)
📝 src/application/javascript/JavaScriptApplicationEvidence.ts (+1 -0)
📝 src/cli/coreBinaryCommands.ts (+1 -2)
📝 tests/acceptance/applications/javascriptAnalysisLimits.test.ts (+3 -0)
➕ tests/acceptance/applications/javascriptInputPathEvidence.test.ts (+123 -0)
➕ tests/boundary/filesystem/javascriptClientVerifier.test.ts (+55 -0)

📄 Description

Summary

Fixes #5258.

JavaScript application analysis succeeds through a directory symlink but previously discarded the caller-selected path from Evidence. Preserve that path in parameters.input_path while keeping subject.local_path and normalized_result.input_path canonical.

Problem and expected behavior

Executed against base 255d906e15a141b0f4acee60dc73976f03adc94c with a synthetic JavaScript directory and a symlink. Both public CLI routes (analyze and analyze-javascript-application) and the MCP tool completed analysis, but all three regression cases failed because parameters.input_path was undefined.

For CLI input ./selected app, the resulting fields now have this shape (temporary absolute prefix abbreviated):

{
  "subject": { "local_path": "/fixture/physical" },
  "parameters": { "input_path": "./selected app", "format": "auto", "integrity_policy": "fail" },
  "normalized_result": { "input_path": "/fixture/physical" }
}

An absolute MCP alias is likewise preserved exactly. Selecting an alias or its canonical path produces identical artifact subjects and analysis results; Evidence IDs differ because the recorded inputs differ.

Change and scope

Record the original input at the JavaScript Evidence producer.
Pass the original CLI argument through the generic JavaScript route, matching the dedicated route.
Replace lexical subject-path equality in 10 client verifiers with one helper that checks caller provenance and canonical identity independently.
Add public CLI/MCP and verifier-boundary regressions; update existing parameter expectations and the owning documentation.
Keep historical parsing unchanged: older Evidence can omit the new parameter, and retained/exported records remain readable after both the symlink and physical artifact are removed.

This is a routine provenance fix within existing static-analysis behavior. A separate maintainer scope decision is not applicable.

Contract and boundary impact

Semantic owner and earliest changed stage: preserve the CLI argument before analysis; serialize caller provenance in JavaScriptApplicationEvidence.parameters.
Contribution scope: existing local static reconstruction; no new provider or execution capability.
CLI and MCP/tool catalog: additive Evidence parameter; unchanged tool inputs and catalog schemas. Generic CLI relative-path spelling is now retained.
Provider/platform compatibility: canonical resolution stays with artifact reconstruction. Verifiers compare each path according to its meaning.
Evidence/artifact contract: unchanged artifact identity and analysis result; caller-path provenance contributes to the Evidence commitment. Legacy records retain the existing normalized-path fallback.
Target selection/authority/cleanup: existing file analysis only; no added target execution, mutation, network behavior, or historical filesystem lookup.
Generated metadata/package/installation: no version or installation changes; generated outputs are not committed.

Expected effects:

Read existing files or captured data.
Launch or attach to a process.
Inject instrumentation or run scripts/code.
Change process memory, registers, files, or configuration.
Contact network services.
Recover protected code or work around protections.
Documentation or usability only; no target interaction.
Not sure yet; investigation or maintainer guidance needed.

Evidence and regression coverage

Executed base reproduction: 3/3 new CLI/MCP cases failed specifically on the missing input-path parameter; analysis itself succeeded and canonical identity assertions passed.
Executed fix verification: 76/76 focused tests passed across 5 files, including both CLI routes, absolute MCP aliases, unchanged artifact identity, retention/export, legacy Evidence, actual timeout partial results, JSON/JSONL output, and source-to-bundle paths.
Strengthened historical coverage: reran the 3 public regressions after requiring deletion of both the alias and physical artifact; 3/3 passed.
Verifier-boundary coverage uses real static analysis and confirms that missing caller provenance or alias-valued canonical identity is rejected.
Source review found no additional required historical-parser changes.

For evidence-bearing changes:

Observed, derived, and inferred claims remain distinguishable.
Artifact identity, source provenance, and failed attempts remain preserved.
Unsupported, incomplete, unavailable, or uncertain outcomes remain visible.

Validation performed

Executed on macOS arm64 with Node 24.18.0 and npm 11.16.0:

npm run build:cached — passed.
npm run test:focused -- tests/acceptance/applications/javascriptInputPathEvidence.test.ts tests/acceptance/applications/javascriptAnalysisLimits.test.ts tests/boundary/filesystem/javascriptClientVerifier.test.ts tests/boundary/cli/javascriptApplicationJsonOutput.test.ts tests/boundary/filesystem/sourceToBundlePaths.test.ts — 5 files, 76 tests passed.
npm run test:focused -- tests/acceptance/applications/javascriptInputPathEvidence.test.ts — final strengthened historical-record coverage: 3 tests passed.
npm run check — passed on the final changes, including typecheck, lint, formatting, metadata, dependency, module-boundary, and architecture checks. Lint reported 0 errors and 1,756 repository warnings.
npm run docs:check — passed, including 5 README consistency tests.
npm run verify:test-discovery — passed; all 1,658 test files assigned exactly once.
git diff --check — passed.
Commit hooks (format/lint) and the push hook (npm run check:fast) — passed; commit hooks did not change the validated content.

Compatibility, safety, and release

No breaking schema migration; older Evidence without parameters.input_path remains readable. Newly produced Evidence IDs change with the newly retained provenance.
Unverified: Windows/Linux, actual external-client verifier runs (including Gemini), live model services, real Hopper/Ghidra/browser workflows, and the full deterministic suite. The helper regression does not establish end-to-end client compatibility.
No package version or release metadata changes. Synthetic local fixtures only; no target code was executed by analysis.

Author readiness

Human owner: @kwakhyun

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.

Before merge

This PR addresses a concrete problem or an agreed enhancement.
Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff.
The PR has one focused outcome and the title follows type(scope): outcome.
Related issue is linked, or the reason for not linking one is stated above.
Tests cover changed observable behavior and meaningful failure paths.
Owning docs, contracts, and generated metadata are updated where needed.
User-visible CLI/MCP changes include representative output.
I checked the final diff for secrets, unrelated cleanup, and unsupported claims.

_{🔄 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/5261 **Author:** [@kwakhyun](https://github.com/kwakhyun) **Created:** 10/19/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `codex/fix-js-input-path-evidence` --- ### 📝 Commits (1) - [`3792ea5`](https://github.com/morluto/rea/commit/3792ea58dba21a3cc1375a52bd977e7d0ba78c9d) fix(javascript): preserve caller-selected input paths in Evidence ### 📊 Changes **18 files changed** (+246 additions, -15 deletions) <details> <summary>View changed files</summary> 📝 `docs/javascript-artifact-reconstruction.md` (+7 -0) ➕ `scripts/lib/javascript-application-paths.d.mts` (+8 -0) ➕ `scripts/lib/javascript-application-paths.mjs` (+25 -0) 📝 `scripts/verify-claude-client.mjs` (+2 -1) 📝 `scripts/verify-commandcode-client.mjs` (+2 -1) 📝 `scripts/verify-copilot-client.mjs` (+2 -1) 📝 `scripts/verify-deepseek-client.mjs` (+2 -1) 📝 `scripts/verify-gemini-client.mjs` (+2 -1) 📝 `scripts/verify-grok-client.mjs` (+5 -4) 📝 `scripts/verify-native-agent-client.mjs` (+2 -1) 📝 `scripts/verify-omp-client.mjs` (+2 -1) 📝 `scripts/verify-opencode-client.mjs` (+2 -1) 📝 `scripts/verify-qwen-client.mjs` (+2 -1) 📝 `src/application/javascript/JavaScriptApplicationEvidence.ts` (+1 -0) 📝 `src/cli/coreBinaryCommands.ts` (+1 -2) 📝 `tests/acceptance/applications/javascriptAnalysisLimits.test.ts` (+3 -0) ➕ `tests/acceptance/applications/javascriptInputPathEvidence.test.ts` (+123 -0) ➕ `tests/boundary/filesystem/javascriptClientVerifier.test.ts` (+55 -0) </details> ### 📄 Description ## Summary Fixes #5258. JavaScript application analysis succeeds through a directory symlink but previously discarded the caller-selected path from Evidence. Preserve that path in `parameters.input_path` while keeping `subject.local_path` and `normalized_result.input_path` canonical. ## Problem and expected behavior Executed against base `255d906e15a141b0f4acee60dc73976f03adc94c` with a synthetic JavaScript directory and a symlink. Both public CLI routes (`analyze` and `analyze-javascript-application`) and the MCP tool completed analysis, but all three regression cases failed because `parameters.input_path` was `undefined`. For CLI input `./selected app`, the resulting fields now have this shape (temporary absolute prefix abbreviated): ```json { "subject": { "local_path": "/fixture/physical" }, "parameters": { "input_path": "./selected app", "format": "auto", "integrity_policy": "fail" }, "normalized_result": { "input_path": "/fixture/physical" } } ``` An absolute MCP alias is likewise preserved exactly. Selecting an alias or its canonical path produces identical artifact subjects and analysis results; Evidence IDs differ because the recorded inputs differ. ## Change and scope - Record the original input at the JavaScript Evidence producer. - Pass the original CLI argument through the generic JavaScript route, matching the dedicated route. - Replace lexical subject-path equality in 10 client verifiers with one helper that checks caller provenance and canonical identity independently. - Add public CLI/MCP and verifier-boundary regressions; update existing parameter expectations and the owning documentation. - Keep historical parsing unchanged: older Evidence can omit the new parameter, and retained/exported records remain readable after both the symlink and physical artifact are removed. This is a routine provenance fix within existing static-analysis behavior. A separate maintainer scope decision is not applicable. ## Contract and boundary impact - Semantic owner and earliest changed stage: preserve the CLI argument before analysis; serialize caller provenance in `JavaScriptApplicationEvidence.parameters`. - Contribution scope: existing local static reconstruction; no new provider or execution capability. - CLI and MCP/tool catalog: additive Evidence parameter; unchanged tool inputs and catalog schemas. Generic CLI relative-path spelling is now retained. - Provider/platform compatibility: canonical resolution stays with artifact reconstruction. Verifiers compare each path according to its meaning. - Evidence/artifact contract: unchanged artifact identity and analysis result; caller-path provenance contributes to the Evidence commitment. Legacy records retain the existing normalized-path fallback. - Target selection/authority/cleanup: existing file analysis only; no added target execution, mutation, network behavior, or historical filesystem lookup. - Generated metadata/package/installation: no version or installation changes; generated outputs are not committed. Expected effects: - [x] Read existing files or captured data. - [ ] Launch or attach to a process. - [ ] Inject instrumentation or run scripts/code. - [ ] Change process memory, registers, files, or configuration. - [ ] Contact network services. - [ ] Recover protected code or work around protections. - [ ] Documentation or usability only; no target interaction. - [ ] Not sure yet; investigation or maintainer guidance needed. ## Evidence and regression coverage - Executed base reproduction: 3/3 new CLI/MCP cases failed specifically on the missing input-path parameter; analysis itself succeeded and canonical identity assertions passed. - Executed fix verification: 76/76 focused tests passed across 5 files, including both CLI routes, absolute MCP aliases, unchanged artifact identity, retention/export, legacy Evidence, actual timeout partial results, JSON/JSONL output, and source-to-bundle paths. - Strengthened historical coverage: reran the 3 public regressions after requiring deletion of both the alias and physical artifact; 3/3 passed. - Verifier-boundary coverage uses real static analysis and confirms that missing caller provenance or alias-valued canonical identity is rejected. - Source review found no additional required historical-parser changes. For evidence-bearing changes: - [x] Observed, derived, and inferred claims remain distinguishable. - [x] Artifact identity, source provenance, and failed attempts remain preserved. - [x] Unsupported, incomplete, unavailable, or uncertain outcomes remain visible. ## Validation performed Executed on macOS arm64 with Node 24.18.0 and npm 11.16.0: - `npm run build:cached` — passed. - `npm run test:focused -- tests/acceptance/applications/javascriptInputPathEvidence.test.ts tests/acceptance/applications/javascriptAnalysisLimits.test.ts tests/boundary/filesystem/javascriptClientVerifier.test.ts tests/boundary/cli/javascriptApplicationJsonOutput.test.ts tests/boundary/filesystem/sourceToBundlePaths.test.ts` — 5 files, 76 tests passed. - `npm run test:focused -- tests/acceptance/applications/javascriptInputPathEvidence.test.ts` — final strengthened historical-record coverage: 3 tests passed. - `npm run check` — passed on the final changes, including typecheck, lint, formatting, metadata, dependency, module-boundary, and architecture checks. Lint reported 0 errors and 1,756 repository warnings. - `npm run docs:check` — passed, including 5 README consistency tests. - `npm run verify:test-discovery` — passed; all 1,658 test files assigned exactly once. - `git diff --check` — passed. - Commit hooks (format/lint) and the push hook (`npm run check:fast`) — passed; commit hooks did not change the validated content. ## Compatibility, safety, and release - No breaking schema migration; older Evidence without `parameters.input_path` remains readable. Newly produced Evidence IDs change with the newly retained provenance. - **Unverified:** Windows/Linux, actual external-client verifier runs (including Gemini), live model services, real Hopper/Ghidra/browser workflows, and the full deterministic suite. The helper regression does not establish end-to-end client compatibility. - No package version or release metadata changes. Synthetic local fixtures only; no target code was executed by analysis. ## Author readiness Human owner: @kwakhyun - [x] 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. - [x] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [x] I declare this PR ready for initial maintainer review. ## Before merge - [x] This PR addresses a concrete problem or an agreed enhancement. - [x] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [x] The PR has one focused outcome and the title follows `type(scope): outcome`. - [x] Related issue is linked, or the reason for not linking one is stated above. - [x] Tests cover changed observable behavior and meaningful failure paths. - [x] Owning docs, contracts, and generated metadata are updated where needed. - [x] User-visible CLI/MCP changes include representative output. - [x] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-11-01 21:43:27 +00:00

closed this issue
added the
pull-request
label

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#6844

No description provided.

Rows
Columns