Skip to content

fix(arg-parsing): allow project display names with spaces in org/project argument#1116

Merged
MathurAditya724 merged 8 commits into
mainfrom
seer/fix/arg-parsing-project-display-names
Jun 24, 2026
Merged

fix(arg-parsing): allow project display names with spaces in org/project argument#1116
MathurAditya724 merged 8 commits into
mainfrom
seer/fix/arg-parsing-project-display-names

Conversation

@sentry

@sentry sentry Bot commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

This PR addresses CLI-1RA, where sentry issue list (and other commands using parseOrgProjectArg) would throw a ValidationError when a project display name containing spaces was provided in the org/project argument format (e.g., sentry/My Project).

The root cause was an inconsistency in src/lib/arg-parsing.ts. While bare project slugs (My Project) and leading-slash project slugs (/My Project) correctly fell back to a fuzzy display-name search when spaces were detected, the explicit org/project path in parseSlashOrgProject directly called validateResourceId, which strictly rejects spaces.

This fix aligns the behavior across all input forms:

  1. src/lib/arg-parsing.ts: In parseSlashOrgProject, if the project segment of an org/project argument contains spaces (i.e., looksLikeDisplayName is true), it now returns a project-search type, preserving the provided organization slug.
  2. src/lib/arg-parsing.ts: The ParsedOrgProject type's project-search variant was updated to include an optional org field to carry this context.
  3. src/lib/resolve-target.ts: In resolveOrgProjectTarget, the display-name based project search (isDisplayName path) now filters the list of organizations to search within if an org is provided in the project-search object, ensuring the search remains scoped to the user's explicit organization.

Fixes CLI-1RA

@github-actions

github-actions Bot commented Jun 20, 2026

Copy link
Copy Markdown
Contributor
PR Preview Action v1.8.1

QR code for preview link

🚀 View preview at
https://cli.sentry.dev/_preview/pr-1116/

Built to branch gh-pages at 2026-06-23 22:38 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

Comment thread src/lib/resolve-target.ts
Comment thread src/lib/resolve-target.ts
Comment thread src/lib/resolve-target.ts Outdated
@MathurAditya724 MathurAditya724 added the jared Trigger the Jared agent to work on stuff label Jun 22, 2026

@jared-outpost jared-outpost Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

the approach is right — falling back to project-search when the project segment has spaces — but the implementation is incomplete:

3 tests fail. parseOrgProjectArg("sentry/My Project") no longer throws ValidationError, but three tests still assert it does:

  • test/lib/arg-parsing.test.ts:220 — "project with spaces in explicit mode throws ValidationError"
  • test/lib/arg-parsing.test.ts:231 — "org with underscores and project with spaces throws ValidationError"
  • test/lib/arg-parsing.test.ts:1133 — "underscores with spaces in explicit mode throws ValidationError"

These need to be updated to assert the new project-search return shape (with org field).

resolveTargetsFromParsedArg still broken for display names. This function (used by issue list, all alert commands) unconditionally calls findProjectsBySlug(parsed.projectSlug) in the project-search case (line 1882). When parsed.originalSlug is set, it should skip the slug lookup and go straight to triageProjectNotFound — the same pattern already used in resolveOrgProjectTarget (line 1643) and handleProjectSearch (line 786). It also needs to filter orgs by parsed.org when present.

The Cursor Bugbot comments flagged the same gap — the parse-layer fix alone doesn't help if the resolution layer still passes a display name to findProjectsBySlug.

@MathurAditya724

Copy link
Copy Markdown
Member

@jared-outpost fix all these issues

…pport

- Update 3 failing tests that still expected ValidationError for
  org/project args with spaces in the project segment
- Fix resolveTargetsFromParsedArg to handle display names: skip
  findProjectsBySlug when originalSlug is set, use triageProjectNotFound
  for fuzzy matching, and scope org search when parsed.org is present
  (mirrors the pattern in resolveOrgProjectTarget and handleProjectSearch)
@jared-outpost

jared-outpost Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

pushed fbeb61a with both fixes:

  1. tests — updated the 3 tests that still expected ValidationError for org/project args with spaces in the project segment. they now assert the new project-search return shape with the org field.

  2. resolveTargetsFromParsedArg — applied the same display-name handling pattern used in resolveOrgProjectTarget and handleProjectSearch: when originalSlug is set, skip findProjectsBySlug and go through triageProjectNotFound for fuzzy matching. also scopes the org search when parsed.org is present.

all arg-parsing (180), property (34), resolve-target (151), and org-list (45) tests pass.

Comment thread src/lib/resolve-target.ts
@github-actions

github-actions Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Codecov Results 📊

✅ Patch coverage is 84.21%. Project has 5047 uncovered lines.
✅ Project coverage is 81.38%. Comparing base (base) to head (head).

Files with missing lines (3)
File Patch % Lines
src/lib/resolve-target.ts 71.43% ⚠️ 8 Missing and 7 partials
src/commands/project/list.ts 91.67% ⚠️ 1 Missing and 3 partials
src/lib/org-list.ts 100.00% ⚠️ 3 partials
Coverage diff
@@            Coverage Diff             @@
##          main       #PR       +/-##
==========================================
+ Coverage    81.34%    81.38%    +0.04%
==========================================
  Files          392       392         —
  Lines        27070     27101       +31
  Branches     17566     17608       +42
==========================================
+ Hits         22019     22054       +35
- Misses        5051      5047        -4
- Partials      1832      1841        +9

Generated by Codecov Action

Comment thread src/lib/resolve-target.ts
@MathurAditya724

Copy link
Copy Markdown
Member

@jared-outpost did you resolved all the issues?

@jared-outpost

jared-outpost Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

the two issues from my review are fixed (test assertions + resolveTargetsFromParsedArg display-name handling). CI was failing on a biome formatting nit in resolve-target.ts — just pushed 5854391 to fix that. waiting on CI to go green now.

Comment thread src/lib/resolve-target.ts Outdated
Comment thread src/lib/arg-parsing.ts
@MathurAditya724

Copy link
Copy Markdown
Member

@jared-outpost there are still pending review comments, resolve them

Both handleProjectSearch implementations (org-list.ts and
project/list.ts) now accept and use the parsed org to scope
display-name searches, matching the pattern already applied in
resolveTargetsFromParsedArg and resolveOrgProjectTarget.
@jared-outpost

jared-outpost Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Pushed 54981ae — both handleProjectSearch implementations (in org-list.ts and project/list.ts) now forward parsed.org and filter the org list for display-name searches. This closes the gap where sentry team list org/My Project (and similar commands using dispatchOrgScopedList) would search across all orgs instead of the one the user specified.

The three items from my earlier review are all resolved:

  • Tests updated (done in prior commit)
  • resolveTargetsFromParsedArg fixed (done in prior commit)
  • handleProjectSearch org scoping (this commit)

Comment thread src/lib/org-list.ts
Comment thread src/lib/resolve-target.ts
@jared-outpost

jared-outpost Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

fix-ci: attempt 1 — lint error useMaxParams on handleProjectSearch in project/list.ts (5 params, max 4). refactoring to use an options object.

Comment thread src/commands/project/list.ts Outdated
Comment thread src/commands/project/list.ts Outdated
@jared-outpost

jared-outpost Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

fix-ci: attempt 2 — biome formatting on the options object in handleProjectNotFound. running lint:fix.

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

There are 4 total unresolved issues (including 3 from previous reviews).

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 0bf3b84. Configure here.

Comment thread src/lib/resolve-target.ts
Comment thread src/commands/project/list.ts
@jared-outpost jared-outpost Bot dismissed their stale review June 22, 2026 10:09

all requested changes have been implemented and CI is green

@MathurAditya724

Copy link
Copy Markdown
Member

@jared-outpost, you missed a few review comments

…org IDs

- Pass scopedOrg through handleProjectNotFound to fuzzy recovery calls
  in both project/list.ts and org-list.ts
- Remove isDisplayName guard from scopedOrg filter so org scoping
  applies to slug-based recovery lookups too
- Call resolveEffectiveOrg for project-search mode when org is set,
  handling DSN-style identifiers (e.g. o1081365 → my-org)
- Improve error messages when specified org is inaccessible to
  distinguish from the generic 'any accessible organization' message
- Extract resolveOrgInParsed helper to keep dispatchOrgScopedList
  within cognitive complexity budget
@jared-outpost

jared-outpost Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

fixed in 5d90116 — the org scope was being dropped in fuzzy recovery paths because:

  1. handleProjectNotFound in project/list.ts didn't receive or forward scopedOrg to the recovery handleProjectSearch call
  2. the org filter was gated on isDisplayName && scopedOrg, so slug-based recovery lookups (where originalSlug is cleared) lost the org constraint
  3. resolveEffectiveOrg wasn't called for project-search mode, so DSN-style org identifiers (e.g. o1081365) wouldn't resolve

also improved the error message when a specified org is inaccessible — it now says which org was searched instead of the generic "any accessible organization".

Comment thread src/lib/org-list.ts

@MathurAditya724 MathurAditya724 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I reviewed this against the stated intent: allow display-name projects in org/project form while preserving explicit-org scoping consistently. This is not ready to merge because the implementation still loses org scoping in important resolution paths.

Blocking findings:

  • src/commands/project/list.ts:625-630 scopes only foundOrgs, but projects remains unscoped. After fuzzy recovery, handleProjectNotFound recurses with scopedOrg, but the recovered slug lookup calls findProjectsBySlug() across all orgs and returns from the unscoped projects array at src/commands/project/list.ts:647-660. sentry project list org-a/My Project can therefore return org-b/<recovered-slug> if that recovered slug exists outside org-a.
  • src/lib/org-list.ts:801-804 has the same issue in the shared list handler. orgs is filtered to the explicit org, but matches is not; the later fetch path at src/lib/org-list.ts:865-879 operates on unscoped matches.
  • src/lib/resolve-target.ts:1840-1857 still does not resolve DSN-style org IDs for explicit or org-all branches. DSN-style org resolution was added only in the project-search branch at src/lib/resolve-target.ts:1898-1901, so commands that directly use resolveTargetsFromParsedArg(parseOrgProjectArg(...)) remain inconsistent for inputs like o123/my-project or o123/.

Test coverage is also too parser-heavy for this change. I would expect tests for cross-org duplicate recovered slugs, shared org-list recovery with scoped org, and DSN-style org identifiers through explicit and org-all resolver branches.

The org-scoping introduced for display names only filtered the orgs list,
not the projects/matches array. Since findProjectsBySlug fans out across
every accessible org, a recovered or directly-matched slug that also exists
in another org could leak into a result that was explicitly scoped to one
org (e.g. 'org-a/My Project' returning 'org-b/<slug>').

Filter the matched projects by scopedOrg in:
- project/list.ts handleProjectSearch (recovery path)
- org-list.ts handleProjectSearch (shared list handler)
- resolve-target.ts resolveOrgProjectTarget + resolveTargetsFromParsedArg
  project-search branches

Also resolve DSN-style org identifiers (o123/...) in the explicit and
org-all branches of resolveTargetsFromParsedArg, which previously only
happened in project-search — making it consistent with
resolveOrgProjectTarget.

Adds tests for cross-org duplicate recovery, shared org-list scoped
recovery, and DSN-style org IDs through the explicit/org-all branches.
@github-actions github-actions Bot added the risk: high PR risk score: high label Jun 23, 2026
Comment thread src/lib/resolve-target.ts
? await resolveEffectiveOrg(parsed.org)
: undefined;

const { projects: rawMatches, orgs: foundOrgs } = isDisplayName

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

addressed all three blocking findings in 607647e — the root issue was that org-scoping only filtered the orgs list, not the matched projects/matches, so a recovered or directly-matched slug from another org could still leak through.

  • project/list.ts handleProjectSearch: recovered-slug lookup now filters projects by scopedOrg
  • org-list.ts handleProjectSearch: shared list handler filters matches by scopedOrg before the fetch fan-out
  • resolve-target.ts: both resolveOrgProjectTarget and resolveTargetsFromParsedArg project-search branches filter matches by scopedOrg; also added resolveEffectiveOrg to the explicit and org-all branches of resolveTargetsFromParsedArg so o123/... resolves consistently

added tests for cross-org duplicate recovery, shared org-list scoped recovery, and DSN-style org IDs through the explicit/org-all resolver branches. each was verified to fail before the fix.

@jared-outpost jared-outpost Bot requested a review from MathurAditya724 June 23, 2026 22:38

@MathurAditya724 MathurAditya724 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Re-reviewed the latest head (607647e) against my previous findings.

The explicit-org scoping issue is fixed: project/list.ts, org-list.ts, and both relevant resolve-target.ts project-search paths now filter the matched project list, not just the org list. The DSN-style org gap is also fixed for resolveTargetsFromParsedArg explicit and org-all branches. The new tests cover scoped same-slug leaks and DSN-style org identifiers through the resolver paths I was concerned about.

CI is green. No remaining blocking findings from me.

@MathurAditya724 MathurAditya724 merged commit 6ba4134 into main Jun 24, 2026
30 checks passed
@MathurAditya724 MathurAditya724 deleted the seer/fix/arg-parsing-project-display-names branch June 24, 2026 05:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

jared Trigger the Jared agent to work on stuff risk: high PR risk score: high

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant