Skip to content

fix(validate): remote-reference awareness for linked-artifact validation (URL doc-refs no longer flagged/pruned as missing files) - #372

Merged
unbraind merged 3 commits into
mainfrom
fix/validate-remote-reference-awareness
Jun 26, 2026
Merged

unbraind merged 3 commits into
mainfrom
fix/validate-remote-reference-awareness

Conversation

@unbraind

Copy link
Copy Markdown
Owner

Problem (pm-k2n4)

A linked-artifact path (pm files / pm docs) is normally a workspace-relative or absolute local file, but pm docs --add legitimately records remote references too — most commonly an https:// URL to a PR, issue, or external design doc. (In this very repo, pm-bhmk/pm-v37g/pm-qawd each link https://github.com/unbraind/pm-cli/pull/362|364.)

Three validation surfaces blindly fs.stat every project-scoped path:

  1. buildFilesCheck (src/cli/commands/validate.ts) — a URL resolves to a bogus local path → stat fails → pushed into missing_linked_paths + staleLinkRows.
  2. classifyStaleLinkedPaths then labels it deleted (no basename candidate match).
  3. planStaleLinkPruneFixes prunes every deleted link → pm validate --prune-missing silently deletes the intentionally-recorded PR/URL doc reference. That is data loss of recorded context (project management = context management).
  4. validateLinkedPaths (pm files/docs --validate-paths) reports the URL as a missing_path for the same reason.

This is the gap left by pm-0v2m (GH-184), which introduced moved/deleted classification + --prune-missing without remote-reference awareness.

Fix

New pure core helper src/core/validate/linked-artifact-reference.ts:

  • isRemoteLinkedArtifactReference(path) — matches ^[a-z][a-z0-9+.-]+:\/\//i (a scheme:// authority, scheme ≥ 2 chars so a Windows drive prefix C:/… / C:\… / UNC //server/share and all relative/absolute POSIX paths stay local).

Wired into both stat-based surfaces:

  • buildFilesCheck — remote refs are collected into a new benign remote_linked_paths{,_count,_total,_truncated} details category and skipped before the existence probe, so they never enter missing_linked_paths, staleLinkRows, classification, or the --prune-missing plan.
  • validateLinkedPaths — remote refs go to a new remote_references[] field instead of missing_paths.

No new CLI flags or contract changes (output-shape additions only); existing behavior for genuine local files is unchanged.

Verification

  • pnpm typecheck clean.
  • Full coverage gate: 5131 tests pass, 100/100/100/100 (statements/branches/functions/lines).
  • contracts:check current · quality:static (incl. docstring coverage) pass · changelog:pm:check unchanged.
  • Manual temp-dir smoke (real built CLI): a PR-URL doc ref shows under remote_linked_paths (not missing_linked_paths); --prune-missing --dry-run plans only the genuine missing local file and never the URL; pm docs --validate-paths reports it under remote_references.

pm tracking

🤖 Generated with Claude Code

https://claude.ai/code/session_0162E7r4dACCeEAoJCigEAJG

… linked-artifact validation

pm validate --check-files and pm files/docs --validate-paths fs.stat'd every
project-scoped linked-artifact path, so an https:// PR/issue/design-doc URL
recorded via `pm docs --add` resolved to a bogus local path, was reported as a
"deleted" missing file, and — critically — was planned for deletion by
`pm validate --prune-missing`, silently destroying intentionally-recorded
context (project management = context management).

Add a pure core helper isRemoteLinkedArtifactReference (scheme:// detection,
2+ char scheme so Windows drive/UNC paths stay local) and wire it into both
stat-based surfaces:
- buildFilesCheck (validate.ts): remote refs collected into a benign
  remote_linked_paths{,_count,_total,_truncated} details category; never
  stat'd, classified moved/deleted, counted missing, or pruned.
- validateLinkedPaths (linked-artifacts.ts): remote refs reported in a new
  remote_references[] field instead of missing_paths.

Follow-up to pm-0v2m (GH-184), which introduced --prune-missing without
remote-reference awareness.

Refs pm-k2n4 (parent pm-mpbb; related pm-0v2m/pm-j8z6/pm-o2kc).

Verification: pnpm typecheck clean; full coverage gate 5131 tests
100/100/100/100; contracts:check current; quality:static (docstrings) pass;
changelog:pm:check unchanged; manual temp-dir smoke confirms a URL doc ref
stays in the remote category and is never targeted by --prune-missing --dry-run.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0162E7r4dACCeEAoJCigEAJG
@gemini-code-assist

Copy link
Copy Markdown

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @unbraind, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@unbraind

Copy link
Copy Markdown
Owner Author

@coderabbitai full review

@gemini-code-assist please review

@codex review

Focus areas:

  • The remote-reference predicate in src/core/validate/linked-artifact-reference.ts (scheme:// detection; correctness on Windows drive/UNC paths).
  • That remote refs are excluded from every consumer of the files-check (missing_linked_paths, classification, staleLinkPruneRows / --prune-missing) and from validateLinkedPaths.missing_paths.

@gemini-code-assist

Copy link
Copy Markdown

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@coderabbitai

coderabbitai Bot commented Jun 26, 2026 •

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@unbraind, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 46 minutes. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits.

🚦 How do rate limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro

Run ID: 62474688-19f5-4481-88de-63af56463570

📥 Commits

Reviewing files that changed from the base of the PR and between 682d94d and 7903e63.

📒 Files selected for processing (11)
  • .agents/pm/history/pm-k2n4.jsonl
  • .agents/pm/history/pm-z5nb.jsonl
  • .agents/pm/issues/pm-k2n4.toon
  • .agents/pm/plans/pm-z5nb.toon
  • CHANGELOG.md
  • src/cli/commands/linked-artifacts.ts
  • src/cli/commands/validate.ts
  • src/core/validate/linked-artifact-reference.ts
  • tests/unit/commands/files-docs-command.spec.ts
  • tests/unit/commands/validate-command.spec.ts
  • tests/unit/validate/linked-artifact-reference.spec.ts
📝 Walkthrough

Walkthrough

Remote URL-style linked artifact references are now detected separately from local paths in core validation, pm validate files checks, and linked-path validation. The helper and related command outputs were updated, and the associated issue and plan records were refreshed.

Changes

Remote linked artifact handling

Layer / File(s) Summary
Remote reference helper
src/core/validate/linked-artifact-reference.ts, tests/unit/validate/linked-artifact-reference.spec.ts
A helper now पहचान? Actually no, let's keep factual. A helper now detects remote URL-style linked artifact references, and unit tests cover URL, local path, Windows path, and malformed scheme cases.
Linked-path validation
src/cli/commands/linked-artifacts.ts, tests/unit/commands/files-docs-command.spec.ts
validateLinkedPaths now reports remote_references, skips filesystem probing for remote inputs, and the linked-path test asserts the new field.
Files check integration
src/cli/commands/validate.ts, tests/unit/commands/validate-command.spec.ts
The pm validate files check now tracks remote-linked paths separately, excludes them from missing/prune handling, and reports counts and summaries in the details payload.
Issue and plan records
.agents/pm/issues/pm-k2n4.toon, .agents/pm/history/pm-k2n4.jsonl, .agents/pm/history/pm-z5nb.jsonl, .agents/pm/plans/pm-z5nb.toon
The issue, history, and plan records add the remote-reference issue text, dependency links, and status audit updates.

Sequence Diagram(s)

sequenceDiagram
  participant Validate as validate.ts
  participant Helper as isRemoteLinkedArtifactReference()
  participant fsStat as fs.stat
  Validate->>Helper: inspect linked artifact reference
  alt remote reference
    Helper-->>Validate: true
    Validate-->>Validate: add path to remoteLinkedPaths
    Validate-->>Validate: skip stat, missing, moved, deleted, and orphan classification
  else local path
    Helper-->>Validate: false
    Validate->>fsStat: probe filesystem path
    fsStat-->>Validate: existing file or missing path result
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: remote-reference awareness that prevents URL doc refs from being flagged or pruned as missing.
Description check ✅ Passed The description is directly related to the changeset and clearly explains the bug, fix, and validation behavior changes.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@chatgpt-codex-connector

Copy link
Copy Markdown

To use Codex here, create a Codex account and connect to github.

@coderabbitai

coderabbitai Bot commented Jun 26, 2026 •

Copy link
Copy Markdown
🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== PR files =="
gh pr view 372 --repo unbraind/pm-cli --json files --jq '.files[].path'

echo
echo "== Diff summary =="
gh pr diff 372 --repo unbraind/pm-cli --name-only

echo
echo "== Outline: src/core/validate/linked-artifact-reference.ts =="
ast-grep outline src/core/validate/linked-artifact-reference.ts --view expanded || true

echo
echo "== File: src/core/validate/linked-artifact-reference.ts =="
cat -n src/core/validate/linked-artifact-reference.ts

echo
echo "== Outline: src/cli/commands/validate.ts =="
ast-grep outline src/cli/commands/validate.ts --view expanded || true

echo
echo "== Relevant symbols in src/cli/commands/validate.ts =="
rg -n -C 4 'buildFilesCheck|missing_linked_paths|remote_linked_paths|staleLinkRows|classifyStaleLinkedPaths|planStaleLinkPruneFixes' src/cli/commands/validate.ts || true

echo
echo "== Slice: src/cli/commands/validate.ts (buildFilesCheck area) =="
sed -n '1,280p' src/cli/commands/validate.ts

echo
echo "== Outline: src/core/linked-artifact/validate-paths.ts =="
ast-grep outline src/core/linked-artifact/validate-paths.ts --view expanded || true

echo
echo "== Relevant symbols in src/core/linked-artifact/validate-paths.ts =="
rg -n -C 4 'validateLinkedPaths|missing_paths|remote_references|isRemoteLinkedArtifactReference' src/core/linked-artifact/validate-paths.ts || true

echo
echo "== File: src/core/linked-artifact/validate-paths.ts =="
cat -n src/core/linked-artifact/validate-paths.ts

echo
echo "== Searches for consumers/callers =="
rg -n -C 3 'missing_linked_paths|remote_linked_paths|staleLinkRows|remote_references|validateLinkedPaths\s*\(' src || true

Length of output: 32529


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Locate validateLinkedPaths =="
rg -n -C 4 '\bvalidateLinkedPaths\b' src tests || true

echo
echo "== buildFilesCheck implementation slice =="
sed -n '1639,1835p' src/cli/commands/validate.ts

echo
echo "== linked-artifacts.ts outline =="
ast-grep outline src/cli/commands/linked-artifacts.ts --view expanded || true

echo
echo "== linked-artifacts.ts relevant symbols =="
rg -n -C 4 'validateLinkedPaths|missing_paths|remote_references|isRemoteLinkedArtifactReference|stat\(' src/cli/commands/linked-artifacts.ts || true

echo
echo "== linked-artifacts.ts full file =="
cat -n src/cli/commands/linked-artifacts.ts

echo
echo "== stale-file-classification references =="
rg -n -C 3 'classifyStaleLinkedPaths|summarizeStaleLinkedPathClassifications' src || true

echo
echo "== fix-planning references for stale link prune =="
rg -n -C 4 'planStaleLinkPruneFixes|type StaleLinkPruneRow' src/core src/cli || true

echo
echo "== remote reference tests =="
cat -n tests/unit/validate/linked-artifact-reference.spec.ts

echo
echo "== validate-command tests relevant to remote refs =="
rg -n -C 4 'remote_linked_paths|prune-missing|missing_linked_paths|remote reference|https://' tests/unit/commands/validate-command.spec.ts || true

echo
echo "== files/docs command tests relevant to remote refs =="
rg -n -C 4 'remote_references|missing_paths|validate-paths|https://' tests/unit/commands/files-docs-command.spec.ts || true

Length of output: 65893



@unbraind Full review complete — I didn’t find any blocking issues in the focus areas you called out.

What I verified:

  • src/core/validate/linked-artifact-reference.ts uses ^[a-z][a-z0-9+.-]+:// with trim(), so https:///ssh://-style references are detected, while C:/..., C:\..., //server/share, and normal relative/absolute local paths stay local.
  • tests/unit/validate/linked-artifact-reference.spec.ts explicitly covers the Windows drive and UNC cases, plus the single-letter-scheme guard.
  • In src/cli/commands/validate.ts, buildFilesCheck() branches on isRemoteLinkedArtifactReference() before any fs.stat(), so remote refs never enter missingLinkedPaths or staleLinkRows.
  • Because of that, stale classification and planStaleLinkPruneFixes() only operate on genuine local missing paths, so --prune-missing won’t target URL refs.
  • In src/cli/commands/linked-artifacts.ts, validateLinkedPaths() also skips fs.stat() for remote refs and returns them under remote_references, not missing_paths.
  • The regression tests line up with that behavior:
    • tests/unit/commands/validate-command.spec.ts checks remote refs are surfaced under remote_linked_paths, excluded from missing_linked_paths, and excluded from staleLinkPruneRows.
    • tests/unit/commands/files-docs-command.spec.ts checks validateLinkedPaths() reports the URL in remote_references and not in missing_paths.

One tiny non-blocking note: buildFilesCheck() stores trimmed remote refs in remote_linked_paths, while validateLinkedPaths() currently returns the original string in remote_references. That’s harmless, but if you want perfectly consistent output normalization across both surfaces, that’d be a small follow-up.

✅ Action performed

Full review finished.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
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 `@src/cli/commands/linked-artifacts.ts`:
- Around line 409-416: The remote-reference handling in linked-artifacts is
storing the raw relativePath while buildFilesCheck stores a trimmed path,
causing inconsistent output when the input has surrounding whitespace. Update
the remoteReferences collection in the linked-artifacts flow to use the same
trimmed value used by buildFilesCheck, and keep the
isRemoteLinkedArtifactReference check operating on the normalized path so both
code paths report the same shape.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro

Run ID: c5cc79bc-972e-4f97-8721-f065d73c010e

📥 Commits

Reviewing files that changed from the base of the PR and between 682d94d and c1921f1.

📒 Files selected for processing (10)
  • .agents/pm/history/pm-k2n4.jsonl
  • .agents/pm/history/pm-z5nb.jsonl
  • .agents/pm/issues/pm-k2n4.toon
  • .agents/pm/plans/pm-z5nb.toon
  • src/cli/commands/linked-artifacts.ts
  • src/cli/commands/validate.ts
  • src/core/validate/linked-artifact-reference.ts
  • tests/unit/commands/files-docs-command.spec.ts
  • tests/unit/commands/validate-command.spec.ts
  • tests/unit/validate/linked-artifact-reference.spec.ts

Comment thread src/cli/commands/linked-artifacts.ts Resolved
…ut-shape parity

Address CodeRabbit review on #372: validateLinkedPaths stored the untrimmed
path in remote_references while buildFilesCheck stores the trimmed value, so a
"  https://…  " input would surface with surrounding whitespace in one flow
only. Store the trimmed value in both; extend the unit test with a
whitespace-wrapped URL asserting the trimmed result.

Refs pm-k2n4.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0162E7r4dACCeEAoJCigEAJG
@unbraind

Copy link
Copy Markdown
Owner Author

Addressed the CodeRabbit nitpick in 9c7909e: validateLinkedPaths now stores the trimmed remote reference, matching buildFilesCheck's output shape, and the unit test now feeds a whitespace-wrapped URL asserting the trimmed result. pnpm typecheck clean.

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Jun 26, 2026 •

Copy link
Copy Markdown

@unbraind Thanks for the update — great catch on normalizing validateLinkedPaths to the trimmed remote reference, and good to hear pnpm typecheck is clean. Kicking off a full review now.


If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

✅ Action performed

Full review finished.

@coderabbitai

coderabbitai Bot commented Jun 26, 2026 •

Copy link
Copy Markdown

@unbraind Understood — I’m kicking off a full review with extra attention on the remote-reference predicate in src/core/validate/linked-artifact-reference.ts, including Windows drive/UNC handling, and on ensuring remote refs are excluded from missing_linked_paths, classification, staleLinkPruneRows / --prune-missing, and validateLinkedPaths.missing_paths.

✅ Action performed

Full review finished.

@coderabbitai

coderabbitai Bot commented Jun 26, 2026 •

Copy link
Copy Markdown

@unbraind Understood — I’m kicking off a full review of #372 with extra attention on:

  • src/core/validate/linked-artifact-reference.ts and the scheme:// predicate behavior, especially around Windows drive and UNC paths.
  • Whether remote refs are excluded everywhere they should be: missing_linked_paths, stale classification, staleLinkPruneRows / --prune-missing, and validateLinkedPaths.missing_paths.
✅ Action performed

Full review finished.

…rence fix

Close the remote-reference validation Issue (pm-k2n4) with resolution +
expected/actual evidence now that PR #372 ships the fix, and regenerate
CHANGELOG.md so the change lands under Unreleased → Fixed for the next
auto-release. changelog:pm:check reports unchanged.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0162E7r4dACCeEAoJCigEAJG
@unbraind

Copy link
Copy Markdown
Owner Author

Pushed 7903e63: closed pm-k2n4 with resolution evidence and regenerated CHANGELOG.md (the fix lands under Unreleased → Fixed). This commit is pm-tracker data + changelog only — no code change. changelog:pm:check reports unchanged.

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Jun 26, 2026 •

Copy link
Copy Markdown

@unbraind Understood — I’m kicking off a full review of #372, with attention to the pm-tracker / CHANGELOG.md-only delta in 7903e635.

✅ Action performed

Full review finished.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant