Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/e2e-advisor.yaml:
- Around line 76-95: The workflow currently hard-codes fetching and diffing
against target/main; add and use the PR’s actual base branch (e.g.
inputs.target_base) instead: when inputs.target_repo and inputs.target_pr are
provided, replace the git fetch of "target main" with fetching "target
${inputs.target_base}" (change the git -C "$TARGET_DIR" fetch --no-tags target
main line) and change the BASE_REF branch selection to use
"target/${inputs.target_base}" instead of "target/main" (update the BASE_REF env
expression in the Run deterministic E2E advisor baseline step); keep HEAD_REF
logic the same.
- Around line 132-136: The fallback printf contains stray shell tokens "},{"
after the redirect which breaks the step when the summary is missing; edit the
conditional fallback (the printf call inside the e2e-advisor summary step) to
remove the extraneous "},{" so the line becomes a plain printf redirecting to
"$GITHUB_STEP_SUMMARY" and ensure the surrounding if/else/fi block remains
properly balanced.

In `@scripts/check-e2e-advisor-config.ts`:
- Around line 20-33: The manifest validation currently only ensures a workflow
field exists and doesn't detect duplicate test ids or verify the workflow
actually exists; update the loop that iterates manifest.tests (and the testIds
Set logic) to (1) detect duplicate test IDs by checking if an id is already in
testIds and push a failure like "duplicate test id X" when found, (2) verify the
referenced workflow id exists in manifest.workflows (e.g., check
manifest.workflows.some(w => w.id === test.workflow)) and push a failure if
missing, and (3) check that the workflow's mapped file exists on disk (use the
same fs.existsSync(path.join(root, <workflow>.file)) pattern) and push an
appropriate failure; keep using requireField for presence checks and append
failures to the existing failures array.

In `@tools/e2e-advisor/advisor.mjs`:
- Around line 131-145: The getChangedFiles function currently swallows failures
from execFileSync and returns [] (making callers think "no changes"); change
this to fail closed and surface degraded-state by throwing a descriptive Error
or returning a distinct sentinel (e.g., null) when both diff variants fail so
callers can emit a degraded-analysis result instead of "no changed files"; also
increase execFileSync's maxBuffer in the options used in getChangedFiles (where
execFileSync is called and the candidates array is iterated) to a larger value
(e.g., { encoding: "utf8", maxBuffer: 10 * 1024 * 1024 }) to avoid silent
truncation on large diffs.

In `@tools/e2e-advisor/comment.mjs`:
- Around line 36-48: When calling the GitHub API to create or patch the PR
comment (the github(...) calls inside the block that checks existing from
findExistingComment), wrap those calls in try/catch and detect permission-denied
errors (HTTP 403 with message like "Resource not accessible by integration" or
similar). If such an error is caught, log a clear info/debug message (e.g.,
"Skipping E2E advisor comment due to permission error") and return/exit the
function without throwing so the step is treated as best-effort; rethrow or
propagate any other unexpected errors. Ensure both the PATCH path (when
existing) and the POST path (when creating) use the same try/catch behavior.

In `@tools/e2e-advisor/pi-analyze.mjs`:
- Around line 236-258: The normalizePiResult function currently copies nested
arrays/objects from the model result into fields like classifiedDomains,
requiredTests, optionalTests, newE2eRecommendations, and dispatchHint without
validating their inner structure; update normalizePiResult to validate and
sanitize each of those fields (e.g., ensure classifiedDomains is an array of
objects with expected keys, requiredTests/optionalTests are arrays of
well-formed test descriptors, newE2eRecommendations are arrays of
strings/objects as expected, and dispatchHint is an object with allowed
properties) and if any field fails validation fallback to
baseline.<function>normalizePiResult</function> Apply the same validation
pattern used for confidence and top-level presence checks: check types, check
element shapes, filter out invalid entries, and only assign
normalized.dispatchHint/result fields when they meet the schema; otherwise
assign baseline.<function>normalized</function> fields should never be assigned
raw model payloads unless they pass these validations.

In `@tools/e2e-advisor/README.md`:
- Line 1: The file begins with the H1 "# E2E Advisor" but is missing the
repo-required SPDX license header; add the SPDX header line (e.g.,
"SPDX-License-Identifier: <LICENSE-ID>") as the very first line of the file,
above the "# E2E Advisor" heading so the SPDX identifier appears at the top of
the file per the coding guidelines.

In `@tools/e2e-advisor/schema.json`:
- Around line 67-74: The dispatchHint schema currently allows an empty object
and must require both properties; update the "dispatchHint" object schema so
that "workflow" and "jobsInput" are listed under "required" (ensuring they are
present when dispatchHint exists) while keeping "additionalProperties": false;
target the "dispatchHint" schema and add "required": ["workflow","jobsInput"] so
summary renderers can safely access dispatchHint.workflow and
dispatchHint.jobsInput.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Duplicate comments:
In `@scripts/check-e2e-advisor-config.ts`:
- Around line 20-33: The manifest validation silently ignores duplicate test IDs
and doesn't validate that each test.workflow points to a declared workflow file;
update the loop that iterates manifest.tests to detect duplicates by tracking
seen IDs (instead of only using testIds Set created from map) and push a failure
when an ID is repeated (referencing testIds and manifest.tests), and also
validate that test.workflow maps to an existing workflow entry/file by checking
the workflows list and/or fs.existsSync(path.join(root, workflowPath)) and
pushing to failures when missing (referencing requireField, failures, root, fs,
and path).

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Duplicate comments:
In `@tools/e2e-advisor/comment.mjs`:
- Around line 36-57: The call to findExistingComment(repo, pr, token, marker)
can throw a permission error before the surrounding try/catch runs; wrap that
lookup in the same try block (or add a small try/catch around
findExistingComment) and on permission errors (using isPermissionError) skip
creating/updating the comment the same way you do for the github POST/PATCH
calls—keep the existing variable name existing and then proceed to the existing
logic for PATCH/POST only if lookup succeeded, otherwise log and return.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/e2e-advisor.yaml:
- Around line 99-100: The workflow step named "Install Pi" currently installs
`@mariozechner/pi-coding-agent` without a version; change the npm install command
in that step to pin the package to a specific known-good semver (e.g., append
@<version>) so the action installs a fixed release instead of the latest, and
add/update repository Dependabot/Renovate configuration to track and safely
propose upgrades for `@mariozechner/pi-coding-agent`.

---

Nitpick comments:
In `@tools/e2e-advisor/pi-analyze.mjs`:
- Around line 226-246: normalizePiResult currently assumes `result` is an object
and will throw if `result` is null or not an object (e.g., JSON.parse("null")).
Add an early guard in normalizePiResult that checks whether `result` is a
non-null object (typeof result === "object" && result !== null); if not, return
a normalized object with the same top-level fields (version, baseRef/headRef
from metadata, changedFiles, and sensible defaults or empty values for
classifiedDomains, requiredTests, optionalTests, newE2eRecommendations,
noE2eReason, confidence) or otherwise set those fields to null/empty via the
existing sanitize* helpers, then continue with the existing dispatchHint
handling. Ensure you reference the sanitize functions (sanitizeDomains,
sanitizeTests, sanitizeNewRecommendations, sanitizeDispatchHint) and the
metadata fields (baseRef, headRef, changedFiles) when building the fallback
normalized result.