Keep the personal half of the desktop in one place, and ask before installing it
Agent instructions, skills, SSH host aliases and expansion triggers are worth having identical on every machine one person owns, and belong in none of the shared configuration. They live in user/ now, with a manifest saying where each piece goes and a link-user stage that puts it there. That stage does nothing unless the machine said yes. Somebody who clones Panama to try the desktop keeps their own ~/.claude/CLAUDE.md exactly where it was; the question names the destinations and defaults to no. Anything displaced goes to config/old rather than being deleted. ~/.claude/CLAUDE.md and ~/.codex/AGENTS.md were byte-identical copies of one file, which is the drift this exists to prevent. Also adds the vitals toggles for the battery and Claude usage readouts, which had preferences and no way to reach them.
This commit is contained in:
Gabriel Brown
@@ -0,0 +1,99 @@
|
||||
---
|
||||
name: review-ticket
|
||||
description: Write the developer review for a Jira story, the implementation layer of the Dev Review SOP. Use when Gib invokes /review-ticket KACP-XXXXX on a story that needs its Developer Review Instructions, estimate, risk level, dependencies, and testing tables filled in. Produces a code-verified draft for review first, applies to Jira only after approval.
|
||||
---
|
||||
|
||||
# Review Ticket Workflow
|
||||
|
||||
Performs the per story portion of the team's Dev Review SOP (see `references/sop.md`): fills the Developer Review Instructions field with implementation guidance, sets the Original Estimate and Story Risk Level, links dependencies, fills the Risk Mitigation and Test Cases tables, and proposes automated test subtasks where a mitigation needs one. It is the third sibling of `ticket` (work a ticket) and `create-ticket` (write the description): this one writes the developer review.
|
||||
|
||||
Invoked as `/review-ticket KACP-XXXXX`. Two phases with a hard stop between them: draft, then apply. Nothing is written to Jira in the draft phase.
|
||||
|
||||
This is a *dev review*, not a code review. It is part of writing a ticket, before any
|
||||
code exists: it fills in implementation guidance, an estimate, a risk level, and the
|
||||
testing tables so a developer can pick the story up. Reviewing code that has already
|
||||
been written is a separate thing entirely, and lives in the `ticket` skill at Phase 2
|
||||
step 5.
|
||||
|
||||
## Files this skill uses
|
||||
|
||||
- `templates/dev-review-template.md`, the team's Developer Instructions Template. The section names come from here.
|
||||
- `references/example.md`, a real reviewed story rendered to markdown. This is the calibration target for structure, depth, and voice. Its file paths belong to a different repo, only its anatomy is the standard.
|
||||
- `references/estimation.md`, the estimation method and the anchor table calibrated from real Command Center estimates. Follow it exactly.
|
||||
- `references/sop.md`, the SOP duties this skill automates.
|
||||
- `scripts/review2adf.py`, markdown to ADF: `render <review.md>` produces the field document, `tables <current-field.json> <rows.md>` appends rows to an existing testing table while preserving its instruction panel and headers.
|
||||
- Jira fetch reuses `~/.agents/skills/ticket/scripts/jira-fetch-issue.sh`. Same credential rule as the ticket skill: `source ~/.bashrc 2>/dev/null` in the same Bash call, then check `JIRA_CREDENTIALS` is set, and stop to ask rather than guessing if it is not.
|
||||
|
||||
## House rules
|
||||
|
||||
- No em dashes, en dashes, semicolons, or arrow glyphs anywhere in `review.md`. Plain punctuation only. Sweep before finishing.
|
||||
- The voice is the Lead writing to the implementing developer. Imperative, concrete, calm. Guidance names the seams that already exist and says do not where a tempting wrong turn exists.
|
||||
- The redundancy rule from the SOP: the review adds context and information not already in the story. Reference acceptance criteria by their IDs (AC-3, AC-7) when pointing at them, never restate their content. The review sits one layer below the story: the story says what and why, the review says where and how.
|
||||
- Every claim about current code must be verified in the actual repo the story targets. Every file path in the review must either exist (verify with a mechanical check, not memory) or be explicitly marked as new. A review that names a file that does not exist burns the Lead's trust and is worse than no review.
|
||||
- Do not reuse another story's Current State bullets without re verifying them. Twenty similar stories reviewed by one skill invites copy paste sameness, and each story deserves fresh eyes on the code.
|
||||
- This skill never closes the epic's Dev Review ticket (a human does that when the whole epic is reviewed), never creates LaunchDarkly flags (no dashboard access, assess and report instead), and never sets an Original Estimate above 10 hours under any circumstances.
|
||||
|
||||
## Phase 0, where does this story stand
|
||||
|
||||
1. Validate the key looks like `[A-Z]+-[0-9]+`.
|
||||
2. Locate the story's docs directory: `find .claude/docs/epics -mindepth 2 -maxdepth 2 -type d -name "<KEY>"`. If the story has no directory yet, create `.claude/docs/epics/<EPIC>/<KEY>/` after fetching (the epic comes from `fields.parent.key`).
|
||||
3. If `<dir>/review.md` already exists, ask the user: apply it to Jira as approved, redraft from scratch, or revise specific sections. Otherwise enter the draft phase.
|
||||
|
||||
## Phase 1, draft
|
||||
|
||||
1. Fetch the story fresh. If a local `story.md` exists, render it with the epic's md2adf tooling if available and diff against the fetched description. The PM edits stories directly in Jira, so treat the fetched version as truth and flag any drift to the user before proceeding. Diff on extracted text and on normalized ADF (drop localId, colwidth, width, and empty attrs objects), a structure only difference with identical text is Jira editor normalization, not drift.
|
||||
2. Load context, in this order: the epic's `summary.md` if one exists (implementation context maintained for agents), `epic.md`, the spike deliverables the story references, and the mock if the story has one. The mock is the visual spec, the review should point the developer at the exact mock screens.
|
||||
3. Research the code. This is the load bearing step and is never optional:
|
||||
- Current State claims come from reading the real models, routers, components, hooks, and permission constants. Note what exists, where, and what does not exist yet.
|
||||
- Identify the established patterns the story should reuse: the closest existing grid, form, modal, router procedure, notification path, seed, migration, or flag wiring. Name them by path.
|
||||
- For datamodel stories, read the actual schema and write the intended model changes as a real Prisma block in the repo's conventions.
|
||||
- Identify cross story seams: what this story owns, what its neighbors own, where the boundary is. The epic's dependency spine matters here.
|
||||
- Where the story's shape is itself in question, how deep a module should be, where a seam belongs, or what an interface should expose, call the Skill tool with "codebase-design" and use its vocabulary. Say seam rather than boundary in the review: boundary is overloaded with bounded context and reads as a domain claim the story is not making.
|
||||
4. Write `<dir>/review.md` following the anatomy below.
|
||||
5. Write the Relationships section from the story's Requires notes, the epic's story map, and the code seams found in research, including the Not blocked by denials for dependencies a reader might wrongly assume.
|
||||
6. Estimate per `references/estimation.md`: derive from the code paths table, show the breakdown, round up to the half hour, sanity check against the anchor table. Bugs and small tasks default to 2 hours. Stories normally land in 2 to 8. Above 8, the review must argue the split and sketch it. Above 10, no estimate is proposed at all until the split is resolved.
|
||||
When the story is oversized because it is a wide refactor, one mechanical change whose blast radius fans across the codebase (renaming a column, retyping a shared symbol), the split is not a vertical slice and should not be sketched as one. Sequence it as expand, then migrate, then contract: add the new form beside the old, migrate the call sites in batches sized by blast radius with each batch its own story, then delete the old form once no caller remains. Every batch stays green because the old form still exists until the last one, which is what keeps the epic deployable throughout. Sketch the split that way and estimate the batches, not the whole.
|
||||
7. Choose the Story Risk Level by uncertainty, not size: LOW is pattern following work any dev can do, MEDIUM has some novel modeling or ambiguity, HIGH is a complex subsystem for a lead, CRITICAL is rare and means top devs collaborating. State the level and one sentence of why in the draft.
|
||||
8. List dependencies for the apply phase, derived from the Relationships section: the story's own Requires notes cross checked against the epic's story map, expressed as intended Blocks links (blocker first). Note which links already exist in Jira.
|
||||
9. Draft the testing rows: Risk Mitigation rows (Risk Summary, Risk Description, Priority, Likelihood, Mitigation Strategy, empty Mitigation Proof) and Test Cases rows (Summary, Steps, Expected Results, empty Working Feature Proof, Notes). Where a mitigation strategy is an automated test, mark it as a proposed subtask with its type, coverage, and estimate. Keep rows to the ones that matter, three to six of each, not padding.
|
||||
10. Assess the LaunchDarkly flag situation for the epic and note it in the draft's handoff section: which flag the epic needs or has, and that creation and the Releases field connection are manual steps.
|
||||
11. Run the checks:
|
||||
- Path check: extract every repo path mentioned in `review.md` and verify each exists on disk, or is marked (new). Fix or mark every miss.
|
||||
- Punctuation sweep.
|
||||
- Anatomy check against the section list below.
|
||||
- Confirm no Jira write has happened.
|
||||
12. Stop. Hand the user the draft with the estimate, risk level, dependency list, and any split recommendation surfaced in the summary, and wait for their review. Do not apply in the same run unless the user has already told you to.
|
||||
|
||||
## Phase 2, apply
|
||||
|
||||
Entered only after the user has seen the draft and said go.
|
||||
|
||||
1. Re fetch the story and re check drift. If the description changed since drafting, stop and show the diff.
|
||||
2. Render and write the field: `python3 scripts/review2adf.py render <dir>/review.md` and PUT it to `customfield_10122` via `{"fields": {"customfield_10122": <adf>}}`.
|
||||
3. Set the estimate and risk level from the approved draft: PUT `{"fields": {"timetracking": {"originalEstimate": "<Nh>"}, "customfield_10146": {"value": "<exact option string>"}}}`. The risk options are LOW, MEDIUM, HIGH, CRITICAL with their full descriptive strings, and their trailing whitespace is inconsistent (LOW and HIGH end with a space, MEDIUM does not), so always fetch editmeta and copy the exact string rather than typing it. Never set above 10h. If the draft recommended a split, do not set an estimate, tell the user the split question blocks it.
|
||||
4. Create the missing dependency links: for each intended link not already present, POST `/rest/api/3/issueLink` with type Blocks, `inwardIssue` the blocker and `outwardIssue` the blocked. This direction is empirically verified: it renders as the blocked story is blocked by the blocker. Always re fetch the links after creating and read the rendered direction, an inverted link silently poisons planning. Fetch existing links first and skip duplicates.
|
||||
5. Fill the testing tables: fetch the current `customfield_10129` (risk) and `customfield_10253` (test cases) field JSON, write the draft's rows as a markdown table matching each table's column count, merge with `scripts/review2adf.py tables <field.json> <rows.md>`, and PUT the result. The merge preserves the instruction panels and header rows, never replace those fields with a from scratch document.
|
||||
6. Create proposed test subtasks the draft called for: POST issue with the Subtask issue type, parent set to the story, a one line description of the test type and coverage, and its estimate.
|
||||
7. Verify: re fetch the story, re render `review.md`, and compare the stored field against it normalizing `localId`, `colwidth`, and `width` attrs, and treating empty `attrs` objects as absent (Jira's editor adds `attrs: {}` to paragraphs on any resave, it is not drift). Confirm the estimate, risk level, links, and tables landed. Report exactly what was set and what remains manual (LaunchDarkly, closing the epic's Dev Review ticket).
|
||||
|
||||
## The review anatomy
|
||||
|
||||
`review.md` uses the template's sections shaped the way the team's real reviews shape them (see `references/example.md`):
|
||||
|
||||
- `## 1. Objective`, one or two sentences, no technical detail.
|
||||
- `## 2. Current State`, bullets, factual, each grounded in a verified path: what exists, what pattern it demonstrates, what does not exist, what neighboring stories own. N/A only for genuinely net new surfaces.
|
||||
- `## 3. Desired State`, observable behavior after the story, not implementation.
|
||||
- `## 4. Scope`, four subsections:
|
||||
- `### In scope`, concrete bullets of what this story includes.
|
||||
- `### Developer acceptance criteria`, opening with the house convention line, `The story's AC-1 through AC-N remain authoritative and unchanged.`, optionally followed by one sentence naming any delta the review introduces. Then the implementation grade conditions the reviewer holds the work to: which seams are used, what is not imported or duplicated, what the tests must demonstrate. These complement the story's ACs, referenced by ID, never restated.
|
||||
- `### Explicitly out of scope`, the guardrails, including what neighboring stories own.
|
||||
- `### Estimate`, the work and hours breakdown table, then `Recommended Jira original estimate: N engineering hours.` Then the risk level line with its one sentence reason. If over 8 hours, the split argument lives here.
|
||||
- `## Relationships`, after Scope. The dependency picture in prose the developer can act on, four labels with a one line reason each: `Blocked by:` (what interface or schema this needs and from whom), `Blocks:` (what downstream work waits on this), `Not blocked by:` (dependencies a reader might wrongly assume, explicitly denied with the reason), and `Coordinates with:` (stories touching the same surfaces where behavior must line up without a blocking edge). The reasons matter more than the list, and the Not blocked by line is often the most valuable, it kills false serialization. The Handoff data links are derived from this section.
|
||||
- `## 5. Suggested Implementation`, opening with `### Code paths and intended updates`, a two column table, one row per file including test files, each intent one or two sentences. After the table, intended code shape snippets as fenced code blocks where shape matters (the authorization check, the Prisma block, the derivation function), and do not guidance naming the existing seams that make workarounds unnecessary.
|
||||
- `## 6. Happy Path`, a numbered straight through flow of the feature working.
|
||||
- `## 7. Edge Cases to Consider`, two subsections: `### Provided` (from the story and its notes) and `### Added during dev review` (the reviewer's own, this is where the review earns its keep).
|
||||
- `## Unresolved product inputs before final approval`, only when genuine open inputs exist, never manufactured. Each item is a decision phrased for its owner (PM decision, PM/UX decision, or a named coordination with another story's review), with the options stated and, where the choice changes the estimate, the cost of each option. A dev implementation choice the reviewer can make is not a product input, decide it in the review instead. When this section exists, surface its items in the draft summary to the user, and keep the list to the smallest real set. Where the section holds several items for one owner and they need answering async rather than in a conversation, offer to turn them into a questionnaire with `/to-questionnaire`, which puts them in one document that owner can fill in in a single pass.
|
||||
|
||||
The template's Acceptance Checklist and Out of Scope sections fold into Scope as shown above, matching house practice. The template's Developer Responsibilities block is omitted, it belongs to the developer at MR time and the ticket skill's mr.md already answers it.
|
||||
|
||||
Below the anatomy, `review.md` ends with a `## Handoff data` section that is not rendered into the Jira field (strip it before rendering): the chosen risk level and reason, the dependency links to create, the testing table rows, proposed subtasks, and the LaunchDarkly note. Keep the cut point clean: `scripts/review2adf.py render` gets a copy of the file truncated at the Handoff data heading.
|
||||
@@ -0,0 +1,44 @@
|
||||
# Estimation method
|
||||
|
||||
The estimate is engineering hours for the story's scope as written. It excludes buffer: the Story Risk Level is what communicates uncertainty buffer, per the dev review SOP. Never conflate the two, a large story can be LOW risk (lots of pattern following work) and a small story can be MEDIUM or HIGH (novel modeling in few files).
|
||||
|
||||
## The method, in order
|
||||
|
||||
1. Build the Code paths and intended updates table first. The estimate is derived from it, never invented independently of it.
|
||||
2. Assign hours per row or per cluster of rows using the anchor table below, at 0.25 hour granularity. Include test files in the rows they verify.
|
||||
3. Sum and round up to the nearest 0.5 hour.
|
||||
4. Sanity check the total against the nearest anchor story below. If the total differs from the anchor by more than about a third, either the scope read is wrong or the anchor does not fit, figure out which and say so in the Estimate subsection.
|
||||
5. Apply the bands:
|
||||
- Bugs and small tasks default to 2 hours, 1 hour when genuinely trivial.
|
||||
- Stories normally land between 2 and 8 hours. 8 hours is the practical upper bound for a single ticket.
|
||||
- A total above 8 hours is a solid argument the ticket should be more than one ticket: the review must say so and sketch the split.
|
||||
- 10 hours is a hard cap. The skill never sets an Original Estimate above 10 hours, ever. A story that computes above 10 gets no estimate set until the split question is resolved with the user.
|
||||
6. Show the work: the review's Estimate subsection carries the breakdown table (work item, hours) and ends with "Recommended Jira original estimate: N engineering hours."
|
||||
7. Calibrate down, not up (Gib, 2026-08-14). The estimates assume Gib develops with agent assistance, so pattern following surfaces, cards, column definitions, copy sweeps, and test files go faster than a solo dev baseline. Lean toward the low end of every band. Across an epic the distribution matters: most stories should land 2 to 6 hours, and a wall of 8 plus estimates reads like gaming the system to the people approving them. Reserve 8 and above for genuinely large compositions, and when a total creeps high, first re-check the scope read for anything that actually belongs to a neighboring story.
|
||||
|
||||
## Anchor table, calibrated from real Command Center estimates
|
||||
|
||||
From 100 estimated KACP issues on the Ksense Command Center account (53 bugs median 2h, 24 tasks median 2h, 23 stories median 5h), with named anchors:
|
||||
|
||||
| Work shape | Hours | Anchors |
|
||||
| --- | --- | --- |
|
||||
| Trivial fix, styling pass, config change | 1 to 2 | KACP-22843 styling 1h, KACP-22868 tutorials fix 2h |
|
||||
| Bug fix on an existing surface | 2 | The overwhelming KCC default, 2h across dozens of bugs |
|
||||
| Small workflow or logic tuneup | 3 | KACP-22201 interest holder workflow tuneup 3h |
|
||||
| Notification or email story on existing plumbing | 3.5 | KACP-22194, KACP-22195 notifications 3.5h each |
|
||||
| Modal or single form on existing patterns | 4 to 5 | KACP-22197 logs modal 4.5h |
|
||||
| Additive schema plus seed, no backfill | 3.5 to 5 | KACP-22200 cron sync tables 5h |
|
||||
| Grid rework or tuneup on the existing DataGrid | 5 | KACP-22196 epic grid tuneup 5h |
|
||||
| New grid page on existing patterns | 6 to 8 | Portal grid reviews ran 6h (KACP-23136) to 10h (KACP-22814), KCC side stays in band |
|
||||
| Detail page with cards or tabs on existing patterns | 5 to 7 | KACP-22882 signature logging and data model 7h |
|
||||
| New end to end surface, UI plus API plus side effects | 6.5 to 8 | KACP-22888 e-sign email 6.5h, KACP-22885 certificate 7.5h |
|
||||
| Schema with production backfill or migration review queue | Add 2 to 3 to the schema base | This is the most common reason a story crosses 8 and should split |
|
||||
|
||||
Historical over the line examples for the split argument: KACP-22881 at 9.5h, KACP-23006 at 14h, KACP-20762 at 17h. Under the current rule each of those gets a split recommendation in its review instead of a large estimate.
|
||||
|
||||
## Per row guidance
|
||||
|
||||
- A focused test file rides with its surface at 0.5 to 1 hour, not as a separate large line.
|
||||
- LaunchDarkly flag plumbing on a surface is 0.5.
|
||||
- Seed catalog entries are 0.5 to 1.
|
||||
- Route or page shell with nav and guard is 1 to 2 on its own, and folds into the page's line when the story includes the page.
|
||||
@@ -0,0 +1,160 @@
|
||||
# Reference example: KACP-22813 Firm - Routes and page (dev review, verbatim)
|
||||
|
||||
This is a real Developer Review Instructions field from a reviewed story, rendered to markdown. It is the calibration target for structure, depth, and voice. Note it comes from the APSCA portal repo, so its file paths and stack idioms are that repo's, only the anatomy is the standard.
|
||||
|
||||
## 1. Objective
|
||||
|
||||
Add the Firm Team page shell at `/firm/team` by reusing the established full-width Firm Tasks page composition. Register Team in the Firm navigation and apply the existing Firm Membership CASL permission consistently to both the menu item and direct route access.
|
||||
|
||||
## 2. Current State
|
||||
|
||||
- The Firm portal layout already authenticates the user, resolves their Firm Membership, renders the Firm sidebar, and protects the Firm portal as a whole.
|
||||
- `src/app/[locale]/firm/tasks/layout.tsx` already demonstrates the Firm data-grid page composition: a shared `PageHeader` followed by `MainContent` using `layoutWidth="dataGrid"`.
|
||||
- `src/app/[locale]/firm/tasks/page.tsx` provides the established responsive `Stack` and full-width `Box` layout for a Firm table or grid.
|
||||
- `src/app/[locale]/firm/tasks/_components/tasks-header.tsx` provides the closest Firm header implementation using the shared `PageHeader`.
|
||||
- `getFirmMenuConfig()` already registers Firm Management menu items. `createSidebarMenu()` applies CASL to each configured `subject` and renders denied items in the existing locked state.
|
||||
- `firmPortal.can()` already exposes a server-side permission check without requiring page code to inspect Firm roles.
|
||||
- No `/firm/team` route or Team menu item currently exists.
|
||||
- The currently implemented `FirmMembership` read rule permits every Firm portal membership. The role-aware Firm Administrator, Team Member, and Contact rules are owned by the intended review for Database, CASL policies, etc. (KACP-22817) and must be available for the required access behavior to work.
|
||||
|
||||
## 3. Desired State
|
||||
|
||||
The Firm sidebar displays Team under Firm Management.
|
||||
|
||||
- Firm Administrators and Team Members receive an enabled Team menu item and can open `/firm/team`.
|
||||
- Contacts see the Team menu item in the existing locked state.
|
||||
- A Contact who enters `/firm/team` directly is redirected to the localized Firm dashboard.
|
||||
|
||||
The page follows the existing portal presentation:
|
||||
|
||||
- A shared Firm page header displays the title **Team** and subtitle **Manage who can access your account and who APSCA contacts.**
|
||||
- The content uses the same data-grid width as Firm Tasks.
|
||||
- One full-width Team content surface provides the insertion point for the grid delivered by Firm - Team Members Grid (KACP-22814).
|
||||
|
||||
The menu and page use the same `read`, `FirmMembership` CASL capability. The page does not inspect roles or duplicate authorization policy.
|
||||
|
||||
## 4. Scope
|
||||
|
||||
### In scope
|
||||
|
||||
- Add the `/firm/team` server page.
|
||||
- Add the Team page header using the shared `PageHeader` and the Firm Tasks header pattern.
|
||||
- Compose the page with `MainContent`, `Stack`, and `Box` using the existing data-grid width layout.
|
||||
- Add Team under Firm Management in the Firm menu configuration.
|
||||
- Use `FirmMembership` as the menu item's CASL subject.
|
||||
- Check the same Firm Membership read capability in the page before rendering.
|
||||
- Redirect denied direct-route access to the localized Firm dashboard.
|
||||
- Add focused route and navigation authorization coverage.
|
||||
|
||||
### Developer acceptance criteria
|
||||
|
||||
1. The Team page remains a server component and resolves authorization before rendering protected content.
|
||||
2. The page uses `firmPortal.can('read', 'FirmMembership')`; it does not import the CASL engine or inspect `FirmContactRole`.
|
||||
3. The menu item and route guard both use the `FirmMembership` subject, preventing navigation and direct access from drifting apart.
|
||||
4. A denied direct request redirects through `localePath('/firm/', locale)` so the current locale is preserved.
|
||||
5. Firm portal authentication and Firm Membership resolution remain owned by the existing Firm layout.
|
||||
6. The Team content uses `layoutWidth="dataGrid"` and a full-width `Box` with `flex: 1` and `minWidth: 0`, matching Firm Tasks.
|
||||
7. The header uses the shared `PageHeader` with the same data-grid width, preserving its semantic heading markup and responsive spacing.
|
||||
8. The page shell introduces no grid query, client state, mutation, or role-specific rendering.
|
||||
9. Focused tests demonstrate the allowed page render, denied redirect, enabled authorized menu item, and locked unauthorized menu item.
|
||||
10. Verification uses the role-aware CASL policy from KACP-22817 to confirm Firm Administrator and Team Member access and Contact denial.
|
||||
|
||||
### Explicitly out of scope
|
||||
|
||||
- Team grid rows, columns, data loading, empty states, pagination, search, sorting, or filtering.
|
||||
- Add Person, Edit Person, View Person, or any other Team interaction.
|
||||
- Add or edit controls.
|
||||
- Prisma schema or migration changes.
|
||||
- Defining or duplicating the Firm Administrator, Team Member, or Contact CASL rules owned by KACP-22817.
|
||||
- Firm Membership commands or mutations.
|
||||
- Clerk identity or invitation behavior.
|
||||
- Communications, templates, or notification behavior.
|
||||
|
||||
### Estimate
|
||||
|
||||
| Work | Hours |
|
||||
| --- | --- |
|
||||
| Team route, header, and data-grid-width page shell | 0.75 |
|
||||
| Firm menu item, CASL route check, and localized redirect | 0.5 |
|
||||
| Focused tests and verification | 0.75 |
|
||||
| **Total** | **2** |
|
||||
|
||||
Recommended Jira original estimate: **2 engineering hours**.
|
||||
|
||||
## 5. Suggested Implementation
|
||||
|
||||
### Code paths and intended updates
|
||||
|
||||
| Code path | Brief intended update |
|
||||
| --- | --- |
|
||||
| `src/app/[locale]/firm/team/page.tsx` | Add the server page. Resolve `params` and `firmPortal.can('read', 'FirmMembership')`, redirect denied users to the localized Firm dashboard, and compose the Team header plus the data-grid-width `MainContent`, responsive `Stack`, and full-width `Box` used by Firm Tasks. Reserve the content surface for the later grid story. |
|
||||
| `src/app/[locale]/firm/team/_components/team-header.tsx` | Copy the small Firm Tasks header wrapper, rename it for Team, and render the required title and subtitle through the shared `PageHeader` with `layoutWidth="dataGrid"`. Do not introduce a new generic header abstraction. |
|
||||
| `src/app/[locale]/firm/_components/menu-config.tsx` | Add Team under Firm Management, linking to the localized `/firm/team/` route, using the existing `Users` icon and `subject: 'FirmMembership'` so the shared menu authorization renders the locked Contact state. |
|
||||
| `src/app/[locale]/firm/team/page.test.tsx` | Add focused server-page coverage for authorized rendering and denied localized redirect. |
|
||||
| `src/app/[locale]/firm/_components/menu-config.test.tsx` | Confirm the Team item is registered under Firm Management and that the shared authorization transform enables it when `FirmMembership` read is allowed and locks it when denied. Do not test role names in the menu configuration. |
|
||||
|
||||
The intended server-page authorization shape is:
|
||||
|
||||
```tsx
|
||||
const [{ locale }, canViewTeam] = await Promise.all([
|
||||
params,
|
||||
firmPortal.can('read', 'FirmMembership'),
|
||||
]);
|
||||
|
||||
if (!canViewTeam) {
|
||||
redirect(localePath('/firm/', locale));
|
||||
}
|
||||
```
|
||||
|
||||
The page should then follow the established composition:
|
||||
|
||||
```tsx
|
||||
<>
|
||||
<TeamHeader />
|
||||
<MainContent
|
||||
layoutWidth="dataGrid"
|
||||
sx={{ py: 3, pb: 8 }}
|
||||
>
|
||||
<Stack
|
||||
direction={{ md: 'column', lg: 'row' }}
|
||||
alignItems="flex-start"
|
||||
>
|
||||
<Box sx={{ flex: 1, minWidth: 0, width: '100%' }}>
|
||||
{/* KACP-22814 supplies the Team grid. */}
|
||||
</Box>
|
||||
</Stack>
|
||||
</MainContent>
|
||||
</>
|
||||
```
|
||||
|
||||
Do not add a client-side role check, new portal service, or grid read model. The shared `PageHeader`, `MainContent`, Firm Portal façade, and menu authorization already provide the required seams.
|
||||
|
||||
## 6. Happy Path
|
||||
|
||||
1. An authenticated Firm Administrator or Team Member opens the Firm portal.
|
||||
2. The Firm layout resolves their current Firm Membership and builds the Firm CASL ability.
|
||||
3. The sidebar applies that ability to the Team menu item's `FirmMembership` subject and renders the item as enabled.
|
||||
4. The user opens Team and reaches the localized `/firm/team` route.
|
||||
5. The server page checks the same Firm Membership read capability.
|
||||
6. The page renders the Team header and full-width data-grid layout.
|
||||
7. The Team content surface is ready for KACP-22814 to supply the grid without changing the route, header, navigation, width, or authorization composition.
|
||||
|
||||
## 7. Edge Cases to Consider
|
||||
|
||||
### Provided
|
||||
|
||||
- A Team Member can view the Team page but cannot edit it.
|
||||
- A Contact sees the Team menu item in a locked state.
|
||||
- A Contact who enters the route directly is redirected to the Firm dashboard.
|
||||
- Grid content and interactions belong to a separate story.
|
||||
|
||||
### Added during dev review
|
||||
|
||||
- The route must preserve the current locale when redirecting a denied user.
|
||||
- A user without a valid Firm Membership remains rejected by the existing Firm layout before the Team page is rendered.
|
||||
- The menu and direct route must not use different CASL subjects or independent role checks.
|
||||
- KACP-22817 must replace the current broad Firm Membership read grant before Contact denial can be verified correctly.
|
||||
- The header and main content must both use the data-grid width so their horizontal alignment remains consistent.
|
||||
- The table/grid wrapper must retain `minWidth: 0` and `width: '100%'` so wide content does not break the portal layout.
|
||||
- The empty shell must not invent temporary grid data, controls, or client state that KACP-22814 would later remove.
|
||||
|
||||
@@ -0,0 +1,14 @@
|
||||
# SOP for Development Review (KACP-23147, verbatim duties)
|
||||
|
||||
Developer Review Instructions. SOP for Development Review:
|
||||
|
||||
1. **Review the Epic.** Review the issues in this epic to ensure you understand what the requirements are. Reach out to the PM for clarification if needed.
|
||||
2. **Handle Sprint Flag.** Assess whether a flag is needed for this Sprint, create it, and connect it to the Epic. Create the flag in LaunchDarkly (if applicable) and connect it to the Epic in the Releases field.
|
||||
3. **Add development instructions.** For each issue in the epic, add development instructions using the Developer Review Instructions field. Follow the Developer Instructions template to provide clear, concise, and actionable guidance for the development team. Focus on providing context and information not already outlined in the task details to avoid redundancy.
|
||||
4. **Add issue dependencies.** Identify and link dependent issues within the epic using the Blocks or is blocked by link type. Ensure that the issue dependencies are accurately represented to facilitate proper planning and execution.
|
||||
5. **Provide time estimates.** Fill out the Original Estimate field for each issue in the epic. Provide your best estimate of how long each issue will take to complete, considering factors such as complexity, dependencies, and potential risks.
|
||||
6. **Fill out the Risk Level.** Choose the appropriate Story Risk Level for each task in the epic. The Risk Level adds a buffer to the original estimate based on the task complexity, accounting for potential unknowns or challenges.
|
||||
7. **Fill out the Testing Tab.** Based on the Technical Review and your assessment of the risks associated with each task, fill out the Risk Mitigation and Test Cases tables.
|
||||
- Risk Mitigation: identify and list the risks associated with each task. Specify the mitigation strategy for each risk, which could include guidance on how to build the feature in a way that mitigates the risk, or outlining automated tests that need to be created to address the risk. If an automated test is required, create a subtask on the main task with the type of test and what it covers, and an estimate for the subtask.
|
||||
- Test Cases: outline the manual test cases that the developer should execute to ensure the functionality works as expected and risks have been addressed.
|
||||
8. Close this issue when finished.
|
||||
@@ -0,0 +1,163 @@
|
||||
import json
|
||||
import re
|
||||
import sys
|
||||
|
||||
INLINE_CODE = re.compile(r"`([^`]+)`")
|
||||
BOLD = re.compile(r"\*\*([^*]+)\*\*")
|
||||
|
||||
|
||||
def text_node(text, marks=None):
|
||||
node = {"type": "text", "text": text}
|
||||
if marks:
|
||||
node["marks"] = [{"type": m} for m in marks]
|
||||
return node
|
||||
|
||||
|
||||
def inline_nodes(text):
|
||||
nodes = []
|
||||
pos = 0
|
||||
pattern = re.compile(r"`([^`]+)`|\*\*([^*]+)\*\*")
|
||||
for m in pattern.finditer(text):
|
||||
if m.start() > pos:
|
||||
nodes.append(text_node(text[pos:m.start()]))
|
||||
if m.group(1) is not None:
|
||||
nodes.append(text_node(m.group(1), ["code"]))
|
||||
else:
|
||||
nodes.append(text_node(m.group(2), ["strong"]))
|
||||
pos = m.end()
|
||||
if pos < len(text):
|
||||
nodes.append(text_node(text[pos:]))
|
||||
return nodes or [text_node("")]
|
||||
|
||||
|
||||
def paragraph(text):
|
||||
return {"type": "paragraph", "content": inline_nodes(text)}
|
||||
|
||||
|
||||
def heading(level, text):
|
||||
return {"type": "heading", "attrs": {"level": level}, "content": inline_nodes(text)}
|
||||
|
||||
|
||||
def code_block(lang, lines):
|
||||
attrs = {"language": lang} if lang else {}
|
||||
return {"type": "codeBlock", "attrs": attrs, "content": [text_node("\n".join(lines))]}
|
||||
|
||||
|
||||
def list_node(items, ordered):
|
||||
return {
|
||||
"type": "orderedList" if ordered else "bulletList",
|
||||
"content": [{"type": "listItem", "content": [paragraph(i)]} for i in items],
|
||||
}
|
||||
|
||||
|
||||
def cell(kind, text):
|
||||
return {"type": kind, "attrs": {}, "content": [paragraph(text)]}
|
||||
|
||||
|
||||
def table(rows):
|
||||
out = {"type": "table", "attrs": {"layout": "default"}, "content": []}
|
||||
for i, row in enumerate(rows):
|
||||
kind = "tableHeader" if i == 0 else "tableCell"
|
||||
out["content"].append({"type": "tableRow", "content": [cell(kind, c) for c in row]})
|
||||
return out
|
||||
|
||||
|
||||
def split_row(line):
|
||||
return [c.strip() for c in line.strip().strip("|").split("|")]
|
||||
|
||||
|
||||
def build_adf(md):
|
||||
lines = md.splitlines()
|
||||
content = []
|
||||
i = 0
|
||||
while i < len(lines):
|
||||
line = lines[i]
|
||||
if not line.strip():
|
||||
i += 1
|
||||
continue
|
||||
if line.startswith("```"):
|
||||
lang = line[3:].strip()
|
||||
body = []
|
||||
i += 1
|
||||
while i < len(lines) and not lines[i].startswith("```"):
|
||||
body.append(lines[i])
|
||||
i += 1
|
||||
i += 1
|
||||
content.append(code_block(lang, body))
|
||||
continue
|
||||
m = re.match(r"^(#{1,6})\s+(.*)", line)
|
||||
if m:
|
||||
content.append(heading(len(m.group(1)), m.group(2).strip()))
|
||||
i += 1
|
||||
continue
|
||||
if line.lstrip().startswith("|"):
|
||||
rows = []
|
||||
while i < len(lines) and lines[i].lstrip().startswith("|"):
|
||||
if not re.match(r"^\s*\|[\s:|-]+\|\s*$", lines[i]):
|
||||
rows.append(split_row(lines[i]))
|
||||
i += 1
|
||||
content.append(table(rows))
|
||||
continue
|
||||
if re.match(r"^\s*- ", line):
|
||||
items = []
|
||||
while i < len(lines) and re.match(r"^\s*- ", lines[i]):
|
||||
items.append(re.sub(r"^\s*- ", "", lines[i]).strip())
|
||||
i += 1
|
||||
content.append(list_node(items, ordered=False))
|
||||
continue
|
||||
if re.match(r"^\s*\d+\.\s", line):
|
||||
items = []
|
||||
while i < len(lines) and re.match(r"^\s*\d+\.\s", lines[i]):
|
||||
items.append(re.sub(r"^\s*\d+\.\s", "", lines[i]).strip())
|
||||
i += 1
|
||||
content.append(list_node(items, ordered=True))
|
||||
continue
|
||||
para = [line.strip()]
|
||||
i += 1
|
||||
while i < len(lines) and lines[i].strip() and not re.match(r"^(#|```|\s*-\s|\s*\d+\.\s|\s*\|)", lines[i]):
|
||||
para.append(lines[i].strip())
|
||||
i += 1
|
||||
content.append(paragraph(" ".join(para)))
|
||||
return {"type": "doc", "version": 1, "content": content}
|
||||
|
||||
|
||||
def append_table_rows(field_json_path, rows_md_path):
|
||||
field = json.load(open(field_json_path))
|
||||
rows = []
|
||||
for line in open(rows_md_path).read().splitlines():
|
||||
if line.lstrip().startswith("|") and not re.match(r"^\s*\|[\s:|-]+\|\s*$", line):
|
||||
rows.append(split_row(line))
|
||||
tables = [n for n in field.get("content", []) if n.get("type") == "table"]
|
||||
if not tables:
|
||||
raise SystemExit("no table found in the existing field, refusing to guess")
|
||||
target = tables[-1]
|
||||
header_cells = len(target["content"][0]["content"])
|
||||
body_rows = [r for r in target["content"][1:] if any(
|
||||
t.strip() for c in r["content"] for t in _cell_texts(c))]
|
||||
target["content"] = [target["content"][0]] + body_rows
|
||||
for row in rows:
|
||||
if len(row) != header_cells:
|
||||
raise SystemExit(f"row has {len(row)} cells, table header has {header_cells}: {row}")
|
||||
target["content"].append({"type": "tableRow", "content": [cell("tableCell", c) for c in row]})
|
||||
return field
|
||||
|
||||
|
||||
def _cell_texts(cell_node):
|
||||
out = []
|
||||
def walk(n):
|
||||
if isinstance(n, dict):
|
||||
if n.get("type") == "text":
|
||||
out.append(n.get("text", ""))
|
||||
for c in n.get("content", []):
|
||||
walk(c)
|
||||
walk(cell_node)
|
||||
return out
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
if len(sys.argv) >= 3 and sys.argv[1] == "render":
|
||||
json.dump(build_adf(open(sys.argv[2]).read()), sys.stdout)
|
||||
elif len(sys.argv) >= 4 and sys.argv[1] == "tables":
|
||||
json.dump(append_table_rows(sys.argv[2], sys.argv[3]), sys.stdout)
|
||||
else:
|
||||
sys.exit("usage: review2adf.py render <review.md> | tables <current-field.json> <rows.md>")
|
||||
@@ -0,0 +1,100 @@
|
||||
# Developer Instructions Template
|
||||
|
||||
## 1. Objective
|
||||
|
||||
<!--
|
||||
What does success look like?
|
||||
1–2 sentences. No technical detail.
|
||||
-->
|
||||
|
||||
---
|
||||
|
||||
## 2. Current State
|
||||
|
||||
<!--
|
||||
Describe existing behavior ONLY if this task changes something that already exists.
|
||||
Bullet points only. Keep it factual and brief.
|
||||
If this is net-new work, write "N/A".
|
||||
-->
|
||||
|
||||
---
|
||||
|
||||
## 3. Desired State
|
||||
|
||||
<!--
|
||||
What behavior should exist after this task is complete?
|
||||
Focus on observable behavior, not implementation.
|
||||
-->
|
||||
|
||||
---
|
||||
|
||||
## 4. Scope
|
||||
|
||||
<!--
|
||||
Explicitly list what IS included in this task.
|
||||
Use bullet points. Be concrete.
|
||||
-->
|
||||
|
||||
---
|
||||
|
||||
## 5. Suggested Implementation
|
||||
|
||||
<!--
|
||||
Guidance for how to approach this task.
|
||||
May include:
|
||||
- Architectural direction
|
||||
- Known patterns to reuse
|
||||
- Constraints or preferences
|
||||
- Optional example code or pseudo-code
|
||||
This is guidance, not strict requirements unless explicitly stated.
|
||||
-->
|
||||
|
||||
---
|
||||
|
||||
## 6. Happy Path
|
||||
|
||||
<!--
|
||||
Describe the normal, straight-through flow.
|
||||
Think: "What is the expected sequence of actions when everything works?"
|
||||
-->
|
||||
|
||||
---
|
||||
|
||||
## 7. Edge Cases to Consider (Provided)
|
||||
|
||||
<!--
|
||||
Known edge cases identified by PM/Lead Dev.
|
||||
These help define expectations but are not necessarily exhaustive.
|
||||
-->
|
||||
|
||||
---
|
||||
|
||||
## 8. Acceptance Checklist
|
||||
|
||||
<!--
|
||||
Clear, testable conditions that define "done".
|
||||
Each item should be verifiable.
|
||||
-->
|
||||
|
||||
---
|
||||
|
||||
## 9. Out of Scope / Guardrails
|
||||
|
||||
<!--
|
||||
Explicitly list what should NOT be done in this task.
|
||||
Prevents scope creep and unintended refactors.
|
||||
-->
|
||||
|
||||
---
|
||||
|
||||
# Developer Responsibilities
|
||||
|
||||
## A. Edge Case Coverage (Required)
|
||||
|
||||
<!--
|
||||
List additional edge cases you considered beyond those above.
|
||||
For each:
|
||||
- State the edge case
|
||||
- Explain how it is handled, tested, or explicitly not covered (and why)
|
||||
Limit to 3–5 meaningful items.
|
||||
-->
|
||||
Reference in New Issue
Block a user