Skip to content

feat(formation): checklist section and Formations queue, Epic 1 scope (#1958) - #2033

Draft
manishdixitlfx wants to merge 48 commits into
mainfrom
feat/GH-1958-formation-checklist-queue
Draft

feat(formation): checklist section and Formations queue, Epic 1 scope (#1958)#2033
manishdixitlfx wants to merge 48 commits into
mainfrom
feat/GH-1958-formation-checklist-queue

Conversation

@manishdixitlfx

@manishdixitlfx manishdixitlfx commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Summary

Implements GH-1958 (Epic 1 scope, revised 2026-09-02): the Formation checklist section embedded on the project page, and the staff Formations queue at the LF root — a list of formations already in flight, not a triage queue. Built entirely against fixtures — the real backend (lfx-v2-formation-service, tracked separately as #1957) doesn't exist yet. The feature ships dark behind formation-enabled until #1955 (badge/sidebar) and #1959 (template seed) land alongside it. Project application submission/review (Accept/Decline, a proposer to notify) moved entirely to Epic 2 (#1962) and is out of scope here.

  • Checklist section — now its own route, /project/formation?project=<slug> (see 2026-09-06 entry below) — with a readiness strip (per-item segment bar), two template sections (gating + non-gating), row-level action rendering per kind (manual/link/provisionable/request/status_only), and an item drawer for notes/assignee/due-date/complete/skip/history.
  • Formations queue (/foundation/formations): stat tiles, status-tab filter pills scoped to in-flight sub-stages (exploratory/engaged/on_hold/activating), and a table whose only row action is opening the formation's project page — gated behind a new root-scoped auditor persona check.
  • New shared types (Formation, FormationItem, 5-state FormationItemStatus including waiting_on_partner), a new violet accent tag severity, and a fixture-backed BFF service (formation.service.ts) with every public method routed through isFormationServiceLive() (hardcoded false, // TODO(#1957)) as the single swap point for the real backend.

Trade-offs / known gaps (intentional, Epic 1 scope)

  • 6 new components ship without component-level .spec.ts files (formation-checklist-row/-section, formation-item-drawer, formation-readiness-strip, formations-table, formations-queue). Coverage instead comes from dual content + structural Playwright e2e specs (formation-checklist.spec.ts / -robust.spec.ts, formations-queue.spec.ts / -robust.spec.ts). Flagged again on every full-branch sweep; accepted as a documented gap rather than a blocker.
  • The e2e spec files could not be executed end-to-end in this sandboxed environment (no dev server / test credentials available here) — they're structurally reviewed and pattern-matched against existing passing specs, but haven't had a live run in this session. Manual verification is planned in the integration worktree.
  • Write-affordance gating for non-writers, the data_source: 'fixture' marker (no UI badge yet), checkRootAuditor's unconditional FGA check, and the inert gate_writer union member are all // TODO(#1957)-marked swap points for when the real backend lands.
  • New permission surfaces added by this branch: root-scoped auditor persona check (requireAuditor middleware + formationsQueueAuditorGuard), and a fixture-only gate_writer stand-in (checkLFStaff) gating completion of gating checklist items.
  • Touches 3 protected files (server.ts — new route registration; require-auditor.middleware.ts/.spec.ts — new middleware) per guard-protected-files.sh — flagged for code-owner review; re-confirmed unchanged by every subsequent commit on this branch.
  • Branch naming matches this epic's established sibling convention (feat/GH-<issue>-<slug>, see feat/GH-1955-formation-badge-card, feat/GH-1959-seed-formation-template) rather than the repo's generic feat/issue-<number> pattern — kept for consistency across the epic's parallel branches. /lfx-self-serve-pr-readiness flags this as SHOULD_FIX on every run; kept as a documented, deliberate deviation rather than renamed, since renaming this late would repoint an already-reviewed, already-iterated PR branch.
  • A number of commits (mostly late-stage write-race and reviewer-finding fix-ups) reference the work by scope/description rather than repeating #1958 in the subject/body; the PR itself carries the ticket reference. A handful of commit headers run 74–82 chars, over the 72-char team-style target but under commitlint's 100-char hard-fail — not reworded, since rewriting history on an already-iterated, already-reviewed branch isn't worth the disruption for a non-blocking style nit.
  • formations-queue.component.ts mixes filtered/unfiltered denominators across its stat tiles; formation.interface.ts's action_href doc-comment warning about untrusted values, and two as unknown as casts (formation-checklist.utils.ts, formation-fixture.helper.ts) — all flagged by review, all in the Formations Queue / shared-types / fixture-helper surface explicitly out of scope for this PR's follow-up work (see 2026-09-06 below).

Review history

Every commit went through the mandatory post-commit reviewer trio (general / self-serve-convention / learnings) while pre-PR; multiple full-branch sweeps run clean (aside from the documented gaps above). /lfx-self-serve-pr-readiness returns READY WITH CHANGES (no CRITICAL; SHOULD_FIX items — branch naming, commit header length, protected files — documented above, none blocking). /preflight mechanical checks (license headers, lint, format, build, protected-files) all pass.

2026-09-03 scope trim: Epic 1 was cut back after this PR opened — project application submission/review moved entirely to Epic 2 (#1962). Follow-up commit removes Accept/Decline from the queue (actions, handlers, fixtures, tests), the Proposed/Draft status filter and the Withdrawn filter, and the stated #1962 dependency; adds an "open the project" link as the queue row's only action. No Lead column (a #1992/#2019 Epic 2 record field) — the item-level owner chip inside the checklist is unaffected. This was a post-PR iteration commit, so it went through CodeRabbit/Copilot rather than the reviewer trio, per this repo's post-PR iteration policy.

2026-09-04 canonical types, row-level actions, one gate vocabulary: Follow-up commit migrates this branch's pre-canonical local Formation types onto the shared types landed by GH-2163 (PR #2175) and closes three product gaps review found:

  • Canonical types. Deleted the branch's local formation.interface.ts/formation.constants.ts in favor of the canonical shapes; added the new formation.enum.ts (FormationTemplateSectionKey, FormationOwnerTeam, FormationActionType). FormationItemStatus is now the canonical 6-state union (not_started | in_progress | blocked | awaiting_acceptance | done | skipped) — supersedes the earlier summary's "5-state including waiting_on_partner". FormationEntityType renamed subprojectchild_project throughout (UI copy, tiles, fixtures). FormationSubStage drops activating (exploratory | engaged | on_hold only) — is_activating is now a boolean flag independent of sub_stage, so a formation's real stage is never overwritten/lost when gates clear or re-open.
  • Every checklist row is actionable. The row's status chip is now a dropdown (lfx-menu) offering the valid next transitions (Mark in progress / Mark done / Accept / Mark blocked… / Back to not started), plus a kebab overflow menu (Assign, Set due date — both open the existing item drawer; Skip with reason). New PATCH status-transition endpoint backs the three "plain" transitions; completion and skip keep their existing dedicated endpoints. No dedicated "blocked reason" field exists on the canonical type — the optional note from "Mark blocked…" rides on the item's existing notes field. ReasonPromptDialogComponent (reused for this) only supports a required reason, not an optional one, so "Mark blocked…" requires a reason, same as the existing skip flow — a deliberate deviation from the plan's original "optional note" framing, made after reading the dialog's actual canConfirm contract.
  • awaiting_acceptance handled correctly, never double-counted. A non-gate_writer caller completing a gating item now sets awaiting_acceptance (not done) — a gate_writer/can_complete caller then "Accepts" it, which is the drawer's existing Mark-complete button relabeled. awaiting_acceptance items count as open for gating math (is_activating/the "N of M open" figure) since the gate hasn't actually cleared yet, render with a blue info chip (With formation team) and a blue readiness-strip segment — never green/done.
  • One gate vocabulary. "Gates Active" (row + drawer chip) and "Gating Active" (readiness strip) are both now "Required for Active"; the readiness strip's zero-open state reads All gating items done — the formation team can set Active in the admin tool on the announcement date. instead of implying "Active" is already set. The queue table gets a new Gates cleared badge beside the Progress cell when is_activating, and its Blocking column falls back to Formation to set Active for a fully-gated row instead of a blank dash.
  • sub_items now actually renders. The fixture generator populates seeded sub-items (previously hardcoded to [], so the drawer's sub-item rendering was dead code); the row shows a compact "N of M sub-items done" summary; the drawer upgrades the sub-item list from raw status text to lfx-tag using the same label/severity maps as the parent item's own status chip. This row-summary + drawer-tag treatment is this branch's own design — no artboard in the design canvas depicts sub-item rendering.
  • Smaller items: Type column now renders a proper display label (Foundation/Child project/Project) instead of the raw entity_type string; gate_writer's stale #2148 TODO re-pointed to #1957 (the real FGA relation question resolves there, not FGA model change for the formation checklist: new formation types vs. new grants on project #2148).

This was a post-PR iteration commit (bots, not the reviewer trio, per this repo's policy). Full gate all green.

2026-09-06 checklist moved to its own project route, stage-gate defect fixed: Product decision — the checklist (17 rows, per-row actions, a drawer) is a workspace, not a dashboard section; it needs to be linkable from a Me-lens row (#1956) and an item-assigned email (#1961), which "scroll down on the project page" can't support. This follow-up:

  • Relocates the checklist off project-dashboard.component.html onto a new route /project/formation?project=<slug>, guarded by a new formationProjectEnabledGuard (CanMatch). The dashboard now renders a small progress-teaser card (formation-entry-card) linking to the new route instead of embedding the full section. Sidebar gets a "Formation" entry under Dashboard, same gate.
  • Fixes a real, pre-existing defect surfaced during diagnosis: the stage gate this branch (and the rest of the app) relied on — isFormationStage(projectService.project()?.stage) — never evaluated true, because ProjectService.project is a dead signal: it's only ever written from getProject(slug, current=true, ...)'s tap(), and every call site in the app (~16, confirmed by direct read) passes current=false. Nobody had ever actually seen this checklist gate open, on this branch or off it. Two other live call sites read this same dead signal today — upcoming-committee-meeting.component.ts:25 and meeting-card.component.ts:225 — so this defect reaches beyond formation; worth its own follow-up ticket, not fixed here beyond this note.
    • Fix: added ProjectContextService.activeProjectStage — a new signal that resolves the stage via projectService.getProject(slug, false) (the working call already used everywhere else in that service), keyed off activeContext()'s slug. Used by the dashboard's teaser-card gate and the sidebar's visibility check.
    • The route's own CanMatch guard can't use activeProjectStageCanMatch runs before projectQueryParamGuard populates activeContext() for the navigation in progress, so it would read the previous route's project. formationProjectEnabledGuard instead resolves the slug directly from router.getCurrentNavigation()?.extractedUrl.queryParams['project'] (falling back to router.parseUrl(router.url)), mirroring the existing mktgOsAgentsEnabledGuard pattern, then fetches the project directly via projectService.getProject(slug, false) and checks isFormationStage(project?.stage).
    • Proof the gate opens: formation-project-enabled.guard.spec.ts exercises a fixture project with stage: 'Formation - Exploratory' and asserts the guard allows the route; a stage: 'Active' fixture asserts it redirects to /project/overview?project=<slug>. Both paths pass.
  • Known limitation: both the sidebar-nav gate and the dashboard entry-card gate read activeProjectStage() with no readiness/loading signal of their own — on a slow initial fetch, the nav item / teaser card can render absent for a beat before the stage resolves, rather than a distinct loading state. Not fixed here; flagged for anyone touching this gate next.
  • Cold-load / SSR verification deferred: this needs a live dev server against a real Formation-stage fixture project to confirm the direct /project/formation?project=<slug> load renders without a redirect flash — not exercised in this sandboxed session; planned as part of the integration-worktree manual pass alongside the rest of this PR's outstanding manual verification.
  • Branch rebased onto latest origin/main as part of this follow-up (previously ~40+ commits behind) — same commits, same content, replayed onto current main; verified via matching commit-message sequence and a diff showing zero file changes unexplained by main's own evolution since the prior base.
  • New/updated specs: formation-project-enabled.guard.spec.ts (allow for formation stage + flag on, deny for non-formation stage, deny for flag off, SSR fast-path), sidebar spec (Formation item present/absent by stage+flag), project-dashboard.component.spec.ts (section no longer rendered, teaser card rendered instead), plus server-side fixes found on this session's whole-branch review: updateFormationItemStatus no longer clobbers item.notes when blocking with a reason (the reason now rides in the activity's metadata instead), and reversing a gating item off done/awaiting_acceptance now requires the same gate_writer/can_complete check as completing it — client-side menu now disables those same actions when can_complete is false, so the UI never offers a transition the server will reject. A template-purity fix in formation-item-drawer also replaced two method calls in the template with a precomputed subItemRows signal, per this repo's signals/pipes-only template rule.

This was a Mode 2 (full-branch pre-PR) review pass, not a post-commit one — see the repo's Pre-PR review protocol. Full gate (./check-headers.sh && yarn lint && yarn format && yarn build && yarn test) all green: 1966/1966 unit tests, build succeeds including the SSR bundle. /lfx-self-serve-pr-readiness re-run against the rebased head: READY WITH CHANGES, no CRITICAL.

2026-09-07 the Formations queue had no way in: Hand-testing on 7 Sep found /foundation/formations had zero navigation path to it — sidebar-nav.service.ts only spliced a "Formation" item (singular, /project/formation) into the project lens; the foundation lens had nothing. For an epic whose first success measure is "the formation team can see per-project progress without asking in Slack," a queue reachable only by typing its URL wasn't shipped.

  • Added a "Formations" item (plural, /foundation/formations) to the foundation lens, spliced between Events and Mailing Lists — a peer of Projects/Meetings/Events under the foundation Dashboard group, per the requested layout.
  • Guarded with the same condition the route's own formationsQueueAuditorGuard enforces (isRootWriter || isAuditor), not a looser one. That guard is CanActivateFn-only — it redirects via Router.createUrlTree, a shape a synchronous nav-item-visibility computed can't call directly — so the condition is reproduced as a Signal (canSeeFormationsQueue) rather than duplicating the guard's redirect/observable plumbing under a different form. This is the guard-logic-not-directly-reusable case the ticket asked to flag if it came up.
  • PersonaService.isAuditor/isRootWriter only populate as a side effect of a root-scoped refreshEnrichedPersonas() call, and no such call was guaranteed to have fired before a user lands straight on the foundation lens (the one unconditional root-scoped caller found, multi-persona-dashboard.component.ts, only fires from the Me-lens root dashboard). Added foundationAuditorBootstrap, a reactive signal mirroring the existing marketingPersonaSlug pattern, that fires the probe whenever an authenticated user is actually on the foundation lens.
  • An auditor-only user (no board role/root-writer/LF-staff/writer-foundation grant) never satisfies hasFullFoundationAccess(), so a single insertion point inside that gate would have hidden the item from exactly the persona the ticket requires it for. Added a second insertion point outside that gate, mirroring the existing foundationMktgOsAgentsNavItem special case for marketing-only FGA users.
  • New specs in sidebar-nav.service.spec.ts: item present + correctly positioned for a full-access user with the flag on; still present (reduced sidebar) for an auditor without full foundation access; absent with the flag off even for an auditor; absent for a user who is neither auditor nor root-writer even with the flag on.

Full gate (./check-headers.sh && yarn lint && yarn format && yarn build && yarn test) all green: 1970/1970 unit tests (11/11 in sidebar-nav.service.spec.ts), build succeeds including the SSR bundle.

2026-09-07 (later) Copilot review on the nav-item commit, one genuine finding fixed: Copilot re-reviewed the pushed head and generated no new comments against the diff above; it also resurfaced 5 previously-unaddressed findings on code unchanged since the last review. Of those, one — formation-entry-card.component.ts:58 — is in scope for this branch (the entry card is this follow-up's own new code, not part of the excluded Formations-queue/checklist/fixture/shared-types surface): a failed getProjectFormation() fetch mapped to the same null summary as a formation with zero items, so an outage rendered as an authoritative "0 of 0 done" instead of an error. Fixed by tracking the failure in its own hasError signal and rendering a distinct error message in that case; new spec covers the failure path. Thread replied-to and resolved.

The other 4 resurfaced findings, plus the ~16 other unresolved threads from earlier reviews, are all on the pre-existing Formations queue / checklist / fixture-helper / shared-types surface this ticket's "Not in scope" section explicitly excludes (formation.service.ts, formations-table.component.html, formation-checklist-row.component.html, formation-item-drawer.component.html, formation.interface.ts, formation.constants.ts, formation-checklist.utils.ts, formation-checklist-robust.spec.ts, formation-fixture.helper.ts) — the same surface already carried in this PR's "Trade-offs / known gaps" section above. Left open rather than silently resolved or fixed here; they belong to whichever ticket picks up that surface (several are already // TODO(#1957)-tagged).

Full gate re-run after this fix, all green: 1971/1971 unit tests, build succeeds including the SSR bundle.

Test plan

  • yarn lint && yarn format && yarn build && yarn test all green (1971/1971 unit tests)
  • License headers pass (./check-headers.sh)
  • Stage-gate fix proven via formation-project-enabled.guard.spec.ts (Formation-stage fixture allows, Active-stage fixture denies)
  • Foundation sidebar visibility on/off, and project sidebar visibility on/off, proven via sidebar-nav.service.spec.ts
  • Post-PR Copilot review on the nav-item commit: no new findings against this branch's own diff; one in-scope resurfaced finding (dashboard entry-card error state) fixed and thread resolved
  • Manual verification in the integration worktree with formation-enabled forced on, including the new route's cold SSR load (flag-gated, not yet exercised against a live dev server in this session)
  • Live Playwright run of the e2e spec pairs (formation-checklist(-robust), formations-queue(-robust)) — structurally reviewed but not executed here

🤖 Generated with Claude Code

Copilot AI balanced review requested due to automatic review settings September 1, 2026 19:35
@coderabbitai

coderabbitai Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Essentials

Run ID: 74730b46-421f-4a05-ad1c-b99fdb39831e

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Comment @coderabbitai help to get the list of available commands.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds fixture-backed Formation checklists and an auditor-only Formations queue, dark-launched behind formation-enabled.

Changes:

  • Adds Formation contracts, fixtures, readiness logic, and BFF endpoints.
  • Adds checklist, drawer, queue, filtering, and action UIs.
  • Adds authorization guards plus unit and Playwright coverage.

Reviewed changes

Copilot reviewed 73 out of 73 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
packages/shared/src/utils/project-stage.utils.ts Detects Formation stages.
packages/shared/src/utils/project-stage.utils.spec.ts Tests stage detection.
packages/shared/src/utils/index.ts Exports Formation utilities.
packages/shared/src/utils/formation-checklist.utils.ts Derives readiness and sections.
packages/shared/src/utils/formation-checklist.utils.spec.ts Tests checklist utilities.
packages/shared/src/interfaces/persona-detection.interface.ts Adds auditor response state.
packages/shared/src/interfaces/index.ts Exports Formation contracts.
packages/shared/src/interfaces/formation.interface.ts Defines Formation domain contracts.
packages/shared/src/interfaces/formation-checklist.interface.ts Defines checklist UI contracts.
packages/shared/src/interfaces/components.interface.ts Adds dialog and accent types.
packages/shared/src/interfaces/access-check.interface.ts Adds provisional gate-writer access.
packages/shared/src/constants/index.ts Exports Formation constants.
packages/shared/src/constants/formation.constants.ts Adds labels, severities, and defaults.
packages/shared/src/constants/feature-flags.constants.ts Adds Formation feature flag.
apps/lfx-one/src/server/services/persona-detection.service.ts Detects root auditor access.
apps/lfx-one/src/server/services/formation-store.service.ts Stores fixture mutations in memory.
apps/lfx-one/src/server/services/formation-store.service.spec.ts Tests fixture storage behavior.
apps/lfx-one/src/server/services/formation-item-access.service.ts Computes temporary completion access.
apps/lfx-one/src/server/services/formation-item-access.service.spec.ts Tests completion access.
apps/lfx-one/src/server/services/formation.service.ts Implements fixture Formation operations.
apps/lfx-one/src/server/services/formation.service.spec.ts Tests Formation operations.
apps/lfx-one/src/server/server.ts Registers Formation routes.
apps/lfx-one/src/server/routes/formations.route.ts Defines checklist and queue endpoints.
apps/lfx-one/src/server/middleware/require-auditor.middleware.ts Guards queue endpoints.
apps/lfx-one/src/server/middleware/require-auditor.middleware.spec.ts Tests auditor middleware.
apps/lfx-one/src/server/helpers/formation-fixture.helper.ts Generates Formation fixture data.
apps/lfx-one/src/server/helpers/formation-fixture.helper.spec.ts Validates fixture links and types.
apps/lfx-one/src/server/helpers/formation-backend.helper.ts Defines future backend swap point.
apps/lfx-one/src/server/controllers/formation.controller.ts Handles Formation API requests.
apps/lfx-one/src/app/shared/services/persona.service.ts Exposes auditor signal.
apps/lfx-one/src/app/shared/services/persona.service.spec.ts Updates persona fixtures.
apps/lfx-one/src/app/shared/services/formation.service.ts Adds Formation HTTP client.
apps/lfx-one/src/app/shared/services/formation.service.spec.ts Tests Formation HTTP calls.
apps/lfx-one/src/app/shared/guards/formations-queue-auditor.guard.ts Guards queue navigation.
apps/lfx-one/src/app/shared/guards/formations-queue-auditor.guard.spec.ts Tests queue authorization.
apps/lfx-one/src/app/shared/guards/formation-enabled.guard.ts Guards queue by feature flag.
apps/lfx-one/src/app/shared/guards/formation-enabled.guard.spec.ts Tests flag behavior.
apps/lfx-one/src/app/shared/components/tag/tag.component.ts Supports accent severity.
apps/lfx-one/src/app/shared/components/tag/tag.component.html Renders violet accent tags.
apps/lfx-one/src/app/shared/components/reason-prompt-dialog/reason-prompt-dialog.component.ts Implements required-reason dialog.
apps/lfx-one/src/app/shared/components/reason-prompt-dialog/reason-prompt-dialog.component.spec.ts Tests reason dialog behavior.
apps/lfx-one/src/app/shared/components/reason-prompt-dialog/reason-prompt-dialog.component.html Renders reason form.
apps/lfx-one/src/app/shared/components/button/button.component.ts Exposes loading state for tests.
apps/lfx-one/src/app/modules/formations/formations-queue/formations-queue.component.ts Orchestrates queue actions and loading.
apps/lfx-one/src/app/modules/formations/formations-queue/formations-queue.component.scss Adds queue stylesheet shell.
apps/lfx-one/src/app/modules/formations/formations-queue/formations-queue.component.html Renders queue states.
apps/lfx-one/src/app/modules/formations/components/formations-table/formations-table.component.ts Implements filtering and row actions.
apps/lfx-one/src/app/modules/formations/components/formations-table/formations-table.component.scss Adds table stylesheet shell.
apps/lfx-one/src/app/modules/formations/components/formations-table/formations-table.component.html Renders Formation table.
apps/lfx-one/src/app/modules/dashboards/project-dashboard/project-dashboard.component.ts Gates checklist on stage and flag.
apps/lfx-one/src/app/modules/dashboards/project-dashboard/project-dashboard.component.html Embeds deferred checklist.
apps/lfx-one/src/app/modules/dashboards/components/formation-readiness-strip/formation-readiness-strip.component.ts Computes readiness labels.
apps/lfx-one/src/app/modules/dashboards/components/formation-readiness-strip/formation-readiness-strip.component.scss Adds readiness stylesheet shell.
apps/lfx-one/src/app/modules/dashboards/components/formation-readiness-strip/formation-readiness-strip.component.html Renders readiness strip.
apps/lfx-one/src/app/modules/dashboards/components/formation-item-drawer/formation-item-drawer.component.ts Loads and edits drawer data.
apps/lfx-one/src/app/modules/dashboards/components/formation-item-drawer/formation-item-drawer.component.scss Adds drawer stylesheet shell.
apps/lfx-one/src/app/modules/dashboards/components/formation-item-drawer/formation-item-drawer.component.html Renders item drawer.
apps/lfx-one/src/app/modules/dashboards/components/formation-checklist-section/formation-checklist-section.component.ts Orchestrates checklist interactions.
apps/lfx-one/src/app/modules/dashboards/components/formation-checklist-section/formation-checklist-section.component.scss Adds checklist stylesheet shell.
apps/lfx-one/src/app/modules/dashboards/components/formation-checklist-section/formation-checklist-section.component.html Renders checklist states and sections.
apps/lfx-one/src/app/modules/dashboards/components/formation-checklist-row/formation-checklist-row.component.ts Maps item actions and statuses.
apps/lfx-one/src/app/modules/dashboards/components/formation-checklist-row/formation-checklist-row.component.scss Adds row stylesheet shell.
apps/lfx-one/src/app/modules/dashboards/components/formation-checklist-row/formation-checklist-row.component.html Renders checklist rows.
apps/lfx-one/src/app/app.routes.ts Adds guarded queue route.
apps/lfx-one/e2e/helpers/formation-checklist.helper.ts Adds checklist E2E setup.
apps/lfx-one/e2e/helpers/formation-api-mock.helper.ts Mocks Formation endpoints.
apps/lfx-one/e2e/formations-queue.spec.ts Covers queue behavior.
apps/lfx-one/e2e/formations-queue-robust.spec.ts Covers queue structure.
apps/lfx-one/e2e/formation-checklist.spec.ts Covers checklist behavior.
apps/lfx-one/e2e/formation-checklist-robust.spec.ts Covers checklist structure.
apps/lfx-one/e2e/fixtures/mock-data/index.ts Exports Formation fixtures.
apps/lfx-one/e2e/fixtures/mock-data/formation.mock.ts Defines Formation E2E fixtures.
apps/lfx-one/e2e/fixtures/mock-data/formation-item.mock.ts Defines item and activity fixtures.

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

</div>
} @else if (item(); as currentItem) {
<div class="flex flex-col gap-2">
@if (currentItem.status !== 'done') {
Comment on lines +104 to +106
const control = page.getByTestId(`formation-checklist-row-manual-${manualItem.uid}`);
await expect(control).toBeAttached();
expect(await control.evaluate((el) => el.tagName)).toBe('BUTTON');
Comment thread apps/lfx-one/e2e/formations-queue-robust.spec.ts Outdated
segments: items.map((item) => item.status),
totalItems: items.length,
counts,
isActivating: totalGatingItems > 0 && openGatingItems === 0,

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 73 out of 73 changed files in this pull request and generated 1 comment.

Suppressed comments (7)

Previously missed (6) — in code that hasn't changed since the last review.

apps/lfx-one/src/server/helpers/formation-fixture.helper.ts:20

  • This placeholder no longer matches the linked #1959 seed: the landed template has 17 items and 4 gates, and removed Draft project record, Intake submitted, PMO-owned rows, and Formation sets Active. This fixture still has 18 items/6 gates and those retired rows, so the checklist and queue show the wrong content and readiness counts. Update this fixture and the E2E mirrors to the current seed, preferably from one shared definition.
    apps/lfx-one/src/server/routes/formations.route.ts:25
  • The current GH-1958 scope explicitly defers request-type actions to #1963 and says request rows are status-only in Epic 1. This route exposes a working request mutation that moves an item to waiting_on_partner, with matching client controls, so it ships an out-of-scope API and state transition. Remove the request endpoint/action path and render those rows as status-only until #1963 supplies the real request contract.
    apps/lfx-one/src/server/services/formation.service.ts:31
  • The PR describes isFormationServiceLive() as the single swap point used by every public method, but only getProjectFormation calls it. Flipping this function later would leave the queue and all item reads/writes on fixtures, creating a mixed live/fixture backend. Route every public operation through the switch (or remove the single-swap claim and introduce an explicit backend abstraction).
    apps/lfx-one/src/server/services/formation.service.ts:56
  • This endpoint generates and seeds a formation for any readable project because it never verifies project.stage; the client-side dashboard gate is bypassable by calling the API directly. An Active, Prospect, or Archived project slug therefore receives a fabricated engaged formation. Reject non-Formation stages with isFormationStage(project.stage) before calling generateMockFormation.
    apps/lfx-one/src/server/services/formation.service.ts:257
  • subprojects includes rows whose type is plain project, but the stat card labels this value as “subprojects.” The queue therefore reports an incorrect entity breakdown whenever a project row exists (the fixture includes one). Add a separate projects count or label the combined metric as projects/subprojects instead of reclassifying projects.
    packages/shared/src/utils/formation-checklist.utils.ts:89
  • announcement_date is an ISO date, but this computes elapsed 24-hour periods from the current instant. As a result, today’s date can become “1 day ago” after noon UTC, and a date later today can become “1 day”; the added tests even encode this asymmetric boundary. Compare normalized calendar dates instead so the countdown agrees with the displayed announcement date.

packages/shared/src/utils/formation-checklist.utils.ts:48

  • The Activating calculation does not match the linked #1957 contract: Activating requires every gating item to be done and an announcement date to be set. Because skipped gates are excluded from openGatingItems and this function has no announcement-date input, a done+skipped checklist can report Activating with no date. Keep skipped gates open and include the announcement date (or consume Formation.is_activating); update the mirrored calculation in formation.service.ts and its tests too.

Comment on lines +42 to +43
<th class="w-[12%]">Progress</th>
<th class="w-[14%]">Announcement</th>
manishdixitlfx added a commit that referenced this pull request Sep 4, 2026
…H-2163)

Six amendments to the canonical formation types, per the Epic 1
architecture review and two product calls (4 Sep):

- FormationItemStatus: drop waiting_on_partner (a UI wording of
  blocked, not a stored value), add blocked and awaiting_acceptance
  (an assignee-completed item stays on Pending Actions until the
  formation team accepts it; only done counts toward readiness).
- FormationSubStage: drop activating — it's derived readiness
  (Formation.is_activating), not a PCC sub-stage, and belongs in a
  separate "Gates cleared" badge, never the Stage column.
- can_complete: keep the field, rewrite the comment — there is no
  gate_writer relation; gating is a property of the item, guarded by
  project write permission + is_gating.
- FormationEntityType: rename 'subproject' -> 'child_project'
  (user-facing term is "child project"); documented as a display
  taxonomy to derive later (TODO #1958). FormationQueueTiles.subprojects
  renamed to child_projects to match.
- is_gating doc: "Gates Active" -> "Required for Active", the agreed
  4 Sep vocabulary for the chip/strip/Me-lens marker/email.
- FormationLead -> FormationUser: not an Epic 2 "formation lead"
  record (#1992); it's a plain user reference.

Adds a runtime regression guard on FORMATION_SUB_STAGE_LABELS' key set
in formation.enum.spec.ts.

Every consumer of these types lives on feat/GH-1958-formation-checklist-queue
(#2033) — verified read-only, not touched here. feat/issue-1959 (#2035)
has zero references to any of the six.

Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org>
manishdixitlfx added a commit that referenced this pull request Sep 4, 2026
…H-2163)

Six amendments to the canonical formation types, per the Epic 1
architecture review and two product calls (4 Sep):

- FormationItemStatus: drop waiting_on_partner (a UI wording of
  blocked, not a stored value), add blocked and awaiting_acceptance
  (an assignee-completed item stays on Pending Actions until the
  formation team accepts it; only done counts toward readiness).
- FormationSubStage: drop activating — it's derived readiness
  (Formation.is_activating), not a PCC sub-stage, and belongs in a
  separate "Gates cleared" badge, never the Stage column.
- can_complete: keep the field, rewrite the comment — there is no
  gate_writer relation; gating is a property of the item, guarded by
  project write permission + is_gating.
- FormationEntityType: rename 'subproject' -> 'child_project'
  (user-facing term is "child project"); documented as a display
  taxonomy to derive later (TODO #1958). FormationQueueTiles.subprojects
  renamed to child_projects to match.
- is_gating doc: "Gates Active" -> "Required for Active", the agreed
  4 Sep vocabulary for the chip/strip/Me-lens marker/email.
- FormationLead -> FormationUser: not an Epic 2 "formation lead"
  record (#1992); it's a plain user reference.

Adds a runtime regression guard on FORMATION_SUB_STAGE_LABELS' key set
in formation.enum.spec.ts.

Every consumer of these types lives on feat/GH-1958-formation-checklist-queue
(#2033) — verified read-only, not touched here. feat/issue-1959 (#2035)
has zero references to any of the six.

Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org>
Copilot AI review requested due to automatic review settings September 4, 2026 23:01

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 75 out of 75 changed files in this pull request and generated 7 comments.

// fixture generator's return shape already matches Formation/FormationItem[], so nothing
// downstream of this branch needs to change.
if (!isFormationServiceLive()) {
const project = await this.projectService.getProjectById(req, uid, false);
Comment on lines +120 to +122
const canComplete = await formationItemAccessService.canComplete(req, item);
const nextStatus: FormationItemStatus = item.is_gating && !canComplete ? 'awaiting_acceptance' : 'done';
const updated: FormationItem = { ...item, status: nextStatus, skip_reason: null, notes: notes ?? item.notes, updated_at: new Date().toISOString() };
Comment on lines +161 to +167
public async requestFormationItem(req: Request, itemUid: string): Promise<FormationItem> {
const item = await this.getFormationItemOrThrow(req, itemUid);
await this.assertItemProjectWriteAccess(req, item);
// Same gate as complete/skip: `request` also changes `status`, so a gating item's status must
// not be movable through this action by a caller `complete`/`skip` would deny.
await this.assertCanComplete(req, item, 'request_formation_item');
const updated: FormationItem = { ...item, status: 'blocked', updated_at: new Date().toISOString() };
Comment on lines +399 to +403
try {
await this.projectService.getProjectById(req, formation.parent_project_uid, false);
} catch (error) {
denyNotFound(error);
}
Comment on lines +354 to +358
// A skipped gating item is resolved, not open — skipFormationItem is the designed escape hatch
// for a gate the project can't complete; treating it as still-open would make isActivating
// permanently unreachable for any formation that ever uses it.
const openGatingItems = gatingItems.filter((item) => item.status !== 'done' && item.status !== 'skipped');
const isActivating = gatingItems.length > 0 && openGatingItems.length === 0;
Comment on lines +128 to +134
* Whether the caller may complete this row — response-only, enrichment output. There is no
* `gate_writer` relation: gating is a property of the item, not the person. The guard is the
* project write permission plus this item's `is_gating` flag, checked service-side.
* TODO(#1957): fabricated today by `FormationItemAccessService.canComplete`; swap for the real
* service-side check once it ships.
*/
can_complete: boolean;
}
if (item.is_gating) {
totalGatingItems += 1;
if (item.status !== 'done' && item.status !== 'skipped') openGatingItems += 1;
Add the Formation/FormationItem/FormationActivity/FormationTemplate
domain types, the FORMATION_ENABLED_FLAG dark-launch flag, gate_writer
and isAuditor permission fields, the readiness-summary derivation
util, and a TagComponent violet severity for the Waiting on partner
status. First slice of the checklist section + Formations queue,
Epic 1 scope (epic #1965).

Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org>
#1958)

Add the Formation Checklist / Formations queue BFF service, controller
and routes, all fixture-backed ahead of the real lfx-v2-formation-service
(#1957). Add a real root-scoped auditor FGA check (checkRootAuditor,
requireAuditor middleware, isAuditor persona field) to guard the queue,
and a fabricated gate_writer stand-in (FormationItemAccessService) for
per-item gating completion. Also fixes review findings from the prior
commit: isFormationStage now prefix-matches instead of a fixed stage
list, deriveFormationReadinessSummary guards against an unmodeled
status value, and FormationTemplateItem's redundant status_only flag
is dropped in favor of action === 'status_only'.

Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org>
Fix two review findings from the prior commit: /formation-items/:uid
had no project-scoped authorization at all (any authenticated user
could read/mutate any project's checklist items via a guessable uid),
and requestFormationItem/updateFormationItem could move a gating
item's status without the gate_writer check complete/skip already
enforce. Both are now centralized in FormationService so a future
mutating method can't omit them.

Also: move the in-memory fixture store out of the (supposed-to-be-pure)
formation-fixture helper into formation-store.service.ts; fix mine
comparing an email against a username; fix a status-tally prototype-chain
gap (in vs hasOwnProperty); drop a dead ternary and the now-fully-redundant
status_only field; validate updateFormationItem's patch fields; and add
spec coverage for the guards, formation-item-access.service, and the
two fixes above.

Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org>
Every complete/skip/request/update endpoint was only gated on project
viewer visibility (assertItemProjectAccess, access=false) — any user
who could merely see a project could mutate its non-gating checklist
items. Add assertItemProjectWriteAccess (getProjectById with
access=true, requires .writer) and call it from all four mutating
methods before the gate_writer check. requestFormationItem also now
recomputes formation readiness after moving an item's status, matching
complete/skip.

Add resetFormationStoreForTests() so specs no longer depend on
inter-test uid uniqueness, and extend formation.service.spec.ts to
cover the new write-access gate, the readiness recompute, and the
assignee-changed activity dedupe.

Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org>
Build the Angular layer against the fixture-backed BFF: an embedded
checklist section on the project overview page (readiness strip, two
template sections, row actions, item drawer with notes/links/sub-items
/history/assignee/due-date editors, skip-with-reason dialog), and the
Formations queue page at foundation/formations (stat tiles, sub-stage
filter pills + search, table, Accept/Decline). Both gated behind
FORMATION_ENABLED_FLAG; the checklist section also only renders for a
project in a Formation - * stage. The queue route omits
projectQueryParamGuard and has no :id child, per the LF-root-only,
no-nested-views scope.

Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org>
…1958)

Fix the item drawer's stuck-loading bug (a failed fetch reused the
"still loading" sentinel, leaving the spinner spinning forever with no
error surfaced); add scheme-validated hrefs (safeActionHref/safeLinks)
so an unvalidated API-sourced URL can never bind into [href]; add
noopener,noreferrer everywhere target="_blank" is used, including a
popup-blocked fallback on Accept; drive the checklist's two sections
from template.sections instead of two hardcoded keys, so a template
revision can't silently drop items; gate the provisionable/request row
actions on can_complete and terminal status, matching the drawer;
separate "item completed" (closes the drawer) from "item metadata
saved" (keeps it open) so a notes/assignee save no longer bounces the
user out; log every previously-silent catchError; move
FormationDrawerData/FormationsQueueFilterState/FormationChecklistPageState
into @lfx-one/shared/interfaces; replace the table's constructor
effect() (which double-fired the initial fetch) with direct emission
from user interaction; and consolidate the two identical
skip/decline-reason dialogs into one shared ReasonPromptDialogComponent.

Add e2e fixtures (formation.mock.ts, formation-item.mock.ts),
FormationApiMockHelper, and a first checklist-section e2e spec —
written but not run live in this session (dev server port already
held by a sibling worktree; see final summary).

Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org>
Pins the Angular FormationService's URLs, HTTP verbs, and request
bodies for all nine BFF endpoints — the piece most repeatedly flagged
as untested across the review passes on this branch, and the cheapest
to lock down (no TestBed component harness needed).

Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org>
Fix stale drawer data after a metadata save (item()/history() now
refetch via a reload trigger instead of only on visible toggle),
validate deep_link_url before window.open on Accept, bucket items
with an unrecognized section_key into a fallback section instead of
dropping them, disable the link/status_only row action with a
tooltip when the href is missing/unsafe, move ReasonPromptDialog's
data/result types to shared interfaces, and add data-testid to the
remaining row action buttons.

Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org>
Tag the drawer's data-fetch trigger so a post-save reload no longer
reuses the open-transition loading/error signals (was blanking the
drawer to a spinner, or an error screen, right after a successful
save); log orphaned section_key items once per fetched response
instead of on every renderedSections recomputation; move the orphan
section's key/title into shared constants; replace the disabled
link-action button's hover-only tooltip with visible text, since a
disabled button isn't focusable; and make the checklist's error-state
Retry test actually exercise recovery instead of only asserting the
button renders, plus add coverage for the new disabled-link fallback.

Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org>
Fix the e2e disabled-link test's toBeDisabled() assertion — it
targeted the <lfx-button> host instead of the native <button> two
levels down, so it could never pass; make the Retry-recovery test
assert on the readiness strip instead of the always-mounted section
wrapper, since the previous assertion passed on the retry click's own
transient loading state. Extend the visible-text-over-tooltip fix to
the provisionable/request row actions, which had the same
disabled-button-isn't-focusable gap. Move RenderedSection and the
empty-response/empty-drawer-data sentinels into @lfx-one/shared, and
extract the section/orphan-item grouping into two pure, unit-tested
shared utilities so the checklist section's computed and its
orphan-key logger can't disagree on what counts as orphaned.

Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org>
Type the row's gated-action *ngTemplateOutlet contexts at their
shared-constants definition site (FORMATION_GATED_ROW_ACTIONS)
instead of as inline object literals, so a bad severity/label typo
fails the build instead of silently shipping an unstyled button; pass
the full data-testid prefix through template context rather than a
bare suffix, so it stays greppable from the literal that produces it.
Move the Formations queue's EMPTY_TILES/EMPTY_RESPONSE sentinels into
@lfx-one/shared, matching the same move already made for the
checklist section and drawer earlier in this branch.

Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org>
Fix an HTML-entity-escaped <ng-template> reference in a JSDoc block
(rendered literally in IDE tooltips); narrow FormationRowActionConfig's
severity from ButtonProps['severity'] (admits undefined) to the
non-optional ButtonSeverity, so the config's `satisfies` check
actually rejects a missing/mistyped severity. Give the row's
link/status_only action the same typed-context treatment already
applied to provisionable/request (FORMATION_LINK_ROW_ACTIONS), and
move the row's remaining STATUS_LABEL/STATUS_SEVERITY lookups and the
queue's tile sentinel into @lfx-one/shared, mirroring
POLL_STATUS_LABELS/POLL_STATUS_SEVERITY and adding the as const that
was missing from the earlier move.

Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org>
Turn the Formations queue's empty-response sentinel into a factory
(emptyFormationsQueueResponse) instead of a shared const object —
the as const on its nested tiles didn't survive being embedded in
the mutably-typed FormationsQueueResponse, so the toSignal
initialValue and the catchError fallback were aliasing one mutable
object. Reword the severity-narrowing JSDoc, which overstated what
changed: a mistyped literal already failed the satisfies check before
this narrowing — it only newly rejects an explicit undefined/null.

Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org>
Convert FORMATION_EMPTY_DRAWER_DATA into a factory
(createEmptyFormationDrawerData), the same mutable-singleton hazard
the queue's empty-response sentinel was just converted for — its
history array backed both a toSignal initialValue and a catchError
fallback. Rename emptyFormationsQueueResponse to
createEmptyFormationsQueueResponse to match this constants
directory's verb-prefix convention for factory exports.

Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org>
The previous fix only closed section-to-drawer: a row action or skip
now blocks the drawer's Mark complete/Save. But the drawer's own
completing/savingDetails were invisible to the section, so closing the
drawer mid-write (or leaving it open) and then firing a row action for
the same item could still issue two concurrent writes with no guard.

The drawer now emits writeStarted/writeEnded around Mark complete and
Save so the section can register the write in the same
submittingItemUids map that already guards row actions and skip,
closing the loop in both directions.

Also fixes the regression the previous commit's general-reviewer round
flagged: binding the drawer's Skip button loading state to the same
broad mutationInFlight signal spun it for a row action on a different
kind of mutation entirely. submittingItemUids is now a uid -> 'row' |
'skip' | 'drawer' map instead of a Set, so the drawer gets two
precisely-scoped inputs: mutationInFlight (any mutation, for the
shared busy()/disabled gate) and skipInFlight (skip only, for the
Skip button's own loading spinner).

Trade-off, not fixed here: no unit spec pins this guard chain
(completing/savingDetails/mutationInFlight/skipInFlight and the
writeStarted/writeEnded round trip). These formation components have
no existing .spec.ts precedent to extend, and scaffolding a new
TestBed harness for this shape was judged disproportionate to add on
top of an already large branch; e2e coverage exercises the drawer's
happy paths but not this specific concurrency contract.

Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org>
writeStarted/writeEnded fired with no payload, so the section read its
own current drawerItemUid() to know which entry to register/release.
That's wrong the moment the drawer switches to a different item (or
closes — onClose() has no busy() guard, and p-drawer's mask/ESC
dismiss bypass it too) before the original write's response lands: the
write's real uid never gets cleared (that row's action button spins
forever, and reopening its drawer permanently disables every button in
it), while whatever uid the drawer now points at gets its own guard
dropped early, re-opening the exact double-write race this guard
chain exists to close.

writeStarted/writeEnded now carry the uid the write was actually
issued for, captured in the handler's own closure at call time.
onDrawerWriteEnded is also now kind-aware — it only retires an entry
tagged 'drawer', so it can never clear a 'row'/'skip' write's guard out
from under it if beginSubmitting had silently no-op'd the drawer's own
registration.

Also: rewords the row's submitting doc comment, which still described
provisionable/request only after 'skip'/'drawer' kinds were folded
into the same signal; and moves beginSubmitting/endSubmitting below
initResponse so private initializers and private helpers stay in two
contiguous groups per component-organization.md's ordering.

Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org>
…egressions

Three commits on this branch (bf20c85, a61bf21, 2cc5ea7) fixed the same
write-guard chain with zero automated coverage, leaving the next
refactor of submittingItemUids free to silently re-break it. Adds an
e2e case that holds two different items' complete requests open at
once, closes the drawer on the first mid-write, starts the second from
a different item, then releases the first response and asserts the
second item's button does not prematurely re-enable — the exact
symptom of resolving the uid from the section's current drawerItemUid()
instead of the uid the write was actually issued for.

Could not run this test locally (no TEST_USERNAME/TEST_PASSWORD
configured in this environment, so global-setup.ts can't produce
playwright/.auth/user.json) — verified via yarn lint (typescript-eslint
project-aware parsing, 0 errors) and by tracing the signal flow by
hand against the fixed component code.

Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org>
…ocal state

The general-code-reviewer traced the previous commit's e2e test by hand
and found it could not have passed, for reasons beyond the test file
itself:

1. onDrawerItemChanged/onSkipRequested's success handlers closed the
   drawer unconditionally. A completed write for item A landing while
   the drawer has since been switched to show item B would incorrectly
   close B's drawer too — both now gate the close on the completed
   item's uid actually matching drawerItemUid().

2. completing/savingDetails live on the drawer *component instance*,
   which is reused across every item the drawer ever opens — they were
   never reset on switching items. A write still pending for a
   previously-open item left Mark complete/Save disabled for every
   item opened afterward, until that stale request eventually
   resolved (or forever, if it never did). Now reset on every genuine
   open; mutationInFlight/skipInFlight (section-owned, keyed by the
   current item's own uid) still correctly reflect that item's busy
   state regardless.

Also rewrites the e2e test the same reviewer round found three
concrete defects in: it clicked an item seeded can_complete: false
(permanently disabled, so the click could never succeed); it asserted
on the drawer after a close that (pre-fix #1 above) would have already
hidden it; and it released a held mock response without confirming
Playwright's route handler had actually registered the resolver first,
which could make the test pass for the wrong reason. Items are now
derived from the fixture (can_complete && status !== 'done', with an
explicit throw if fewer than two qualify) instead of hard-coded uids,
the mock 404s loudly on a fixture-uid miss instead of spreading
undefined into a partial body, and releasing a held response now polls
for the resolver's registration and waits on the real network response
instead of a fixed sleep.

Traced by hand against the current component code (including a dry
run through the interleaved-writes sequence the test constructs) and
verified with yarn lint/format/build/test, but still not run against a
live browser — TEST_USERNAME/TEST_PASSWORD are not configured in this
environment, so global-setup.ts cannot produce
playwright/.auth/user.json and no e2e spec can execute here regardless
of which project or test is selected.

Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org>
completing/savingDetails were instance-level booleans, but this drawer
component instance is reused across every item it opens and writes
can overlap across items. A's finalize clearing a shared boolean could
wrongly clear B's own still-in-flight write's flag (only cosmetic
today — busy()'s disabled state survives via the section-owned,
correctly per-uid mutationInFlight — but the same defect class the
last two commits closed elsewhere). onSaveDetails' reload$ had the
identical bug: it read the drawer's current itemUid() instead of the
uid the save was actually for, so switching items mid-save could
refetch and overwrite the wrong item's form.

completingUid/savingDetailsUid now track which uid each write belongs
to; completing()/savingDetails() derive against the currently-open
item, so switching items scopes each flag correctly without an
explicit reset (the reset added in the previous commit is now
redundant and removed). Each finalize only clears its own uid's flag,
and the reload only fires if the drawer still shows the item the save
was for.

Also fixes the e2e regression test's tail: releasing item B's held
response is a real (uid-matching) completion, so the uid-gated close
from the previous commit correctly closes the drawer there too — the
test was still asserting the drawer/button stayed present and would
have failed on that assertion. Rewrote it to assert the close, then
reopen item A directly to confirm its own guard was retired. Also
deletes a pendingResolvers entry once released, so a second write for
the same uid can't poll-pass against an already-fired resolver.

Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org>
completingUid/savingDetailsUid were single string|null slots, so two
overlapping drawer-initiated writes across different items (Mark
complete on A, then Mark complete on B before A's response lands)
still clobbered each other: B's click overwrote completingUid to B's
uid, and A's finalize later cleared it — wrongly dropping B's own
still-in-flight flag. Impact was cosmetic only (busy()'s disabled
state survives via the section-owned mutationInFlight either way,
which is what the e2e test added two commits ago actually exercises),
but it's the same defect class those commits closed elsewhere, just
narrowed rather than eliminated.

completingUids/savingDetailsUids are now sets, keyed by uid, mirroring
the section's own submittingItemUids map — add on start, delete on
finalize, no uid-equality guard needed since a Set delete of an
already-absent key is already a no-op.

Also corrects the e2e test's comment to say what it actually proves:
the section-level guard chain (writeStarted/writeEnded resolving to
the right uid), not the drawer-internal completingUids isolation this
commit adds — the button's [disabled] there is driven by
mutationInFlight regardless of the drawer's own tracking, so that
assertion would already hold either way.

Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org>
…isolation

Two comments from the previous commit misstated which half of the old
single-slot code was actually broken: the finalize's uid-guard already
protected a second write's tracking from being cleared by an earlier
write's finalize. The real defect was earlier — starting a second
write immediately overwrote the first write's own tracked uid via
completingUid.set(...), silently losing that first write's [loading]
isolation while it was still genuinely in flight. Corrected both the
component's JSDoc and the e2e test's inline comment to describe that.

Also extracts beginWrite/endWrite helpers so the add/delete Set logic
isn't duplicated across both finalize blocks, mirroring the section's
own beginSubmitting/endSubmitting pair.

The e2e test previously couldn't discriminate the drawer-internal
completingUids/savingDetailsUids fix from the section-level guard,
since [disabled] is driven by busy() and mutationInFlight (section-
owned) stays true either way. Rewrote it to close B and reopen A while
both writes are still pending, and assert on PrimeNG's loading-spinner
icon specifically (the one signal that's actually per-action) rather
than only [disabled] — with the old single-slot code, starting B's
write would have silently dropped A's own spinner at exactly this
point, even though A's write is still genuinely in flight.

Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org>
…internals

The formation drawer write-guard regression test reached into PrimeNG's
internal spinner-icon markup ([data-p-icon="spinner"]) to assert
per-button loading state. Add a data-loading host attribute reflecting
ButtonComponent's own loading() input so specs can assert against an
app-owned attribute instead, per testing-best-practices.md's element
selection priority.

Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org>
Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org>
Move formation-item-drawer's visible model signal ahead of inputs per
the documented drawer component ordering, and add structural e2e
coverage for the checklist section and formations queue.

Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org>
Flagged by post-commit review: FormationApiMockHelper was imported
but never referenced in formation-checklist-robust.spec.ts.

Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org>
A single failed can_complete check (e.g. a transient checkLFStaff
error) no longer fails the entire checklist/queue read; the failed
item is logged and dropped instead.

Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org>
Log the rejected Error via the err field instead of collapsing to
error.message, correct the JSDoc's queue reference (only the
checklist path calls enrichItems), and add a unit test for the
allSettled drop path.

Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org>
…1958)

Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org>
…ary (GH-1958)

Migrate onto the canonical shared Formation types from GH-2163/PR #2175
and close three product gaps: every checklist row is now actionable via
a status-control menu and overflow menu (assign/due date/skip), the new
awaiting_acceptance status is handled end-to-end (Accept flow, correct
counts, blue segment color), and gate-related copy is unified to
"Required for Active" across the row, drawer, readiness strip, and
queue table.

Refs #1958

Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org>
Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org>
- Trim the formation page's own duplicate heading now that the checklist
  section renders only on its dedicated route.
- Add a gotoProjectFormation e2e helper and repoint all formation
  checklist specs at the new route; rewrite the two negative gating
  tests to assert the guard's redirect instead of an absent testid on a
  page that no longer hosts the section.
- Fix the entry card's inverted open/total gating count.
- Drop dead catchError wrappers around ProjectService.getProject, which
  already swallows its own errors and returns null.
- Add missing specs for FormationEntryCardComponent and
  FormationPageComponent.

Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org>
…1958)

updateFormationItemStatus overwrote an item's persisted notes with the
block reason instead of filing it as activity metadata, and let any
project writer reopen a gating item off done/awaiting_acceptance
without the gate_writer check its sibling mutations already enforce.
Gate the row's status menu the same way so it doesn't offer an action
the server now rejects. Also moves the drawer's two sub-item status
lookups into a precomputed view model, since templates may only read
signals/pipes.

Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org>
Copilot AI review requested due to automatic review settings September 7, 2026 02:36
@manishdixitlfx
manishdixitlfx force-pushed the feat/GH-1958-formation-checklist-queue branch from a27107d to 700f43b Compare September 7, 2026 02:36

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 86 out of 86 changed files in this pull request and generated 6 comments.

Comment on lines +60 to +67
const project = await this.projectService.getProjectById(req, uid, false);
const { formation, items } = generateMockFormation({
projectUid: uid,
projectSlug: project.slug,
projectName: project.name,
parentProjectUid: project.parent_uid || null,
stage: project.stage,
});
Comment on lines +27 to +35
@if (item().owner_team; as ownerTeam) {
<span [attr.data-testid]="'formation-checklist-row-owner-chip-' + item().uid">
<lfx-tag [value]="ownerTeam" severity="secondary" />
</span>
}
@if (item().is_gating) {
<span [attr.data-testid]="'formation-checklist-row-gates-active-chip-' + item().uid">
<lfx-tag value="Required for Active" severity="warn" />
</span>
Comment on lines +145 to +148
next: (updated) => {
this.itemChanged.emit(updated);
this.messageService.add({ severity: 'success', summary: 'Marked done', detail: `"${updated.title}" is done.` });
},
Comment on lines +207 to +215
const blockNote = nextStatus === 'blocked' && typeof note === 'string' ? note : null;
const updated: FormationItem = {
...item,
status: nextStatus,
skip_reason: null,
updated_at: new Date().toISOString(),
};
putStoredItem(updated);
this.recordActivity(req, updated, 'item_reopened', `moved "${updated.title}" to ${nextStatus}`, blockNote !== null ? { note: blockNote } : null);
Comment on lines +125 to +133
/** `FormationReadinessStripComponent`'s per-segment fill color, keyed by item status. Not `done` must never read green. */
export const FORMATION_ITEM_SEGMENT_COLORS = {
done: 'bg-emerald-600',
in_progress: 'bg-amber-500',
blocked: 'bg-red-500',
awaiting_acceptance: 'bg-blue-500',
not_started: 'bg-gray-200',
skipped: 'bg-gray-400',
} as const satisfies Record<FormationItemStatus, string>;
Add a "Formations" nav item to the foundation lens sidebar, guarded by
the same isRootWriter||isAuditor condition formationsQueueAuditorGuard
enforces on the route itself (that guard is CanActivate-only and needs
Router.createUrlTree for its redirect, so it can't be invoked directly
as a reactive Signal — the condition is reproduced instead of duplicated).
A second insertion point outside hasFullFoundationAccess() covers an
auditor-only user who has no other foundation grant, mirroring the
existing marketing-only-FGA special case. A new foundationAuditorBootstrap
signal probes refreshEnrichedPersonas() whenever an authenticated user is
on the foundation lens, so isAuditor/isRootWriter are populated even when
the user never passed through the Me-lens dashboard first.

Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org>
Copilot AI review requested due to automatic review settings September 7, 2026 22:17

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 86 out of 86 changed files in this pull request and generated no new comments.

Suppressed comments (5)

Previously missed (3) — in code that hasn't changed since the last review.

apps/lfx-one/src/app/modules/dashboards/components/formation-checklist-row/formation-checklist-row.component.ts:115

  • This disables the only completion action when can_complete is false, but the BFF explicitly accepts that request and moves a gating item to awaiting_acceptance. As a result, an ordinary project writer cannot submit completed work for formation-team acceptance from the row menu. Please model submission permission separately from acceptance permission and leave the submit action enabled for eligible writers.
    apps/lfx-one/src/app/modules/dashboards/components/formation-item-drawer/formation-item-drawer.component.html:58
  • The drawer also disables completion when can_complete is false, making the BFF's intended awaiting_acceptance branch unreachable from this surface. A project writer who cannot accept a gate still needs to submit it for acceptance; use a distinct submission capability instead of treating can_complete as permission to invoke the complete endpoint at all.
    apps/lfx-one/src/server/helpers/formation-fixture.helper.ts:81
  • The seeded checklist no longer matches the canonical Epic 1 template: it still includes the removed Draft project record and Intake form rows, contains six gates instead of four, and elsewhere retains the forbidden “Formation sets stage to Active” row. The resulting fixture has 18 rows rather than the specified 17, so the feature currently demonstrates and tests an obsolete workflow. Please align the fixture and E2E mocks with the canonical #1959 seed.

apps/lfx-one/src/server/services/formation.service.ts:362

  • skipped gates are treated as closed here, so skipping the last required item sets is_activating and the queue/readiness UI reports all gates cleared. That conflicts with the shared contract in formation.interface.ts, which says every gate must be done and only done counts. Please keep skipped gating items open (or change the canonical contract/product requirement consistently); the same rule also needs correcting in the fixture initializer and client readiness utility.
    apps/lfx-one/src/app/modules/formations/components/formations-table/formations-table.component.html:43
  • The queue columns are plain headers and the table has no sortField, so the required default announcement-date ordering and user sorting by announcement/readiness are both absent. lfx-table already exposes sorting inputs; please wire a null-last announcement default and sortable Announcement/Progress headers, with tests covering both directions.

The dashboard teaser card mapped a failed getProjectFormation() call to
the same null summary as a formation with no items, so an outage
rendered as an authoritative "0 of 0 done" instead of an error.
Tracks the failure in its own hasError signal and renders a distinct
error message instead of the misleading zero count.

Addresses Copilot finding on PR #2033 (#1958).

Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org>
Copilot AI review requested due to automatic review settings September 7, 2026 22:30

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 86 out of 86 changed files in this pull request and generated 1 comment.

Suppressed comments (4)

Previously missed (2) — in code that hasn't changed since the last review.

apps/lfx-one/src/server/services/formation-item-access.service.ts:32

  • can_complete does not match the permission enforced by the mutation methods: every non-gating item is returned as completable to project readers, while every write subsequently requires project.writer; gating completion additionally uses the gate_writer/LF-staff stand-in even though the canonical interface and GH-1958 state that relation does not exist. This produces enabled controls that always 403 for readers and prevents non-LF-staff project writers from accepting gates. Derive the response capability from the actual project-writer check and remove the synthetic relation.
    packages/shared/src/interfaces/formation.interface.ts:205
  • This edit drops the canonical template action_link field even though the existing round-trip contract test in packages/shared/src/enums/formation.enum.spec.ts:72-120 and PR #2175 define it for relative and absolute template destinations. Consumers typed as FormationTemplate can no longer represent the backend payload, and the eventual expansion cannot produce FormationItem.action_href. Keep the optional template field in the shared contract.

packages/shared/src/utils/formation-checklist.utils.ts:41

  • Skipped gating items are being treated as complete, which contradicts the canonical contract in formation.interface.ts:81-82 and GH-1957's requirement that only done contributes to readiness. A skipped gate can therefore set isActivating/“Gates cleared” even though it is not done. Count every gating status except done as open, and make the same correction in the server readiness rollup and fixture generator.
    apps/lfx-one/src/app/modules/formations/components/formations-table/formations-table.component.html:43
  • The queue requirement includes sorting by readiness and announcement date, with announcement date as the default and nulls last, but these headers are plain text and displayRows preserves API order. Users cannot perform either sort and the default ordering is absent. Wire sortable headers/default ordering (including explicit null-last handling) and add coverage for both fields.

Comment on lines +471 to +475
// A root `auditor` FGA grant reaches this lens without any of hasFullFoundationAccess's grants
// (board role, root-writer, LF-staff, writer-foundation) — same reduced-sidebar situation as
// marketing-only FGA users above. Surface Formations here too, or an auditor-only formation-team
// member has no way into a route `formationsQueueAuditorGuard` already lets them reach.
if (this.isFormationEnabled() && this.canSeeFormationsQueue() && !this.hasFullFoundationAccess()) {
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants