Skip to content

Trim social follow action values#737

Merged
ralyodio merged 1 commit into
profullstack:masterfrom
rissrice2105-agent:codex/sh1pt-social-action-trim
Jun 14, 2026
Merged

Trim social follow action values#737
ralyodio merged 1 commit into
profullstack:masterfrom
rissrice2105-agent:codex/sh1pt-social-action-trim

Conversation

@rissrice2105-agent

Copy link
Copy Markdown
Contributor

Closes #736.

Summary

  • Trim surrounding whitespace before normalizing sh1pt promote social follow --action values.
  • Add a regression assertion for " unfollow ".

Verification

  • corepack pnpm vitest run packages/cli/src/social-follow.test.ts
  • corepack pnpm --filter @profullstack/sh1pt typecheck (fails on existing workspace/module-resolution errors unrelated to this change: missing @profullstack/sh1pt-core, @profullstack/sh1pt-openapi/*, etc.)

@greptile-apps

greptile-apps Bot commented Jun 13, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes whitespace-padded --action values (' follow ', ' unfollow ') being incorrectly rejected by normalizeFollowAction. The fix is a one-character .trim() call inserted before .toLowerCase(), and a regression test for the ' unfollow ' case is added.

  • social-follow.ts: .trim() is applied to the action string before case-normalisation; the error message still reflects the original (untrimmed) user input, which is the correct UX.
  • social-follow.test.ts: New assertion normalizeFollowAction(' unfollow ') === 'unfollow' guards the fix; the symmetric ' follow ' case is not yet tested.

Confidence Score: 5/5

Safe to merge — the change is a single .trim() call that fixes an edge case without altering any other behaviour.

The fix is minimal and targeted: one method added to an existing chain, covered by a new test. The error message still exposes the original user input, which is correct. The only gap is a missing symmetric test for ' follow ', but the production code handles it correctly.

No files require special attention. social-follow.test.ts could benefit from a paired ' follow ' assertion, but this is a test-coverage gap rather than a defect.

Important Files Changed

Filename Overview
packages/cli/src/social-follow.ts Added .trim() before .toLowerCase() in normalizeFollowAction to strip surrounding whitespace from user-supplied --action values; no other logic changed.
packages/cli/src/social-follow.test.ts Adds regression assertion for ' unfollow ' (padded with spaces) returning 'unfollow'; existing cases are preserved.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["CLI: --action value"] --> B{"unfollow flag set?"}
    B -- yes --> C["return 'unfollow'"]
    B -- no --> D["action ?? 'follow'"]
    D --> E[".trim()"]
    E --> F[".toLowerCase()"]
    F --> G{"=== 'follow' or 'unfollow'?"}
    G -- yes --> H["return normalized value"]
    G -- no --> I["throw Error (shows original action)"]
Loading

Reviews (1): Last reviewed commit: "Trim social follow action values" | Re-trigger Greptile

Comment on lines +32 to 33
expect(normalizeFollowAction(' unfollow ')).toBe('unfollow');
expect(normalizeFollowAction('follow', true)).toBe('unfollow');

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.

P2 The new assertion covers ' unfollow ' (trimmed 'unfollow'), but the symmetric case ' follow ' (trimmed 'follow') is not tested. Since the trim now applies to both values equally, a paired assertion would make the coverage symmetric and guard against future regressions on the follow side.

Suggested change
expect(normalizeFollowAction(' unfollow ')).toBe('unfollow');
expect(normalizeFollowAction('follow', true)).toBe('unfollow');
expect(normalizeFollowAction(' unfollow ')).toBe('unfollow');
expect(normalizeFollowAction(' follow ')).toBe('follow');
expect(normalizeFollowAction('follow', true)).toBe('unfollow');

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!

@rissrice2105-agent

Copy link
Copy Markdown
Contributor Author

CI is green for PR #737.

Verification:

  • corepack pnpm vitest run packages/cli/src/social-follow.test.ts
  • corepack pnpm --filter @profullstack/sh1pt typecheck was attempted and still fails on existing workspace/module-resolution errors unrelated to this PR.

uGig invoice evidence has been sent for this PR.

@github-actions

Copy link
Copy Markdown

🤖 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: git fetch upstream master && git rebase upstream/master.

5 similar comments
@github-actions

Copy link
Copy Markdown

🤖 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: git fetch upstream master && git rebase upstream/master.

@github-actions

Copy link
Copy Markdown

🤖 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: git fetch upstream master && git rebase upstream/master.

@github-actions

Copy link
Copy Markdown

🤖 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: git fetch upstream master && git rebase upstream/master.

@github-actions

Copy link
Copy Markdown

🤖 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: git fetch upstream master && git rebase upstream/master.

@github-actions

Copy link
Copy Markdown

🤖 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: git fetch upstream master && git rebase upstream/master.

@ralyodio ralyodio merged commit c794db7 into profullstack:master Jun 14, 2026
5 checks passed
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.

social follow rejects action values with surrounding whitespace

2 participants