Trim Bluesky social follow targets#735
Conversation
Greptile SummaryThis PR fixes a bug where leading/trailing whitespace on a Bluesky follow target caused
Confidence Score: 5/5Safe to merge — the change is a single defensive trim with no behavioral side-effects beyond the intended fix. The production change is one line that cannot regress any existing behavior; it only widens the set of accepted inputs. The sole gap is a missing test case for the auto-detected bare-handle trim path, which is a coverage concern rather than a functional defect. No files require special attention; the optional additional test case in Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["parseSocialFollowTarget(input, explicitPlatform)"] --> B["input = input.trim() NEW"]
B --> C["normalizePlatform(explicitPlatform)"]
C --> D{platform and not bluesky?}
D -- Yes --> E["throw: unsupported platform"]
D -- No --> F["new URL(input)"]
F -- success --> G{host === bsky.app?}
G -- No --> H["throw: unsupported URL host"]
G -- Yes --> I["parse /profile/:actor/:tab segments"]
I --> J["return { platform, actor, source }"]
F -- throws --> K{platform === bluesky OR looksLikeBlueskyActor?}
K -- Yes --> L["return { platform: bluesky, actor: input.replace(@,''), source: profile }"]
K -- No --> M["throw: expected social account URL"]
Reviews (1): Last reviewed commit: "Trim social follow targets" | Re-trigger Greptile |
| it('trims whitespace around Bluesky handles and URLs', () => { | ||
| expect(parseSocialFollowTarget(' @alice.bsky.social ', 'bluesky')).toEqual({ | ||
| platform: 'bluesky', | ||
| actor: 'alice.bsky.social', | ||
| source: 'profile', | ||
| }); | ||
| expect(parseSocialFollowTarget(' https://bsky.app/profile/source.bsky.social/followers ')).toEqual({ | ||
| platform: 'bluesky', | ||
| actor: 'source.bsky.social', | ||
| source: 'followers', | ||
| }); | ||
| }); |
There was a problem hiding this comment.
The new test block covers trimming for (a) a handle with an explicit
'bluesky' platform and (b) a URL. It misses the auto-detection path where no explicit platform is passed and looksLikeBlueskyActor is relied upon — e.g., alice.bsky.social (no @, no explicit platform). Before the fix that input would slip through trimming and the regex in looksLikeBlueskyActor would reject it due to the leading space; the fix cures it, but there is no regression test for it. Adding one case here would close the coverage gap.
| it('trims whitespace around Bluesky handles and URLs', () => { | |
| expect(parseSocialFollowTarget(' @alice.bsky.social ', 'bluesky')).toEqual({ | |
| platform: 'bluesky', | |
| actor: 'alice.bsky.social', | |
| source: 'profile', | |
| }); | |
| expect(parseSocialFollowTarget(' https://bsky.app/profile/source.bsky.social/followers ')).toEqual({ | |
| platform: 'bluesky', | |
| actor: 'source.bsky.social', | |
| source: 'followers', | |
| }); | |
| }); | |
| it('trims whitespace around Bluesky handles and URLs', () => { | |
| expect(parseSocialFollowTarget(' @alice.bsky.social ', 'bluesky')).toEqual({ | |
| platform: 'bluesky', | |
| actor: 'alice.bsky.social', | |
| source: 'profile', | |
| }); | |
| // auto-detected bare handle (no @ prefix, no explicit platform) | |
| expect(parseSocialFollowTarget(' alice.bsky.social ')).toEqual({ | |
| platform: 'bluesky', | |
| actor: 'alice.bsky.social', | |
| source: 'profile', | |
| }); | |
| expect(parseSocialFollowTarget(' https://bsky.app/profile/source.bsky.social/followers ')).toEqual({ | |
| platform: 'bluesky', | |
| actor: 'source.bsky.social', | |
| source: 'followers', | |
| }); | |
| }); |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
|
CI is green for PR #735. Verification:
Note: package typecheck still hits pre-existing workspace resolution/type errors unrelated to this patch, as documented in the PR body. uGig invoice evidence has been sent for this PR. |
|
🤖 Auto-rebase: The branch was rebased successfully locally but could not be pushed to the fork. Please enable 'Allow edits from maintainers' in the PR settings, or rebase manually: |
6 similar comments
|
🤖 Auto-rebase: The branch was rebased successfully locally but could not be pushed to the fork. Please enable 'Allow edits from maintainers' in the PR settings, or rebase manually: |
|
🤖 Auto-rebase: The branch was rebased successfully locally but could not be pushed to the fork. Please enable 'Allow edits from maintainers' in the PR settings, or rebase manually: |
|
🤖 Auto-rebase: The branch was rebased successfully locally but could not be pushed to the fork. Please enable 'Allow edits from maintainers' in the PR settings, or rebase manually: |
|
🤖 Auto-rebase: The branch was rebased successfully locally but could not be pushed to the fork. Please enable 'Allow edits from maintainers' in the PR settings, or rebase manually: |
|
🤖 Auto-rebase: The branch was rebased successfully locally but could not be pushed to the fork. Please enable 'Allow edits from maintainers' in the PR settings, or rebase manually: |
|
🤖 Auto-rebase: The branch was rebased successfully locally but could not be pushed to the fork. Please enable 'Allow edits from maintainers' in the PR settings, or rebase manually: |
Fixes #734.
Summary
Verification
corepack pnpm vitest run packages/cli/src/social-follow.test.tsNote
corepack pnpm --filter @profullstack/sh1pt typecheckstill fails on pre-existing workspace resolution/type errors unrelated to this patch, including unresolved@profullstack/sh1pt-core,@profullstack/sh1pt-actions-fleet-core, and@profullstack/sh1pt-openapi/*.