feat(community): add collaborator community view (stats + leaderboard + kanban)#2042
feat(community): add collaborator community view (stats + leaderboard + kanban)#2042hognek wants to merge 5 commits into
Conversation
…ard, and live kanban
- Widen member_kind validation in project_store to include 'human'
- Add community projection endpoints (snapshot + stats/leaderboard)
at GET /api/projects/{id}/community/{snapshot,stats}
- Build CommunityView React component with overview stats, leaderboard
table, read-only kanban columns, and activity feed
- Add 'community' tab to ProjectWorkspace
- Add community API types and fetch functions to projects.ts
- Add unit tests for leaderboard rollup function
Refs: jaylfc#2021 (Collab F, Milestone F — community view)
📝 WalkthroughWalkthroughAdds read-only community snapshot and stats APIs, leaderboard aggregation, human member handling, and a desktop Community tab with stats, leaderboard, kanban, and activity views. ChangesCommunity View
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CommunityView
participant projectsApi
participant community_snapshot
participant ProjectStore
CommunityView->>projectsApi: request community.snapshot(projectId)
projectsApi->>community_snapshot: GET snapshot endpoint
community_snapshot->>ProjectStore: read project, tasks, and audit events
ProjectStore-->>community_snapshot: project data and events
community_snapshot-->>projectsApi: sanitized snapshot payload
projectsApi-->>CommunityView: render CommunitySnapshot
Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
|
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| return {k: v for k, v in task.items() if k in _TASK_ALLOWLIST} | ||
|
|
||
|
|
||
| def _parse_labels(task: dict) -> list[str]: |
There was a problem hiding this comment.
SUGGESTION: Dead code — _parse_labels is defined but never called anywhere in the PR.
The task store already parses labels via json.loads in _row_to_task, so task_store.list_tasks returns lists, and the projection passes them through untouched. This function is unreachable. Either remove it or actually use it to normalize labels (e.g. when the store returns a raw string fallback). Right now it's misleading dead code that implies labels are handled here.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| - project metadata (name, slug, description, status) | ||
| - board tasks (id, title, status, claimant, labels, timestamps) | ||
| - contribution stats (per-contributor claim/close counts, leaderboard) | ||
| - recent activity feed (last 50 board-audit events) |
There was a problem hiding this comment.
SUGGESTION: Docstring says "last 50 board-audit events" but the code fetches limit=500 (line 109) and the activity feed is sliced to the first 20 (audit_events[:20], line 121).
This is a documentation/implementation mismatch. The feed is newest-first (board_audit returns ORDER BY rowid DESC) and capped at 20, not 50. Update the docstring (and the recent_activity count semantics) to match the actual behavior so future maintainers aren't misled.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| raise ValueError(f"invalid member_kind: {member_kind}") | ||
| if member_kind == "human": | ||
| # Human members are remote collaborators — no agent lifecycle fields. | ||
| memory_seed = "none" |
There was a problem hiding this comment.
SUGGESTION: add_member silently overrides caller-supplied memory_seed and source_agent_id to "none"/None for human members.
This is reasonable for the human-collaborator model, but it silently discards any non-default values a caller passes. If a future caller passes a populated source_agent_id expecting it to be stored, it will be dropped with no error. Consider either documenting this override in the function docstring, or asserting the inputs are None/"none" when member_kind=="human" so the silent discard is at least visible in tests.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review SummaryStatus: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Previously-flagged issues — now RESOLVED in this diff
Files Reviewed (full PR — 11 files)
Fix these issues in Kilo Cloud Previous Review Summaries (3 snapshots, latest commit 2f74a30)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 2f74a30)Status: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)CRITICAL
Previously-flagged issues — now RESOLVED in this incremental diff
Files Reviewed (incremental: 1 file)
Fix these issues in Kilo Cloud Previous review (commit d32de75)Status: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)CRITICAL
Previously-flagged issues — now RESOLVED in this incremental diff
Still-open pre-existing issue (not changed by this diff, active CodeRabbit comment)
Files Reviewed (incremental: 3 files)
Fix these issues in Kilo Cloud Previous review (commit eac6503)Status: 3 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)SUGGESTION
Files Reviewed (8 files)
Reviewed by hy3:free · Input: 89.4K · Output: 5.7K · Cached: 752.2K |
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
- Remove dead _parse_labels function (task store already parses labels) - Fix docstring: actual fetch is 500 events, feed truncated to 20 - Add logger.warning when overriding memory_seed/source_agent_id for human members in add_member
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
desktop/src/apps/ProjectsApp/ProjectWorkspace.tsx (1)
105-112: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCommunity tab isn't reachable on mobile.
mobileTabOrder(which drivestabPillsfor the mobile pill nav) wasn't updated to include"community", unlike the desktopTABSarray. Mobile users have no in-app control to reach the new tab (only a deep link would work).🐛 Proposed fix
- const mobileTabOrder: Tab[] = ["workspace", "messages", "board", "tasks", "canvas", "files", "members", "activity", "decisions", "routines"]; + const mobileTabOrder: Tab[] = ["workspace", "messages", "board", "tasks", "canvas", "files", "members", "activity", "decisions", "routines", "community"];🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@desktop/src/apps/ProjectsApp/ProjectWorkspace.tsx` around lines 105 - 112, Update the mobileTabOrder array in ProjectWorkspace so it includes the "community" tab, matching the available desktop TABS entries and ensuring it is exposed through the existing tabPills mobile navigation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@desktop/src/apps/ProjectsApp/CommunityView.tsx`:
- Around line 157-166: Update the columns construction in CommunityView to
retain the existing statusOrder columns while appending any task statuses not
included in that list. Derive unknown statuses from tasksByStatus, ensure each
appears only once, and preserve the current filtering of empty status groups so
every task remains visible on the board.
In `@tests/test_community.py`:
- Around line 144-150: The leaderboard test conditionally skips all checks when
body["leaderboard"] is empty. In the leaderboard assertions near test_community,
assert that body["leaderboard"] is non-empty, then assign its first entry and
run the existing actor, claims, closes, and total assertions unconditionally,
matching test_community_snapshot_with_tasks.
In `@tinyagentos/routes/community.py`:
- Around line 84-88: Replace the nonexistent project_store.get call with
project_store.get_project in the affected community route, and apply the same
method-name correction in community_stats. Preserve the existing 404 and
authorization checks.
- Line 77: Update the community endpoint docstring to state that the recent
activity feed returns the last 20 board-audit events, matching the
audit_events[:20] limit and its inline comment. Apply the same correction to the
corresponding documentation around the additional referenced section.
---
Outside diff comments:
In `@desktop/src/apps/ProjectsApp/ProjectWorkspace.tsx`:
- Around line 105-112: Update the mobileTabOrder array in ProjectWorkspace so it
includes the "community" tab, matching the available desktop TABS entries and
ensuring it is exposed through the existing tabPills mobile navigation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 5eca8580-4fb3-46dc-9b04-c5766ba1bd35
📒 Files selected for processing (8)
desktop/src/apps/ProjectsApp/CommunityView.tsxdesktop/src/apps/ProjectsApp/ProjectWorkspace.tsxdesktop/src/apps/ProjectsApp/ProjectsApp.module.cssdesktop/src/lib/projects.tstests/test_community.pytinyagentos/projects/project_store.pytinyagentos/routes/__init__.pytinyagentos/routes/community.py
| - project metadata (name, slug, description, status) | ||
| - board tasks (id, title, status, claimant, labels, timestamps) | ||
| - contribution stats (per-contributor claim/close counts, leaderboard) | ||
| - recent activity feed (last 50 board-audit events) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Docstring says "last 50", code slices to 20.
The endpoint docstring claims the recent activity feed returns the "last 50 board-audit events", but the implementation caps it at audit_events[:20] (matching the inline comment "last 20 events"). Align the docstring with actual behavior.
Also applies to: 112-122
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tinyagentos/routes/community.py` at line 77, Update the community endpoint
docstring to state that the recent activity feed returns the last 20 board-audit
events, matching the audit_events[:20] limit and its inline comment. Apply the
same correction to the corresponding documentation around the additional
referenced section.
… access The community route referenced request.app.state.task_store, but the lifespan registers the attribute as project_task_store (matching the ProjectTaskStore class name). This caused 5/8 community tests to fail with KeyError/AttributeError on CI. Also fixes the doc-gate by documenting why the structural changes (adding desktop/src/apps/ProjectsApp/CommunityView.tsx and tinyagentos/routes/community.py) don't need README.md or docs/agent-coordination.md updates: the Community View is a new feature scoped under Projects, not a standalone desktop app or a cross-cutting agent-coordination API. Docs-Reviewed: Community View is a Projects sub-feature, not a standalone app; route is an internal projection endpoint scoped to project boards, not part of the agent-coordination surface Refs: jaylfc#2042
- Remove dead _parse_labels function (task store already parses labels) - Fix docstring: actual fetch is 500 events, feed truncated to 20 - Add logger.warning when overriding memory_seed/source_agent_id for human members in add_member
… access The community route referenced request.app.state.task_store, but the lifespan registers the attribute as project_task_store (matching the ProjectTaskStore class name). This caused 5/8 community tests to fail with KeyError/AttributeError on CI. Also fixes the doc-gate by documenting why the structural changes (adding desktop/src/apps/ProjectsApp/CommunityView.tsx and tinyagentos/routes/community.py) don't need README.md or docs/agent-coordination.md updates: the Community View is a new feature scoped under Projects, not a standalone desktop app or a cross-cutting agent-coordination API. Docs-Reviewed: Community View is a Projects sub-feature, not a standalone app; route is an internal projection endpoint scoped to project boards, not part of the agent-coordination surface Refs: jaylfc#2042
| @@ -0,0 +1 @@ | |||
| {"signing_private": "1cf1ac0cbeea7bbd6a68bf38abfaa683800d4dea498079c83d154b427d56ea14", "signing_public": "0012434ac9e5bc51c7ca8d0e7de5ad9c59faf2a6afa92dec00b935dfd2e85377", "encryption_private": "40a212e227c8897dadf6548bf542d0be09031f595e26b3a6fce5ecdda02e1953", "encryption_public": "91a91b4d7ac1ba8ad7d9fc1a4bd56beea659d60298f93da0ca0f1e3d376f3947", "created_at": 1784460139.0949676} No newline at end of file | |||
There was a problem hiding this comment.
CRITICAL: Private cryptographic keys committed to the repository.
This new file contains signing_private and encryption_private — the secret/private halves of the hub identity keypair. Committing private keys to git history exposes them permanently (even if later removed, they remain in history). Anyone with repo access can impersonate the hub's signing/encryption identity.
Recommended fix:
- Remove this file from the commit and add it to
.gitignore. - Load
identity.jsonfrom a local/secrets path at runtime (e.g. env-var or mounted secret), never from the repo. - If these keys were already pushed, rotate them — treat the private material as compromised and generate a new identity.
- Scrub the key from history (e.g.
git filter-repo) before merge if it has reached a shared remote.
Note: data/hub/identity.json is currently NOT gitignored, so it will be tracked.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
tinyagentos/routes/community.py (1)
71-71: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winFix
project_store.getmethod calls toget_project.
ProjectStoreonly exposes theget_projectmethod. Calling.get()will result in anAttributeErroron every request.
tinyagentos/routes/community.py#L71-L71: Changeawait project_store.get(project_id)toawait project_store.get_project(project_id).tinyagentos/routes/community.py#L135-L135: Changeawait project_store.get(project_id)toawait project_store.get_project(project_id).🐛 Proposed fixes
For
community_snapshot(Line 71):- project = await project_store.get(project_id) + project = await project_store.get_project(project_id)For
community_stats(Line 135):- project = await project_store.get(project_id) + project = await project_store.get_project(project_id)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tinyagentos/routes/community.py` at line 71, Replace the invalid project_store.get calls with project_store.get_project in both community_snapshot (tinyagentos/routes/community.py:71-71) and community_stats (tinyagentos/routes/community.py:135-135), preserving the existing await and project_id argument.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@data/hub/identity.json`:
- Line 1: Remove data/hub/identity.json from version control and add its path or
parent directory to .gitignore so generated identity state is not committed.
Treat the exposed signing_private and encryption_private values as compromised
and rotate them wherever they may have been used; only retain replacement dummy
keys in explicitly documented test fixtures if testing requires them.
---
Duplicate comments:
In `@tinyagentos/routes/community.py`:
- Line 71: Replace the invalid project_store.get calls with
project_store.get_project in both community_snapshot
(tinyagentos/routes/community.py:71-71) and community_stats
(tinyagentos/routes/community.py:135-135), preserving the existing await and
project_id argument.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 63fffac0-f078-4934-96a5-c43cc5555614
📒 Files selected for processing (3)
data/hub/identity.jsontinyagentos/projects/project_store.pytinyagentos/routes/community.py
🚧 Files skipped from review as they are similar to previous changes (1)
- tinyagentos/projects/project_store.py
| @@ -0,0 +1 @@ | |||
| {"signing_private": "1cf1ac0cbeea7bbd6a68bf38abfaa683800d4dea498079c83d154b427d56ea14", "signing_public": "0012434ac9e5bc51c7ca8d0e7de5ad9c59faf2a6afa92dec00b935dfd2e85377", "encryption_private": "40a212e227c8897dadf6548bf542d0be09031f595e26b3a6fce5ecdda02e1953", "encryption_public": "91a91b4d7ac1ba8ad7d9fc1a4bd56beea659d60298f93da0ca0f1e3d376f3947", "created_at": 1784460139.0949676} No newline at end of file | |||
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win
Remove hardcoded private keys from source control.
This file contains sensitive cryptographic keys (signing_private, encryption_private). Committing private keys to the repository is a critical security vulnerability.
If this is a dynamically generated local state file, please remove it from version control and add its path (data/hub/identity.json) or parent directory to .gitignore. If these are intended to be mock keys for testing, they should be relocated to a dedicated test fixtures directory and explicitly documented as non-production dummy keys. Note that any compromised keys that were ever used in a production or publicly accessible context must be considered burnt and immediately rotated.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@data/hub/identity.json` at line 1, Remove data/hub/identity.json from version
control and add its path or parent directory to .gitignore so generated identity
state is not committed. Treat the exposed signing_private and encryption_private
values as compromised and rotate them wherever they may have been used; only
retain replacement dummy keys in explicitly documented test fixtures if testing
requires them.
ProjectStore exposes get_project, not get. Both community_snapshot and community_stats were calling project_store.get(project_id) which would raise AttributeError at runtime. This was masked in CI by the earlier task_store KeyError. CodeRabbit CRITICAL finding fixed. Docs-Reviewed: method-name correction only, no new structural changes Refs: jaylfc#2042
ProjectStore exposes get_project, not get. Both community_snapshot and community_stats were calling project_store.get(project_id) which would raise AttributeError at runtime. This was masked in CI by the earlier task_store KeyError. CodeRabbit CRITICAL finding fixed. Docs-Reviewed: method-name correction only, no new structural changes Refs: jaylfc#2042
|
Deep review at head 2f74a30. CI is genuinely green and the earlier CodeRabbit critical (project_store.get AttributeError) is fixed, but this is a HOLD. Numbered folds, 1/5/6/7 are hard blockers:
Priority order: 1 first (key hygiene, branch rewrite), then 5/6/7, then the deferral notes (2/3) and 4/8/9/10. |
…gerprint jaylfc deep review at 4b5903b — fold all six findings: 1. BLOCKER: peer_fingerprint retrofit migration was a no-op on every pre-existing DB. BaseStore's migration runner uses baseline-at-latest semantics — existing DBs get stamped at version 1 without executing the ALTER, so the column was absent after init(). The broad except in _try_handshake swallowed the resulting OperationalError, and the block-cascade security fix was similarly swallowed. Replaced the MIGRATIONS list with a guarded _post_init that checks PRAGMA table_info('contacts') and ALTER TABLE ADD COLUMN only when peer_fingerprint is absent. Same pattern as agent_registry_store's _migration_v1_add_status. Fresh databases still get the column from SCHEMA; upgraded databases get it from _post_init. Added two ContactsStore upgrade tests in test_store_upgrades.py following the existing pattern — column-presence check and add_contact-after-upgrade. 2. Fold 1 (send_handshake): A2 intentionally stores the inbound token locally without delivering it — the token exchange channel doesn't exist yet. A3 completes the two-way exchange. The send_handshake envelope builder from jaylfc#2046 is deferred to a follow-up PR linked from the tracking issue. This is a spec deviation from cross-user-collaboration.md Day 0 (mint token on BOTH sides), filed as a tracking issue. 3. Fold 3 (.gitignore): the data/hub/ ignore line is justified — this branch's own history committed identity.json with throwaway test keys at 2b28043 (removed at 500da60). The .gitignore prevents future accidental commits. Squash merge will keep dev history clean. 4. Key hygiene: the keys in 2b28043 were throwaway test keys never used against real endpoints. Squash merge removes them from dev history. jaylfc#2042 re-commits the same file; coordination note added in-thread. 5. Re-trigger: @coderabbitai review after push. 6. Track-don't-block (Kilo W1): accepted the documented NOTE about contact_id bound to peer-controllable username. Follow-up issue filed for fingerprint-keyed contact IDs in a future slice. BONUS: Fixed CodeRabbit nit from head review — test_accept_reupsert_contact now actually revokes between accepts to verify re-establishment clears revoked_at (was a no-op assertion before).
CRITICAL fixes: - Scrub data/hub/identity.json from git history (private keys exposed) - Add data/hub/ to .gitignore so runtime identity never lands again Hard blockers: - Auth pattern: bare 403 → masked 404 with admin bypass (community.py now uses + 404, matching projects.py) - Cross-user ownership tests: add two_member_clients fixture + snapshot + stats cross-user 404 tests (masked, no existence leak) - Fix conditional assertion in test_community_stats: remove vacuous guard, assert directly - mobileTabOrder: add 'community' so tab is reachable on mobile Quality: - StatusBadge: replace nonexistent --pico-color-* CSS vars with --board-status-ready/claimed/closed + --color-traffic-close (the actual taOS design tokens) - Kanban: include unknown statuses as extra columns so no task silently vanishes from board (was: only hardcoded 5) - Live polling: CommunityView now polls /community/stats every 30 s for live leaderboard + status_counts update - vitest: add 4 CommunityView component tests (loading, error, empty, populated render) Deferred (per review findings 2/3): - Community chat: scoped out of Milestone F; will land in a follow-up slice. Do not close jaylfc#2021 on merge. - Peer-serving: route is owner-only for v1; tracked as follow-up. Collaborator auth will be wired once milestones A/B fully land. Python tests: 10/10 pass. vitest: 4/4 pass. SPA build: clean.
2f74a30 to
f71bbc7
Compare
| project = await project_store.get_project(project_id) | ||
| if project is None: | ||
| raise HTTPException(status_code=404, detail="project not found") | ||
| if not user.is_admin and project.get("user_id") != user.user_id: |
There was a problem hiding this comment.
WARNING: Admin bypass grants any admin read access to every user's community projection.
if not user.is_admin and project.get("user_id") != user.user_id lets any admin read the full task board, contributor leaderboard, and activity feed for projects they don't own. This mirrors the existing projects.py owner/admin pattern, so it's consistent with the app's model — but the Community View is explicitly scoped as a privacy-sensitive projection meant for the owner plus delegated peer-token contacts. Every admin now silently gets cross-user read access to this data with no audit/visibility.
Consider whether admin access is actually required here. If it is only needed for support/debugging, gate it behind an explicit capability or at least log the cross-user access (as add_member already does for human overrides). If it's not needed, drop the not user.is_admin clause so the view is owner-scoped like the feature description implies.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
|
Checking in on this one. The fold list has been open for about 24 hours with no response, and item 1 is a security finding rather than a style point, so I do not want it to drift. The blocking item remains: The rest of the list stands: owner-only routes mean the actual audience (a remote collaborator) cannot reach the view, the bare 403 leaks project existence where the house pattern uses an admin bypass and a masked 404, and Flagging priority honestly: Jay asked today where the contributor project view stands, and the answer is that it is this PR. So it is wanted, not parked. If the branch-history rewrite is the awkward part, say so and we can talk about landing it as a fresh branch instead, which may genuinely be less work than rewriting history. Also relevant to you: taosmd's maintainer documented their response envelopes and built a |
|
Separate from the fold list: you have a pending collaborator invite on The problem it solves is the one that cost this PR two rounds. taOSmd's contract questions were answerable in one message, but you had no way to ask its maintainer, so you inferred from what looked reasonable and I only caught it in review. That is a process failure, not a you failure. How it will work:
This is deliberately temporary scaffolding. It goes away when an external contributor can hold a taOS identity and reach the bus directly, which is a feature Jay wants to build with you as the first real test. Documented in the taOS development skill so it is not just this comment. |
2 participants
Summary
Implements Milestone F (Collaborator Community View) from #2021.
Deferred (per review)
Verification
npm run buildpasses cleanRefs: #2021, #2012