Repository navigation
Feature/clipboard source tracking - #3
Conversation
- Add ClipboardSource enum (ui/sdk/mcp) - Add source field to ClipboardEntry model - Hardcode source as 'sdk' in sync/async set/push methods
- Add DEFAULT_CLIPBOARD_SOURCE constant for backward compatibility - Make ClipboardEntry.source Optional to handle older API responses - Export constant from __init__.py Addresses code review feedback about breaking existing call sites.
- Add Clipboard Operations section to README - Document set, get, push, pop, list, and delete operations - Add ClipboardEntry structure with source field - Note that source is auto-set to ClipboardSource.SDK when using this SDK - Add Clipboard/Memory to Features list
- Use 'is not None' check for ttl_seconds instead of truthiness - Add explicit validation that ttl_seconds > 0 when provided - Fix clear_all to only increment counter when delete() returns True - Applied to both sync and async service classes (4 locations each)
Reviewer's GuideAdds clipboard source tracking to the SDK’s clipboard operations and exposes it in the public types, while improving TTL validation and making clear_all only count successful deletions. Also documents clipboard usage and the new source behavior in the README. Sequence diagram for clipboard set operation with source tracking and TTL validationsequenceDiagram
actor Caller
participant ClipboardClient
participant HttpClient
participant ClipboardAPI
Caller->>ClipboardClient: set(name, value, content_type, encoding, visibility, created_by_tool, created_by_model, ttl_seconds)
alt ttl_seconds is not None and ttl_seconds <= 0
ClipboardClient-->>Caller: raise ValueError
else valid ttl_seconds or None
ClipboardClient->>ClipboardClient: build payload
ClipboardClient->>ClipboardClient: set source = sdk
opt ttl_seconds is not None
ClipboardClient->>ClipboardClient: add ttlSeconds to payload
end
ClipboardClient->>HttpClient: request(POST, /api/clipboard, json=payload)
HttpClient->>ClipboardAPI: POST /api/clipboard
ClipboardAPI-->>HttpClient: 200 ClipboardEntry JSON
HttpClient-->>ClipboardClient: ClipboardEntry JSON
ClipboardClient-->>Caller: ClipboardEntry(source=sdk)
end
Sequence diagram for clear_all counting only successful clipboard deletionssequenceDiagram
actor Caller
participant ClipboardClient
participant HttpClient
participant ClipboardAPI
Caller->>ClipboardClient: clear_all()
ClipboardClient->>HttpClient: request(GET, /api/clipboard)
HttpClient->>ClipboardAPI: GET /api/clipboard
ClipboardAPI-->>HttpClient: list ClipboardEntry JSON
HttpClient-->>ClipboardClient: entries
ClipboardClient->>ClipboardClient: deleted = 0
loop for each entry in entries
alt entry has name
ClipboardClient->>ClipboardClient: success = delete(name)
else entry has idx
ClipboardClient->>ClipboardClient: success = delete(idx)
else no name or idx
ClipboardClient->>ClipboardClient: success = False
end
alt success is True
ClipboardClient->>ClipboardClient: deleted += 1
else success is False
ClipboardClient->>ClipboardClient: skip increment
end
opt delete raises PluggedInError
ClipboardClient->>ClipboardClient: ignore error
end
end
ClipboardClient-->>Caller: deleted
Class diagram for updated clipboard types with source trackingclassDiagram
class ClipboardVisibility {
<<enumeration>>
PRIVATE
WORKSPACE
PUBLIC
}
class ClipboardSource {
<<enumeration>>
UI
SDK
MCP
}
class ClipboardEntry {
+str uuid
+str name
+int idx
+str value
+str content_type
+ClipboardEncoding encoding
+int size_bytes
+ClipboardVisibility visibility
+str created_by_tool
+str created_by_model
+ClipboardSource source
+datetime created_at
+datetime updated_at
+datetime expires_at
}
class ClipboardEncoding {
<<enumeration>>
UTF8
BASE64
HEX
}
class DEFAULT_CLIPBOARD_SOURCE {
<<constant>>
+ClipboardSource value
}
ClipboardEntry --> ClipboardVisibility : uses
ClipboardEntry --> ClipboardEncoding : uses
ClipboardEntry --> ClipboardSource : uses
DEFAULT_CLIPBOARD_SOURCE --> ClipboardSource : default_for_legacy_entries
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- In
ClipboardEntry, consider makingsourcea non-optionalClipboardSourcewith a default (or usingField(default=DEFAULT_CLIPBOARD_SOURCE)), since it now always has a meaningful default and your comment implies it should only be absent in older API responses. - In the clipboard service, you currently hardcode the string literal
"sdk"for the payloadsourcefield; using theClipboardSource.SDK.valueenum instead would avoid a magic string and keep things consistent with the new type.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `ClipboardEntry`, consider making `source` a non-optional `ClipboardSource` with a default (or using `Field(default=DEFAULT_CLIPBOARD_SOURCE)`), since it now always has a meaningful default and your comment implies it should only be absent in older API responses.
- In the clipboard service, you currently hardcode the string literal `"sdk"` for the payload `source` field; using the `ClipboardSource.SDK.value` enum instead would avoid a magic string and keep things consistent with the new type.
## Individual Comments
### Comment 1
<location> `src/pluggedinkit/services/clipboard.py:176-179` </location>
<code_context>
+ success = False
if entry.name:
- self.delete(name=entry.name)
+ success = self.delete(name=entry.name)
elif entry.idx is not None:
- self.delete(idx=entry.idx)
- deleted += 1
+ success = self.delete(idx=entry.idx)
+ if success:
+ deleted += 1
except PluggedInError:
</code_context>
<issue_to_address>
**issue (bug_risk):** clear_all now depends on delete returning a truthy value, which may change existing behavior.
`deleted` used to increment after every `delete` call unless an exception was raised. Now it only increments when `delete` returns truthy, which changes semantics if `delete` historically returned `None` or didn’t signal success. That could cause `clear_all` to underreport deletions. Consider keeping the old behavior (increment on non-exception) or update `delete` to reliably return a boolean and document the change in behavior.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| success = self.delete(name=entry.name) | ||
| elif entry.idx is not None: | ||
| self.delete(idx=entry.idx) | ||
| deleted += 1 | ||
| success = self.delete(idx=entry.idx) | ||
| if success: |
There was a problem hiding this comment.
issue (bug_risk): clear_all now depends on delete returning a truthy value, which may change existing behavior.
deleted used to increment after every delete call unless an exception was raised. Now it only increments when delete returns truthy, which changes semantics if delete historically returned None or didn’t signal success. That could cause clear_all to underreport deletions. Consider keeping the old behavior (increment on non-exception) or update delete to reliably return a boolean and document the change in behavior.
Pull Request Review: Feature/clipboard source trackingOverviewThis PR adds clipboard source tracking to distinguish between entries created via UI, SDK, or MCP. The implementation includes new enums, automatic source attribution, bug fixes, and comprehensive documentation. ✅ Strengths1. Code Quality & Best Practices
2. Bug Fixes
3. API Design
|
| Category | Rating | Notes |
|---|---|---|
| Code Quality | ⭐⭐⭐⭐ | Clean, follows patterns, good practices |
| Bug Fixes | ⭐⭐⭐⭐ | Addresses real issues correctly |
| Security | ⭐⭐⭐⭐ | No new vulnerabilities, good audit trail |
| Performance | ⭐⭐⭐⭐⭐ | No concerns |
| Documentation | ⭐⭐⭐⭐⭐ | Excellent coverage |
| Test Coverage | ⭐ | Missing - must add |
🚦 Recommendation: Request Changes
Required Before Merge:
- ✅ Fix TTL validation inconsistency (message vs logic)
- ✅ Resolve Pydantic vs manual validation redundancy
- ✅ Add test coverage (at minimum: source tracking, TTL validation, clear_all accuracy)
Suggested Improvements:
- Use
ClipboardSource.SDK.valueinstead of string literal - Handle edge case in
clear_all()for entries without name/idx - Verify backend API enforces source attribution
Once the required items are addressed, this will be a solid addition to the SDK. The feature is well-designed and the documentation is exemplary. Great work overall! 🎉
Review generated with Claude Code
- Import ClipboardSource from types - Replace hardcoded "sdk" string with ClipboardSource.SDK.value - Consistent with enum usage across the SDK
Pull Request Review: Clipboard Source TrackingOverviewThis PR introduces clipboard source tracking functionality to distinguish between entries created via UI, SDK, or MCP. The implementation includes new enums, model updates, validation improvements, and comprehensive documentation. StrengthsCode Quality
Bug Fixes
Documentation
Critical Issues1. Incorrect Default Source Value The default value is set to ClipboardSource.UI, but when SDK creates entries, they should have source=sdk. If the API does not return a source field, entries created by the SDK will incorrectly show source=ui instead of source=sdk. Recommendation: Set default to None and handle the fallback logic in a property accessor or Pydantic validator. 2. Missing Test Coverage
High Priority Issues3. TTL Validation Security Recommendation: Add reasonable upper bound (e.g., 1 year = 31,536,000 seconds) 4. Type Safety in Parameters Recommendation: Use Union[str, EnumType] in function signatures for better IDE autocomplete and type safety. Medium Priority Issues5. clear_all() Performance Recommendations:
6. Missing Async Examples 7. Code Duplication Minor Issues8. Unused Type Imports in README Recommendations PriorityCritical (Must Fix Before Merge)
High Priority Medium Priority Low Priority Final VerdictStatus: Needs changes before merge The PR demonstrates solid engineering with excellent backward compatibility handling and comprehensive documentation. However, the default source value issue could cause incorrect attribution, and the lack of tests is concerning for a feature tracking data provenance. Estimated effort: 2-3 hours
Great work on the careful refactoring across commits! |
Summary by Sourcery
Add clipboard source tracking to SDK clipboard entries and enhance clipboard operations documentation.
New Features:
Bug Fixes:
Enhancements:
Documentation: