Preserve live URL query trailing slashes by rissrice2105-agent · Pull Request #732 · profullstack/sh1pt

rissrice2105-agent · 2026-06-13T21:38:21Z

Fixes #731.

What changed

Keeps query strings and fragments intact when normalizing live-site URLs.
Still strips trailing slashes from the path segment.
Adds a regression test for query and fragment values ending in /.

Validation

corepack pnpm vitest run packages/cli/src/input.test.ts
corepack pnpm --filter @profullstack/sh1pt typecheck currently fails on existing workspace/type errors outside this change, including unresolved workspace package imports and unrelated implicit-any errors.

greptile-apps · 2026-06-13T21:42:19Z

Greptile Summary

This PR fixes a bug where normalizeUrl stripped trailing slashes from query string values (e.g., ?q=/) by replacing the blunt /\/+$/ regex with /\/+(?=[?#]|$)/ (without the global flag), so only the first slash-before-?/#/end-of-string is removed — which in a well-formed URL is always the path's trailing slash.

input.ts: normalizeUrl regex updated to use a lookahead, preserving query and fragment content when a path trailing slash is present.
input.test.ts: Two new assertions verify that /search/?q=/ strips only the path slash and that /docs/#/intro/ strips only the path slash before the fragment.

Confidence Score: 4/5

Safe to merge — the targeted bug is correctly fixed and tested; the remaining gap is a pre-existing limitation, not a regression.

The regex fix works correctly for every case where a path trailing slash precedes ? or #. The no-global-flag design is intentional and sound. The gap — a trailing slash at the very end of a query value when no path slash exists — was already broken under the old regex and is not a regression, but it leaves the fix slightly short of fully preserving all query/fragment trailing slashes.

Both changed files are small and low-risk; input.ts line 109 is the only logic change worth a second look for the edge case described above.

Important Files Changed

Filename	Overview
packages/cli/src/input.ts	Regex in `normalizeUrl` changed from `/\/+$/` to `//+(?=[?#]
packages/cli/src/input.test.ts	New regression test covers path-slash-before-query and path-slash-before-fragment; missing a case for query/fragment trailing slash without a preceding path slash.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["normalizeUrl(u)"] --> B{"URL contains\npath trailing slash\nbefore ? or #?"}
    B -- "Yes\ne.g. /search/?q=/" --> C["regex matches /\\/+(?=[?#]|$)/\nfirst match = path slash\nno g flag → stops here"]
    C --> D["Strips path slash only\n→ /search?q=/\n(query preserved ✓)"]
    B -- "No path slash\ne.g. /search?q=/" --> E["regex scans to end\nfirst match = trailing /\ninside query value"]
    E --> F["Strips query trailing slash\n→ /search?q=\n(query modified ⚠)"]
    B -- "No ? or # at all\ne.g. /pricing/" --> G["regex matches trailing /\nbefore end-of-string"]
    G --> H["Strips path slash\n→ /pricing ✓"]

_{Reviews (1): Last reviewed commit: "Preserve live URL query trailing slashes" | Re-trigger Greptile}

greptile-apps · 2026-06-13T21:42:22Z

-  return u.replace(/\/+$/, '');
+  // For live-site URLs keep query strings and fragments intact; only strip
+  // trailing slashes from the path segment.
+  return u.replace(/\/+(?=[?#]|$)/, '');


Query/fragment trailing slash still stripped when no path slash precedes it

When the URL has no path trailing slash but the query or fragment itself ends with /, the regex still matches that final / and strips it. For example, https://example.com/search?q=/ → https://example.com/search?q= because the regex scans left-to-right, finds no slash before ?, and then matches the / at end-of-string inside the query value. The no-g-flag design only protects fragment/query slashes when a path slash is found first.

A URL-based approach (new URL(u)) would be more surgical — it lets you rewrite only pathname and reconstruct the full URL with search and hash untouched, regardless of whether those components happen to end in /.

greptile-apps · 2026-06-13T21:42:23Z

+  it('preserves query and fragment trailing slashes for live-site urls', () => {
+    expect(resolveInput('https://example.com/search/?q=/').value).toBe(
+      'https://example.com/search?q=/',
+    );
+    expect(resolveInput('https://example.com/docs/#/intro/').value).toBe(
+      'https://example.com/docs#/intro/',
+    );
+  });


Missing coverage for query-only trailing slash (no path slash)

The test suite covers the case where a path trailing slash exists (/search/, /docs/) before the ? or #, but there's no test for the complementary case where there is no path trailing slash yet the query or fragment ends in / — e.g. https://example.com/search?q=/. Under the current implementation that URL normalizes to https://example.com/search?q= (trailing slash stripped), which may surprise callers. Adding an assertion for this input would make the boundary of the fix explicit and prevent silent regressions.

rissrice2105-agent · 2026-06-13T21:48:36Z

CI is green for PR #732.

Verification:

corepack pnpm vitest run packages/cli/src/input.test.ts

Note: the package typecheck still hits pre-existing workspace resolution errors unrelated to this patch, as documented in the PR body. uGig invoice evidence has been sent for this PR.

github-actions · 2026-06-14T00:51:12Z