Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 28e0b681dd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
skills/followupboss/followupboss
Outdated
| def cmd_tasks(args: list[str]) -> None: | ||
| """List tasks, optionally filtered by person.""" | ||
| params: dict[str, Any] = {"limit": DEFAULT_LIMIT} | ||
|
|
||
| if args: | ||
| # First arg could be a person ID or a limit | ||
| try: | ||
| pid = int(args[0]) | ||
| params["personId"] = pid | ||
| if len(args) > 1: | ||
| params["limit"] = validate_limit(args[1]) |
There was a problem hiding this comment.
The tasks command always treats the first argument as a person ID, so followupboss tasks 5 ends up filtering to personId=5 instead of returning the 5 most recent tasks. This makes it impossible to request a smaller limit without also providing a person ID, which contradicts the help text that frames the person filter as optional. In practice, users trying to list N tasks will silently get the wrong dataset when they pass a single number.
Useful? React with 👍 / 👎.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Standalone UV script for querying and managing Follow Up Boss real estate CRM — contacts, notes, tasks, deals, and pipeline stages. Includes 18 tests (unit + validation + integration). Tested live against Julianna's account (6,230 contacts). Deployed to her Mac Mini. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
TechNickAI
force-pushed
the
feature/followupboss-skill
branch
from
833c20f to
f92461c
Compare
- Tasks command now interprets single arg ≤100 as limit, not person ID Fixes: 'followupboss tasks 5' now returns 5 tasks, not person #5's tasks - cmd_raw validates GET request JSON is dict before passing to params Prevents AttributeError crash when user passes array/primitive Addresses Codex P2 and Cursor Low severity feedback. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Code reviewFound one issue to address: VERSION file needs to be bumpedThe VERSION file should be incremented when adding a new skill. Per CLAUDE.md line 27:
Established pattern: Previous PRs that added skills have incremented VERSION: Suggested fix: Update VERSION from Otherwise, the code looks good! The Follow Up Boss skill is well-structured, follows the UV script pattern correctly, includes comprehensive tests with proper API key handling, and has no bugs or security issues. |
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
Addresses Cursor and Codex bot feedback: 1. Fix null name rendering - Use `or ""` pattern to handle API null values that would render as literal "None" string 2. Fix tasks command ambiguity - Remove heuristic that prevented querying person IDs 1-100, now matches documented signature 3. cmd_raw GET validation already present - No changes needed Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| output.append(f"**Stage:** {stage}") | ||
| if source: | ||
| output.append(f"**Source:** {source}") | ||
| output.append(f"**Assigned to:** {assigned}") |
There was a problem hiding this comment.
Null assignedTo renders as "None" instead of default
Low Severity
When the API returns "assignedTo": null, person.get("assignedTo", "Unassigned") evaluates to None (key exists, so default isn't used), and the output displays literal "Assigned to: None" instead of "Assigned to: Unassigned". This is the same null-key-exists issue documented in the comment on line 152 — the fix used for firstName/lastName (or "") wasn't applied here. Same issue in format_person and format_tasks.
Additional Locations (1)
1 participant
Summary
followupbossskill — standalone UV script for querying/managing Follow Up Boss real estate CRMTest plan
uv run --with pytest pytest tests/test_followupboss.py -v)FUB_API_KEY🤖 Generated with Claude Code