feat: migrate release intelligence - #18
Conversation
4339cf3 to
4bf7be2
Compare
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR #18 — feat: migrate release intelligence
This PR migrates provider-neutral Release Intelligence contracts and analysis into `@shiplightai/quality-core`, adds reusable UI views to `@shiplightai/quality-ui`, and wires a GitHub Actions run adapter into the Explorer. The code is generally well-structured and the test coverage is solid. However there is one critical determinism violation on the scoring path and several medium-severity issues that should be resolved before merge.
CRITICAL
C1 — `evaluateReleasePolicy` defaults to `new Date()` on the scoring path
File: `packages/core/src/release-intelligence/policy.ts` (diff line ~2698–2700)
export function evaluateReleasePolicy(input: {
readonly now?: Date; // ← optional
...
}): ReleasePolicyDecision {
const now = input.now ?? new Date(); // ← wall-clock fallback
...
.filter((item) => item.revokedAt === null && item.expiresAt.getTime() > now.getTime())When a caller omits `now` and passes a non-empty `exceptions` array, two evaluations of identical inputs at different instants can return different `ALLOW`/`BLOCK` decisions — violating the core determinism invariant. The Explorer calls this function without `now` today (safe only because it passes no `exceptions`), but `quality-core` is a shared library bundled into `quality-tools` and consumed by the Shiplight platform, which does use exceptions.
Fix: Either make `now` required whenever `exceptions` is non-empty (guarded by a runtime check or type overload), or always require it on callers that touch the scoring path.
HIGH
H1 — `.git/worktrees/` metadata leaks on cleanup failure
File: `apps/explorer/src/lib/release-intelligence/action-run.ts` (diff lines ~701–712)
} finally {
if (registeredWorktree) {
await execFileAsync("git", ["-C", projectPath, "worktree", "remove", "--force", worktreePath])
.catch(() => undefined); // ← silently swallowed
}
await rm(temporaryRoot, { recursive: true, force: true });
}When `git worktree remove --force` fails, `rm` still deletes `temporaryRoot` (physical cleanup succeeds), but git's internal bookkeeping in `/.git/worktrees/` is never cleaned. Repeated analysis failures accumulate stale entries. `git worktree list` will show phantom entries and git may refuse new worktrees with the same path if names collide.
Fix: After silently catching the git failure, call `git worktree prune` on `projectPath` (also swallowing failures), or switch to `git worktree add --lock` and a two-step prune on the next startup.
MEDIUM
M1 — `.sort()` without explicit comparator is locale-dependent
File: `packages/core/src/release-intelligence/components.ts` (diff line ~1122)
.transform((components) => [...new Set(components)].sort())The default `.sort()` comparator is locale-sensitive for Unicode strings. Component IDs with non-ASCII characters will sort differently on different platforms/locales, making the canonical component list non-deterministic. Use an explicit comparator: `.sort((a, b) => (a < b ? -1 : a > b ? 1 : 0))`.
M2 — Explorer calls `evaluateReleasePolicy` without `now` (fragile contract)
File: `apps/explorer/src/lib/release-intelligence/action-run.ts` (diff line ~455–460)
const decision = evaluateReleasePolicy({
policy: ...,
assessments,
systemFacts,
// no now, no exceptions — safe only by accident
});The call is harmless today because no `exceptions` are passed. But it encodes a silent assumption: "this function never uses `now` when `exceptions` is absent." That assumption breaks the moment exceptions are wired up. Explicitly pass `now: new Date()` (at the adapter boundary, not inside the engine) to make the invariant visible.
M3 — `evidenceProviderLink` passes any `https://`-prefixed string as an anchor href
File: `packages/ui/src/release-intelligence/IssueEvidenceDrawer.tsx` (diff lines ~5739–5749)
return providerRef.startsWith("https://")
? { href: providerRef, label: "Open evidence" }
: null;A `providerRef` value stored in a `.quality/` evidence file could point to any external domain. While `javascript:` is already excluded by the `startsWith("https://")` guard, a corrupted or adversarially crafted evidence file could link to a phishing or malicious page. Use `new URL(providerRef).protocol === "https:"` and optionally restrict to known hostnames, to harden the link before rendering.
M4 — Stale worktree entries could write into the scanned project's `.git` directory
File: `apps/explorer/src/lib/release-intelligence/action-run.ts` (diff lines ~686–696)
The `git worktree add` call targets ``'s git repo. The Explorer's invariant is that it is strictly read-only against the project it inspects. Adding (and removing) a worktree modifies `/.git/worktrees/`. This is an internal-to-git write inside `QUALITY_PROJECT_ROOT`. It is not a write to the project's tracked files, but it does mutate `QUALITY_PROJECT_ROOT/.git/`. Document this explicitly in the README as the intentional exception, and ensure the Explorer's read-only contract is updated to reflect it.
LOW
L1 — `parseRepoFullName` silently accepts three-segment paths
File: `packages/core/src/release-intelligence/operations.ts` (diff line ~2462)
const [owner, name] = value.split("/");`"owner/name/extra"` is accepted as `{ owner: "owner", name: "name" }` rather than rejected. This is safe because `encodeURIComponent` guards the API call surface, but it is an inconsistency with `assertRepoAndCommit` in `github-links.ts` which validates the segment count.
L2 — Cache key omits `GITHUB_TOKEN`
File: `apps/explorer/src/lib/release-intelligence/action-run.ts` (diff lines ~299–304)
The module-level preview cache keyed on `[projectPath, configuredRun, RELEASE_ENVIRONMENT, RELEASE_COMPONENTS]` does not include `GITHUB_TOKEN`. A token rotation without a server restart would serve the cached result from the old token. Minor in practice (dev-server restart resets the cache), but worth noting.
L3 — No `quality-tools` size-gate update visible in this diff
Context: `packages/quality-tools/package-size.json` and `packages/quality-tools/package.json` are not in this diff.
This PR adds two new entry points to `quality-core` (`release-intelligence` and `release-intelligence/operations`). If `quality-tools` re-exports these, the packed size will increase. Verify that the packed/unpacked delta stays within 1 % of the published `@shiplightai/quality-tools` version, or add a human-approved `approvedIncrease` entry. The build gate will catch this, but it should be verified before the PR is merged.
Summary
| # | Severity | File | Issue |
|---|---|---|---|
| C1 | CRITICAL | `packages/core/src/release-intelligence/policy.ts` ~2700 | `evaluateReleasePolicy` defaults to `new Date()` — non-deterministic ALLOW/BLOCK when exceptions are present |
| H1 | HIGH | `apps/explorer/src/lib/release-intelligence/action-run.ts` ~701–712 | Silent `git worktree remove` failure leaks `.git/worktrees/` metadata |
| M1 | MEDIUM | `packages/core/src/release-intelligence/components.ts` ~1122 | `.sort()` without explicit comparator — locale-dependent for non-ASCII component IDs |
| M2 | MEDIUM | `apps/explorer/src/lib/release-intelligence/action-run.ts` ~455 | `evaluateReleasePolicy` called without `now` — safe today, fragile if exceptions are added |
| M3 | MEDIUM | `packages/ui/src/release-intelligence/IssueEvidenceDrawer.tsx` ~5747 | Unvalidated `https://`-prefixed `providerRef` rendered as anchor href |
| M4 | MEDIUM | `apps/explorer/src/lib/release-intelligence/action-run.ts` ~686–696 | `git worktree add` writes into `QUALITY_PROJECT_ROOT/.git/` — violates stated read-only contract unless documented |
| L1 | LOW | `packages/core/src/release-intelligence/operations.ts` ~2462 | `parseRepoFullName` silently accepts three-segment paths |
| L2 | LOW | `apps/explorer/src/lib/release-intelligence/action-run.ts` ~299 | Preview cache key omits `GITHUB_TOKEN` |
| L3 | LOW | `packages/quality-tools/` (not in diff) | Verify size-gate compliance after new `quality-core` entry points are bundled |
There was a problem hiding this comment.
Release Intelligence Migration — PR Review
Overview
This PR migrates Release Intelligence contracts and analysis into @shiplightai/quality-core, adds reusable views to @shiplightai/quality-ui, and wires local GitHub Actions run analysis into the Quality Explorer. The architecture is sound: provider-neutral types, immutable snapshots, host-owned persistence, and the deterministic assessment engine. Coverage across unit, integration, and contract tests is solid. However, several issues require attention before merging.
Findings
HIGH — Explorer writes to scanned project's .git/ directory
File: apps/explorer/src/lib/release-intelligence/action-run.ts:687-711
withCommitCheckout runs git worktree add with -C projectPath, which writes into QUALITY_PROJECT_ROOT/.git/worktrees/. CLAUDE.md states: "Explorer is strictly read-only against the project it inspects. Flag any write, mkdir, or delete targeting the scanned project root." The Explorer README reinforces this: "it does not fetch into or modify the selected repository" — but that claim is false when the commit is available locally, because git worktree add does mkdir/write inside the project's .git/.
The cleanup in the finally block runs git worktree remove --force with .catch(() => undefined) followed by rm(temporaryRoot, ...). If worktree remove fails silently and the temp directory is deleted first, the project ends up with a stale .git/worktrees/ entry requiring manual git worktree prune.
Required: Either document and accept the transient write to .git/worktrees/ (update the README claim accordingly), or always use the archive path to avoid any write to the project's git metadata. If the worktree path is kept, propagate worktree remove errors instead of silently swallowing them.
MEDIUM — evaluateReleasePolicy defaults now to new Date(), making exception evaluation non-deterministic
File: packages/core/src/release-intelligence/policy.ts
const now = input.now ?? new Date();
now is used to decide whether exceptions are still active. If a host calls evaluateReleasePolicy with exceptions but omits now, two calls to the same frozen inputs at different wall-clock times will produce different decisions. This violates the determinism invariant for the public API exported from @shiplightai/quality-core/release-intelligence.
The local preview path in this PR is safe (exceptions are always [] in buildPreviewModel), but the function is exported as a public contract. The policy tests already pass now explicitly every time, suggesting determinism is intentional — the default should be removed and now made required, or at minimum the API doc must state that callers must supply now for reproducible results.
MEDIUM — parseRepoFullName silently truncates multi-segment paths
File: packages/core/src/release-intelligence/operations.ts
function parseRepoFullName(value: string) {
const [owner, name] = value.split("/");
if (!owner || !name) return null;
return { owner, name };
}
A value like "ShiplightAI/shipyard/extra" destructures to owner="ShiplightAI", name="shipyard", silently discarding "extra". This function gates observationProfileTargetsRepository and validates input.repository in compileReleaseFactsOp. A malformed coordinate could pass validation when it should be rejected. Add a check that value.split('/').length === 2 before returning a result.
LOW — Silent swallow of git worktree remove errors
File: apps/explorer/src/lib/release-intelligence/action-run.ts (finally block in withCommitCheckout)
If worktree remove fails (e.g., permission issue) but rm succeeds, the project is left with a stale worktree entry. At minimum, log the error or surface it as a diagnostic so the developer knows to run git worktree prune.
LOW — Over-indented function body in compileReleaseFactsOp
File: packages/core/src/release-intelligence/operations.ts
The body of compileReleaseFactsOp is indented by an extra level after the two guard clauses, making the entire function body appear nested when it is not. Style inconsistency that makes the block structure confusing.
LOW — githubResponse error drops the response body
File: apps/explorer/src/lib/release-intelligence/action-run.ts
throw new Error(`GitHub API ${path} returned ${response.status}.`);
GitHub returns structured error JSON (rate-limit messages, repo-not-found details) in non-2xx responses. Dropping the body makes debugging 403/404/422 failures harder. Consider reading and including the response body in the error message.
LOW — Cache key omits GITHUB_TOKEN
File: apps/explorer/src/lib/release-intelligence/action-run.ts
The global cache key does not include the token. If a developer restarts with a different GITHUB_TOKEN but identical other env vars, the cached promise from the previous token is returned. In practice a process restart clears the global, so risk is minimal — but a short comment explaining this design choice would prevent future confusion.
Not Flagged (by design)
- Score independence:
assessBehavior,evaluateReleasePolicy, andbuildReleaseSystemFactsare pure functions of immutable inputs. No agent or LLM touches scores. - Human-gated fields: No code promotes
reviewStatusorstructure_provenanceautomatically. - Dependency direction:
release-intelligencelives incoreandui; the Explorer adapter is the only host. No upward imports observed. quality-toolssurface: No changes topackages/quality-tools, CLI flags, or exported commands.- Secret handling:
diagnosticSecrets: [token]is passed tocompileReleaseFactsOpandredactDiagnosticSecretsremoves the token from all persisted diagnostics. - Token validation: Commit SHA validated as a full 40-hex string before use in git commands or GitHub URLs; repository coordinates validated before fetch calls; SSH remote parsing handles non-standard host aliases correctly.
- Test coverage: assessment, evidence, policy, system-facts, components, fix-prompt, workflow-reference, and operations each have unit tests; contract tests guard the published API surface; integration test exercises fact compilation against a fixture project.
There was a problem hiding this comment.
Review of PR 18: feat: migrate release intelligence
Overview
This PR migrates provider-neutral Release Intelligence contracts and analysis into @shiplightai/quality-core, adds reusable UI components to @shiplightai/quality-ui, and wires up a local GitHub Actions run preview in the Explorer. The architecture is sound: GitHub API calls and temporary checkouts are isolated to the Explorer adapter, the engine functions (assessBehavior, evaluateReleasePolicy) are pure and provider-neutral, and the UI correctly stays behind a host-provider context. The test coverage (unit, integration, and two new contract tests) is thorough.
One issue requires a fix before merging.
Findings
MEDIUM — now: new Date() on the scoring path violates the policy engine's determinism contract
File: apps/explorer/src/lib/release-intelligence/action-run.ts, buildPreviewModel()
evaluateReleasePolicy carries an explicit JSDoc on its now parameter:
"Evaluation time is an explicit fact so identical inputs always produce identical decisions."
That contract exists precisely so callers cannot accidentally introduce wall-clock dependence. buildPreviewModel breaks it:
const decision = evaluateReleasePolicy({
policy: ...,
assessments,
systemFacts,
now: new Date(), // <- wall clock
});Because exceptions is hardcoded to [] in the returned model, now is currently a no-op — exception expiry filtering has nothing to iterate over. But this is a latent defect: if exception support is added to the preview path (a natural next step), the decision will silently become time-dependent on identical inputs. The scoring-path determinism invariant exists precisely to prevent this class of silent drift.
Fix: anchor evaluation to the run's own timestamp, e.g. new Date(input.run.created_at) or new Date(input.run.updated_at). This makes the preview decision a pure function of the run data.
LOW — UI platform-isolation contract test uses a hardcoded file list
File: tests/contract/release-intelligence-ui.contract.test.ts
The test that enforces @shipyard/, drizzle-orm, node:fs / node:child_process etc. are absent reads a fixed array of source file names. The equivalent test for the core package uses readdirSync so new files are covered automatically. The UI's client-bundle-boundary.test.ts already does the same — the platform-isolation check in the contract test should follow that pattern to avoid a gap if a new component is added.
LOW — tar extraction has no path-traversal hardening
File: apps/explorer/src/lib/release-intelligence/action-run.ts, materializeLocalArchive and materializeGitHubArchive
Both functions run tar -xzf archivePath -C checkoutPath without --no-absolute-paths. GNU tar strips absolute paths by default, and both archive sources (a git archive pipe and GitHub's authenticated tarball endpoint pinned to a validated 40-character SHA) are trusted, so practical risk is very low. Adding --no-absolute-paths would be cheap defence-in-depth and would make the confinement intent explicit.
LOW — Missing unit tests for the archive materialization and preview-build paths
File: apps/explorer/src/lib/release-intelligence/action-run.ts
action-run.test.ts covers parseGitHubRemote and resolveActionRunReference. The more complex functions — withCommitCheckout, materializeLocalArchive, materializeGitHubArchive, and buildPreviewModel — are exercised only end-to-end. The finally block that removes the temporary directory on error is correct but not unit-isolated.
What is good
- Dependency direction is clean.
quality-corenever imports fromapps/explorer; the UI never touches Node.js, GitHub, or auth; the Explorer adapter correctly sits at the top of the stack. - GITHUB_TOKEN is never sent to the client.
page.tsxis an async Server Component;ReleasePreviewModelserialized to the component tree carries only facts and decisions — no credentials.diagnosticSecrets: [token]scrubs the token from every persisted diagnostic. - Engine functions are pure and deterministic.
assessBehavior,collectWorkflowEvidence,buildReleaseSystemFacts,evaluateReleasePolicy,redactDiagnosticSecretstake explicit inputs and produce reproducible outputs with no ambient state — thenowissue is the sole exception. - Commit SHA is validated at every entry point.
/^[0-9a-f]{40}$/is checked on the GitHub response beforegit cat-fileandgit archiveare called;compileReleaseFactsOpre-validates before scanning. - Repository cross-check.
run.repository.full_nameis compared against the resolved reference after every GitHub fetch; a redirected or substituted run is caught before any checkout starts. - UI size gate follows the correct approval flow. The
check-package-size.tsscript forquality-uitreats re-baselining as the explicit approval (consistent with its own comments), so the baseline update 0.1.0 → 0.1.1 in the same commit is the documented approval path for that package. - Contract and integration tests provide solid coverage of the public surface, dependency isolation, and the happy-path fact-compilation pipeline.
There was a problem hiding this comment.
Review: feat/release-intelligence-migration
This PR migrates Release Intelligence contracts into @shiplightai/quality-core, adds reusable UI components to @shiplightai/quality-ui, and wires a local preview into Quality Explorer. The architecture is well-structured — scoring is deterministic, evidence stays in the engine, and platform code is correctly separated from the shared UI. Two issues need attention before merge.
HIGH
1. extractArchive does not reject absolute symlink targets
apps/explorer/src/lib/release-intelligence/action-run.ts lines 747–766
tar -tzf lists symlinks as path/to/link -> target. The current guard:
.find((entry) => entry.startsWith("/") || entry.split("/").includes(".."))catches /absolute-path archive members and relative ../ escapes, but misses a symlink whose target is absolute. For example, ./link -> /etc/passwd produces the listing entry ./link -> /etc/passwd, which does not start with / and contains no .. component. After extraction, checkoutPath/link resolves to /etc/passwd, and any quality-map analysis that reads a file at that path will follow the symlink out of the sandbox.
The risk is real for git-archived commits (git archive preserves symlinks verbatim) from a repository the developer does not fully control, and would compound if this extractor is reused in a multi-tenant hosted environment.
Fix: add a symlink-target check to the listing scan, e.g. split on -> and separately validate both the source path and the link target; or pass --no-dereference to the extraction call so symlinks are created but never followed.
MEDIUM
2. compileReleaseFactsOp body is over-indented after the validation guards
packages/core/src/release-intelligence/operations.ts
After the two early-exit guards and const root = input.projectPath; (2-space function-body indent), every subsequent statement uses 6-space indent — 4 extra spaces. This is a refactor artifact: the body was almost certainly cut from a try block that no longer exists. It is not a runtime bug, but it makes structural intent ambiguous. A reader will wonder whether the indentation signals a scope that was silently removed, and any future try/finally cleanup wrapper will be invisible because the existing indent already looks nested.
Fix: dedent the body to 2-space function scope.
3. @shiplightai/quality-core adds public exports and a new production dependency without a visible version bump
packages/core/package.json
Two new export entries (./release-intelligence, ./release-intelligence/operations) and a new production dependency (zod: 3.25.76) are introduced, but the version field does not change in this diff (it remains 0.3.1). Per CLAUDE.md, a patch bump is required by default, and the publish workflow skips a package already at its declared version on npm. If 0.3.1 has already been published, these additions will never ship until a bump lands. Please verify with npm view @shiplightai/quality-core version and bump to 0.3.2 if needed.
(quality-ui correctly bumps to 0.1.1 and updates package-size.json; the concern is quality-core only.)
LOW
4. redactDiagnosticSecrets heuristic regex may miss newer GitHub token formats
packages/core/src/release-intelligence/operations.ts
The pattern gh[pousr]_ covers classic PATs, OAuth, user, installation, and refresh tokens but not e.g. ghc_ (GitHub CLI). Because explicit diagnosticSecrets string replacement is the primary defence this is low risk, but consider widening the character class to gh[a-z]_ to be more future-proof.
5. Dual IssueEvidenceDrawer instances in ReleaseDetail
packages/ui/src/release-intelligence/ReleaseDetail.tsx
ReleaseDetail renders a drawer driven by assessmentId (set by FeatureBrowser) and Summary renders a second driven by selectedIssueId. With keepMounted={false} these do not conflict in practice, but a maintainer adding state persistence could accidentally open both. A brief comment explaining why two drawers coexist would help future readers.
What looks good
- Score independence preserved:
assessBehavior,collectWorkflowEvidence,evaluateReleasePolicy, andbuildReleaseSystemFactsare called from the deterministic engine; no LLM, agent, or UI component writes a score. Thefix-promptpath builds text for a developer to copy — it is advisory, not a scoring path. - Determinism:
evaluateReleasePolicyreceivesnow: new Date(input.run.updated_at)(the run's immutable timestamp), notDate.now(). Same inputs produce the same decision. - Dependency direction:
packages/coredoes not import fromapps/explorer;packages/uiimports no GitHub, filesystem, or auth code — both verified by the contract tests. - Project-root confinement: Temporary checkouts land under
os.tmpdir(), not insideQUALITY_PROJECT_ROOT. Thefinallyblock guarantees cleanup.compileReleaseFactsOpdocuments that hosts must not pass client-supplied paths. - Token hygiene: The GITHUB_TOKEN is hashed in the cache key (never stored raw), error messages expose only the API path and HTTP status, and
diagnosticSecretsredacts the token from diagnostics before they are persisted. - Pagination safety: Jobs and artifacts are fetched with
per_page=100; if either result set is truncated,evidenceTruncated: trueblocks the analysis as a critical system fact — no partial-evidence decision slips through. - Test coverage: Unit tests for every new engine module, integration test for
compileReleaseFactsOp, contract tests verifying no platform imports leak intopackages/coreorpackages/ui, and UI component tests covering the happy path and failure modes.
|
Addressed the latest review in
For the core version: |
There was a problem hiding this comment.
Release Intelligence Migration — Review
Overview
This PR migrates provider-neutral Release Intelligence contracts into packages/core, adds reusable views to packages/ui, and wires a GitHub Actions analysis adapter into apps/explorer. The architecture is clean, tests are thorough, and the security-sensitive paths (archive extraction, symlink checks, secret redaction) are well-tested with explicit contract tests. All project-critical invariants pass.
Invariant Checks
| Invariant | Result | Notes |
|---|---|---|
| Independence of scoring | ✅ PASS | assessBehavior, evaluateReleasePolicy, collectWorkflowEvidence are pure deterministic functions in packages/core. No LLM, agent, or skill writes or adjusts any score. |
| Human-gated fields | ✅ PASS | structure_provenance, user_authored, and spec provenance fields are untouched. Exception approval requires canApproveExceptions explicitly granted by the host; the UI makes no attempt to self-grant. |
| Determinism | ✅ PASS | evaluateReleasePolicy receives now: Date as an explicit parameter sourced from input.run.updated_at — not Date.now(). No Math.random() on the scoring path. The Date.now() calls in AnalysisAutoRefresh.tsx are client-side UI timers, not part of scoring. |
| Dependency direction | ✅ PASS | quality-map ← core ← ui ← explorer. Contract tests in tests/contract/release-intelligence.contract.test.ts explicitly verify that packages/core/src/release-intelligence imports no filesystem, platform, or upward-chain modules. |
| Explorer read-only against project root | ✅ PASS | All filesystem writes in action-run.ts use paths under mkdtemp(join(tmpdir(), ...)). The scanned project root is only ever passed to read-only git commands (git -C, git archive, git cat-file). withCommitCheckout cleans up via rm(..., { recursive: true, force: true }) in a finally block. |
| Project-root confinement | ✅ PASS | QUALITY_PROJECT_ROOT is resolved once at process startup. API handlers ignore client-supplied paths. The checkout materializes in a separate temp directory with no symlink traversal out of it. |
| Schema-version compatibility | ✅ PASS | No schema_version bump introduced by this PR. |
Published surface (quality-tools) |
✅ PASS | @shiplightai/quality-tools is not modified. New exports are additive sub-paths on quality-core and quality-ui. |
| Agent-skill safety | ✅ PASS | Agent-skill SKILL.md files are not modified. |
| Docs accuracy | ✅ PASS | Explorer README accurately documents RELEASE_ACTION_RUN, RELEASE_ACTION_RUN_ID, GITHUB_TOKEN, the tmp-only write constraint, and the new UI page route. |
Findings
LOW — runId interpolated into GitHub API URL paths without encodeURIComponent
apps/explorer/src/lib/release-intelligence/action-run.ts lines 134–142
reference.runId is interpolated directly into API URL paths (e.g., `/actions/runs/${reference.runId}`). In practice the value is validated against /^\d+$/ before use, so actual injection is impossible and encodeURIComponent on digits is a no-op. Still, applying encodeURIComponent(reference.runId) (along with owner and repo) is a good defensive habit for URL path segments, especially if future callers extend this function.
LOW — Archive listing safety check does not reject null bytes
apps/explorer/src/lib/release-intelligence/action-run.ts lines 569–581 (archiveListingHasUnsafePath)
The check correctly rejects absolute paths and .. components in both entry names and symlink targets. It does not check for null bytes, though GNU tar normalizes these away before emitting the listing output, and the defense-in-depth of writing only under tmpdir() means this is not exploitable. Low risk; a null-byte check would harden the implementation further.
LOW — Locale-sensitive sort in releaseComponentsSchema.transform
packages/core/src/release-intelligence/components.ts lines 7–9
The sort comparator uses JavaScript's </> string operators, which compare by Unicode code point order (deterministic across environments). This is not locale-sensitive and causes no determinism issue. Noted for completeness; no action required.
LOW — GitHub API 403/429 surfaces an opaque error with no user guidance
apps/explorer/src/lib/release-intelligence/action-run.ts lines 579–605
Rate-limit and auth errors produce "GitHub API /path returned 403." with no suggestion to check GITHUB_TOKEN or rate-limit quota. For a dev tool this is acceptable; a follow-up UX improvement would improve the developer experience.
LOW — quality-ui size baseline increased 33% without approvedIncrease
packages/ui/package-size.json
The baseline rises from ~39 KB packed / ~166 KB unpacked to ~52 KB / ~219 KB (~33% increase). This is permitted for quality-ui, which uses re-baselining as its approval mechanism (unlike quality-tools, which requires an explicit approvedIncrease entry per CLAUDE.md). Verify that the CI size gate ran and passed against the new numbers before merging.
Summary
Solid, well-tested migration. All project-critical invariants are satisfied — scoring is fully deterministic and engine-computed, the Explorer is strictly read-only against the project root, dependency direction is respected, and human-gated provenance fields are untouched. The LOW findings above are defence-in-depth suggestions; none blocks merging. Approving.
1 participant
Summary
@shiplightai/quality-core@shiplightai/quality-uiLocal preview
Release Intelligence can be previewed with
GITHUB_TOKEN,QUALITY_PROJECT_ROOT, and eitherRELEASE_ACTION_RUNorRELEASE_ACTION_RUN_ID.Notes
The UI package remains host-independent; GitHub, filesystem, authentication, and temporary-checkout handling live in the Explorer adapter.