Review: #19 Docs adoption Tier 1 (security, pipeline, testing, local-dev)

review-1800-2026-07-05 Review

review needs-refinement

Verdict: NEEDS_REFINEMENT

Board item 1800 (board-iss), Forgejo issue ldraney/intelligentstaffingsystems#19, 5 points. Scope quality is high — every file target verified accurate against main (commit 670c515) — but the ticket trips two decomposition triggers, both traceability backing notes are missing, and the issue body contains one self-contradiction about deleting the spec file.

Template Completeness (Feature template)

  • [x] Type — Feature (routes to template-issue-feature)
  • [x] Lineage — standalone, executes Tier 1 of docs/adoption-plan.md
  • [x] Repo — ldraney/intelligentstaffingsystems
  • [x] User Story
  • [x] Context — names the spec doc and scopes to Tier 1 only
  • [x] File Targets — create/modify/do-not-touch all present
  • [x] Feature Flag — "none"; correct (docs-only, and repo has no docs/feature-flags.md yet — that is itself a Tier 2 item)
  • [x] Acceptance Criteria — 7 checkboxes
  • [x] Test Expectations — manual mermaid + link checks, run command N/A (appropriate for docs-only)
  • [x] Constraints
  • [x] Checklist
  • [x] Related
All 12 sections present. No template gaps.

Traceability

  • [x] story:project-setup label — present on board item 1800
  • [ ] story note MISSING — project page project-iss does not exist (404); project iss contains only the board note. [SCOPE] Create user story entry on project-iss user-stories section.
  • [x] arch:rails-app label — present on board item 1800
  • [ ] arch note MISSING — search_notes("arch-rails-app") and search_notes("rails-app") both return empty (semantic search unavailable — Ollama down — so verified by keyword only). [SCOPE] Create architecture note arch-rails-app.
  • [x] Forgejo issue — https://forgejo.tail5b443a.ts.net/ldraney/intelligentstaffingsystems/issues/19, open, label sprint:0
Note: the missing project page and arch note affect every ticket on board-iss, not just this one. Items 1798/1799 already passed to validation with the same gap.

File Targets

Verified against origin/main (= local HEAD, identical trees, commit 670c515).
  • [x] docs/security.md (create) — does not exist on main; matches plan Tier 1 row; content spec (Controller Role Map, Lead-claiming attack seed, SOPS secrets table, Hetzner-edge network) matches the plan's security.md section verbatim
  • [x] docs/pipeline.md (create) — does not exist; matches plan (4-flow overview, three-repo table, Update Lifecycle cross-linked to docs/ios.md, which exists)
  • [x] docs/testing-strategy.md (create) — does not exist; matches plan (endpoint-first, no system tests, role-stubbing section, owns lint/test claims)
  • [x] docs/local-dev-setup.md (create) — does not exist; port 9999 claim verified (README.md:105, docs/architecture.md:335); ~/rails-base copy step verified in README local-dev section
  • [x] docs/user-stories.md (modify) — exists; currently has Roles / Client Journey / Epics 1–6 / Role-Tab Access Matrix and lacks all six additions named (Role Permission Matrix, Delivery Phases, Open Questions, Related Tickets, default tab per role, personas) — additions are genuinely new
  • [x] docs/architecture.md (modify) — exists; no Key Design Decisions or Rails Structure sections yet; plan supplies 6 KDDs so the AC's ≥5 is achievable
  • [ ] README.md (modify) — exists, Documentation table rows verified (user-stories, architecture, ios, adoption-plan) — BUT the instruction "remove the adoption-plan.md row and delete docs/adoption-plan.md if all Tier 1 items land" contradicts both the plan's own deletion rule (adoption-plan.md line 3: "delete this file when the tree below exists" — the tree includes Tier 2/3) and the issue's own Context ("Tier 2/3 docs are triggered later per the plan's trigger column"). Deleting at Tier 1 destroys the only spec for Tier 2/3 (feature-flags, keycloak-setup + Postmark SMTP, networking Hetzner rewrite, messaging template, the do-not-adopt list, open decision 3). See [BODY] recommendation.
  • [x] docs/ios.md (do NOT touch) — exists on main
  • [x] ~/landscaping-assistant/docs/ (read-only reference) — exists; all four Tier 1 source files present (security.md, pipeline.md, testing-strategy.md, local-dev-setup.md)

Repo Placement

OK. Single-repo docs work; the issue is filed on the repo it modifies. CLAUDE.md is a symlink to README.md (git mode 120000), so README edits propagate into agent session context — same repo, no additional issue needed.

Dependencies

Blocked by: nothing. Item 1780 (#1, which created the docs this ticket amends) is merged to main and sitting in validation; the spec docs/adoption-plan.md is committed at main. No in_progress items touch the same files.
Blocks (undocumented): effectively all build tickets — the plan sets testing-strategy.md's trigger as "before first PR", so this ticket gates #3 (bootstrap) and downstream #6–#16. AC 2 also reads issues #6–#16 as input (all exist on the board; spot-check: #6 and #11 name controllers, #16 names none — harmless). The issue documents the docs-before-code intent in the User Story but never states "blocks #3" explicitly.

Acceptance Criteria

All 7 criteria are objectively verifiable by an agent: doc existence + section outlines (diff vs plan), Controller Role Map coverage (enumerable from issues #6–#16), matrix size (≥12 actions x 3 roles), KDD count (≥5 numbered), README table completeness, mermaid/link check (manual, stated as such in Test Expectations). No fake test commands. Quality is good — the problem is quantity: 7 ACs exceeds the 5-AC single-pass threshold.

Blast Radius

Docs-only; no code or config. Two propagation effects: (1) README.md edits flow through the CLAUDE.md symlink into every future agent session's context for this repo — the Documentation table must stay accurate; (2) deleting docs/adoption-plan.md prematurely (see [BODY]) would erase the Tier 2/3 spec with no other copy. The framework-neutral test-wording constraint correctly defers plan open decision 1 (Minitest vs RSpec) to testing-strategy.md itself.

Decomposition Assessment

NEEDS DECOMPOSITION — route to skill-decompose-ticket. 7 file targets (4 create + 3 modify) in 1 repo (repo-count rule not tripped); 7 acceptance criteria > 5 (tripped); estimated agent work well over 5 minutes (tripped) — a single pass must read the full plan, the landscaping-assistant reference docs, and issues #6–#16, then author four full docs with mermaid diagrams and amend three more. The plan's per-doc content specs give a clean natural split: (1) security.md, (2) pipeline.md, (3) testing-strategy.md, (4) local-dev-setup.md, (5) user-stories.md amendments, (6) architecture.md amendments + README table + adoption-plan status update as the closer.

Recommendation

  • [DECOMPOSE] 7 ACs and ~7 docs of authoring exceed the single-pass rule — route to skill-decompose-ticket; split per the plan's per-doc content specs (4 create tickets + 2 amendment tickets, README/plan-status update in the closer)
  • [BODY] Fix the README.md file-target instruction: do not delete docs/adoption-plan.md when Tier 1 lands — keep the file and mark the Tier 1 rows done in its status table; delete only when the full target tree (Tier 2/3 included) exists, per the plan's own line-3 rule and the issue's own Context
  • [SCOPE] Create user story entry on project-iss user-stories section (project page project-iss does not exist — board-wide gap)
  • [SCOPE] Create architecture note arch-rails-app for component rails-app (board-wide gap)