feat: add channel purpose lines - #119
Conversation
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
Codex review: found issues before merge. Reviewed August 2, 2026, 3:13 AM ET / 07:13 UTC. ClawSweeper reviewWhat this changesAdds an optional channel purpose field across SQLite/PostgreSQL storage, HTTP/OpenAPI/TypeScript SDK contracts, and the main and embedded channel headers. Merge readinessThe PR has convincing real-app proof for its proposed UI, but it is not mergeable as written: its SQLite and PostgreSQL migration filenames collide with current main’s display-title migrations, and a maintainer must decide whether a distinct persisted purpose belongs beside Priority: P3 Review scores
Verification
How this fits togetherClickClack channel creation and updates pass through the HTTP API into SQLite or PostgreSQL, then return channel metadata to the web application and embedded channel view. This change adds optional channel metadata that the headers render after the existing update/refetch flow. flowchart LR
A[Authorized channel update] --> B[HTTP channel API]
B --> C[Purpose validation]
C --> D[SQLite or PostgreSQL]
D --> E[Channel response and update event]
E --> F[Main channel header]
E --> G[Embedded channel header]
Decision needed
Why: Both fields are optional metadata shown in the same public channel-header path, but current main’s display-title behavior supplies an overlapping newer contract. Source evidence cannot determine the intended long-term product model. Before merge
Findings
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Copy recommended automerge instructionTechnical reviewBest possible solution: Decide whether purpose is a distinct channel contract; if approved, rebase onto current main, assign fresh migration numbers in both stores, preserve Do we have a high-confidence way to reproduce the issue? Not applicable as a bug: the prepared screenshots and E2E notes demonstrate the requested feature in a real local setup, while current main does not offer a channel purpose line. Is this the best way to solve the issue? No: the feature path is coherent in isolation, but this branch is unsafe on current main and the relationship to Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against b8293987baa5. LabelsLabel justifications:
EvidenceWhat I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (28 earlier review cycles; latest 8 shown)
|
40ff39c to
3deb9e9
Compare
|
@clawsweeper re-review Proof and validation have been refreshed for exact head |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
Closing after maintainer review because this branch conflicts with the current channel display-title migration and omits the display-title behavior now on If a separate purpose-name concept is approved, it should be implemented from a fresh branch with a deliberately distinct contract. That product decision remains tracked in #118. Thanks for the contribution and the useful exploration. |
Closes #118
What Problem This Solves
Channels currently show only their name and structural metadata, so people entering operational, project, incident, or agent-created rooms must search the timeline to understand what the room is for.
Why This Change Was Made
Adds one optional, plain-text channel purpose across SQLite and PostgreSQL storage, HTTP/OpenAPI/TypeScript SDK contracts, and the main and embedded channel headers. Values are trimmed, limited to 280 Unicode code points, reject control characters and line breaks, render as text, and reuse the existing channel create/update authorization and
channel.updatedrefetch path.User Impact
Visible channel members can read a compact Purpose line immediately, expand long values, and see updates or clearing in open main-app and embed views without reloading. Existing channels and older clients remain compatible because the field is additive and optional.
Evidence
Exact reviewed head:
3deb9e98d3a6ca51b3f145e219ec886b70f784a1, rebased ontomainat7fd4cd1d01afe230b01eea4948e64eeecf0d5216.umask 022; pnpm check— passed.pnpm generate:sqlcandpnpm build— passed; generated database and embedded web output are current after the rebase.pnpm exec playwright test tests/e2e/channel-purpose.spec.ts --workers=1— 1 passed.git diff --check origin/main...HEAD— passed.Inspectable real-app proof
The proof uses the production web bundle, the real Go API, a fresh SQLite database, and explicit local-development authentication. It creates a channel with an initial purpose, opens the main and embedded views together, updates the value through the real API, verifies both open views refresh through
channel.updatedwithout reload, then clears it and verifies both surfaces remove it in realtime. Generated identifiers are used; no hosted token, private endpoint, or user data is included.AI assistance: implementation, tests, and review were AI-assisted; the final diff was independently reviewed and all findings were resolved.