diff --git a/.github/workflows/strip-smoke.yml b/.github/workflows/strip-smoke.yml index 7fea0b1..6eadaf8 100644 --- a/.github/workflows/strip-smoke.yml +++ b/.github/workflows/strip-smoke.yml @@ -200,6 +200,45 @@ jobs: if: steps.scope.outputs.run == 'true' run: dart run tool/strip_sample_features.dart --apply ${{ matrix.flags }} + # `flutter analyze` and `flutter test` only prove the stripped tree still + # builds. They would stay green if the process-only removals silently + # stopped happening, which is the whole point of the strip for an + # adopter, so assert the absence directly. Keep this list in step with + # `_processOnlyPaths` in tool/strip_sample_features.dart. + - name: Assert process-only artifacts are gone + if: steps.scope.outputs.run == 'true' + shell: bash + run: | + set -euo pipefail + leftovers=0 + for path in \ + .claude \ + .github/workflows/issue-refs.yml \ + docs/issue-workflow.md \ + docs/verification \ + scripts/bootstrap-issue-labels.sh \ + scripts/dev/check_issue_refs.sh \ + test/tooling/epic_coverage_test.dart \ + tool/check_epic_coverage.dart + do + if [ -e "$path" ]; then + echo "still present after strip: $path" + leftovers=$((leftovers + 1)) + fi + done + # Whole-line match only: tool/README.md documents the marker syntax + # inline, and that sentence is meant to survive. + marker='^[[:space:]]*[[:space:]]*$' + if grep -rlE "$marker" --include='*.md' . ; then + echo "a process-only marker survived the strip" + leftovers=$((leftovers + 1)) + fi + if [ "$leftovers" -ne 0 ]; then + echo "$leftovers process-only artifact(s) survived the strip." + exit 1 + fi + echo "All process-only artifacts were removed." + - name: Install dependencies (after strip) if: steps.scope.outputs.run == 'true' run: flutter pub get diff --git a/CLAUDE.md b/CLAUDE.md index 9ea01c1..c8387e2 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -29,6 +29,7 @@ Code generation (Freezed, json_serializable, Riverpod) is committed. Regenerate with `flutter pub run build_runner build --delete-conflicting-outputs` only when you touch an annotated source. + ## Workflow GitHub issues are the single source of truth. Full spec: @@ -97,9 +98,24 @@ diff". Every tracked path maps to exactly one epic, enforced by [`tool/check_epic_coverage.dart`](tool/check_epic_coverage.dart) under `flutter test`. If no epic fits a change, that is a taxonomy gap: file it, do not force the nearest label - the parallelism rule below keys on that label. + ## Acceptance verification + +### Scope test: would an adopter notice? + +This is a starter template, not an app that ships. A criterion proves the +**mechanism** is correct; it is not there to prove the *product* runs. So before +writing one, ask: **would an adopter notice if this were missing?** If answering +it needs a physical device, a custom CA, an Apple certificate, a reachable +backend or store credentials, that answer belongs to the adopter's app, not to +this repository - prove the mechanism at tier 1 or tier 2 here and hand the +residual over. Precedent: koniz-dev/flutter-starter#48, #59, #62 and #108 each +sat blocked for months of calendar time on exactly that (a real device, a custom +CA, an Apple cert, on-device storage inspection) after the mechanism had already +been proven, and all four closed with the residual assigned to the adopter. + Run `./scripts/test/run_acceptance.sh `. It runs the format check, `flutter analyze`, `flutter test`, and the golden-tagged acceptance tests, tees every log into `docs/verification/issue-/`, and copies captured PNGs there so @@ -118,6 +134,7 @@ Three tiers exist here: runner opts in with `--run-skipped --tags golden`. Goldens are **evidence, not a CI gate** - a golden regression will not fail CI, by design. - **Tier 3 - not drivable here.** Route to `status:needs-uat`. + ### What this tooling cannot verify @@ -160,6 +177,8 @@ make the loop worthless. reachable API exists here. - **Coverage thresholds.** [`coverage.yml`](.github/workflows/coverage.yml) is manual plus weekly, not per-PR. + + ### Which checks a PR gets Four workflows run on PRs. Only one of them still filters at the workflow @@ -344,6 +363,7 @@ reading them. issue bodies. - Only one session at a time may hold a `status:in-progress` claim on a given issue. The claim-then-re-read step in step 7 above is what enforces this. + ## Conventions diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index fcffb54..bdc4f51 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -116,11 +116,13 @@ Users prefer dark mode for better battery life and eye comfort, especially in lo Pull requests are the best way to propose changes to the codebase. We actively welcome your pull requests. + > Issues are the single source of truth for what gets worked on, in what order, > and when it counts as done. Before picking something up, read > [docs/issue-workflow.md](docs/issue-workflow.md) — it defines the label state > machine, the acceptance-criteria requirement, and why commits link to issues > rather than closing them. + 1. Fork the repository @@ -399,6 +401,7 @@ test(auth): add login use case tests - Keep subject line under 50 characters - Capitalize first letter of subject - No period at end of subject + - Reference issues in the footer as `Refs koniz-dev/flutter-starter#123` > **Do not use `Closes`, `Fixes`, or `Resolves`.** GitHub auto-closes an issue @@ -407,6 +410,7 @@ test(auth): add login use case tests > removes the verification gate the workflow exists to enforce. Use the fully > qualified `owner/repo#N` form so the reference survives being quoted elsewhere. > See [docs/issue-workflow.md](docs/issue-workflow.md). + ### Git hooks diff --git a/docs/guides/onboarding/fork-and-customize.md b/docs/guides/onboarding/fork-and-customize.md index 0aae15a..b6f26eb 100644 --- a/docs/guides/onboarding/fork-and-customize.md +++ b/docs/guides/onboarding/fork-and-customize.md @@ -79,6 +79,16 @@ Naming both samples is the same request as naming neither, so All three variants are exercised by [`strip-smoke.yml`](../../../.github/workflows/strip-smoke.yml), which applies each one and then runs `flutter analyze` and `flutter test` over the result. +Every variant also removes the machinery that serves the upstream repository's +own issue loop rather than your app: the acceptance-evidence archive under +`docs/verification/`, `docs/issue-workflow.md`, `.claude/agents/`, +`scripts/bootstrap-issue-labels.sh`, the epic-coverage guard and its test, and +the commit-reference check with its workflow (it requires `Refs` pointing at the +upstream tracker, so it would reject your commits). The guards that protect your +app - env-asset, docs, signature and symbol checks, the CI workflows and the git +hooks - all stay. See [`tool/README.md`](../../../tool/README.md) for the full +list and the reasoning. + Then delete or adjust any remaining docs under `docs/features/` that referenced removed modules. ## 5. Remove auth sample (advanced) diff --git a/tool/README.md b/tool/README.md index 2354180..d787086 100644 --- a/tool/README.md +++ b/tool/README.md @@ -76,6 +76,7 @@ code it described changed. so a stripped tree does not fail `test/docs/doc_symbols_test.dart` over a feature that is no longer there. + ## `check_epic_coverage.dart` Taxonomy guard: every tracked surface maps to **exactly one** `epic:*` label. @@ -100,6 +101,7 @@ in the same `epic:*`" rule keys on that label. It reads paths from version control rather than from the filesystem, so `strip_sample_features.dart` deleting the sample slices does not make the sample epics look stale under **Strip smoke**. + ## `strip_sample_features.dart` @@ -114,6 +116,29 @@ flutter analyze flutter test ``` +### Process-only artifacts + +Every variant also removes what serves **this repository's issue loop** rather +than the app a fork ships: `docs/verification/` (the acceptance-evidence +archive, by far the largest thing in `docs/`), `docs/issue-workflow.md`, +`.claude/agents/`, `scripts/bootstrap-issue-labels.sh`, +`tool/check_epic_coverage.dart` with `test/tooling/epic_coverage_test.dart`, +and `scripts/dev/check_issue_refs.sh` with `.github/workflows/issue-refs.yml`. +The last two pairs would actively break a fork: the epic guard shells out to a +labels script that no longer exists, and the refs check rejects any commit that +does not name *this* tracker. + +Sections of files a fork keeps are marked in place with +`` / `` (see +`CLAUDE.md`, `CONTRIBUTING.md` and this file) rather than listed in the script, +so the two cannot drift. Unbalanced markers are a hard error, reported before +anything is deleted. + +Kept deliberately, because they protect the adopter's app rather than this +repository's workflow: `check_env_assets.dart`, `check_docs.dart`, +`doc_signatures.dart`, `doc_symbols.dart`, `ci.yml`, `strip-smoke.yml`, the git +hooks, and `scripts/test/run_acceptance.sh`. + ### Golden tree Edit **`tool/golden/stripped/`** when the stripped baseline should change (e.g. new `AppRoutes`, `main` bootstrap). Mirrored paths include `lib/`, `test/core/routing/` (`app_router_test.dart`, `app_routes_test.dart`), and `integration_test/`. The tree is **excluded from** `flutter analyze` via `analysis_options.yaml` so it does not conflict with the real `lib/`. diff --git a/tool/strip_sample_features.dart b/tool/strip_sample_features.dart index 0cacc2f..e884423 100644 --- a/tool/strip_sample_features.dart +++ b/tool/strip_sample_features.dart @@ -1,6 +1,11 @@ // Removes sample `tasks` and `feature_flags` feature modules and rewires // the app using golden files under tool/golden//. // +// Every variant also removes this repository's own process machinery - the +// issue loop, its evidence archive and the guards that only make sense against +// this tracker. See `_processOnlyPaths` for the list and the reasoning per +// entry. +// // Usage (from repository root): // dart run tool/strip_sample_features.dart --apply both // dart run tool/strip_sample_features.dart --apply --remove-tasks @@ -57,6 +62,69 @@ const _goldenOverrides = >{ ], }; +/// Paths that exist to run **this repository's** issue loop rather than to +/// serve an adopter's app. Every variant removes all of them. +/// +/// The test for each entry is the one question an adopter can answer: *would +/// they notice if this were missing?* A guard that protects the shipped app +/// stays; a guard that enforces this tracker's conventions goes. Reasoning per +/// entry: +/// +/// * `docs/verification/` - 31 MB of acceptance evidence about issues closed +/// in this repository. An adopter inherits an archive of work they never +/// saw, and `scripts/test/run_acceptance.sh` recreates the directory for +/// their own issues. +/// * `docs/issue-workflow.md` and `.claude/agents/` - the agent protocol and +/// the four role definitions that drive it. Process, not product. +/// * `scripts/bootstrap-issue-labels.sh` - creates this repository's labels +/// and owns its path -> epic map. A fork has neither. +/// * `tool/check_epic_coverage.dart` and `test/tooling/epic_coverage_test.dart` +/// - assert every tracked surface maps to exactly one `epic:*` label. The +/// tool shells out to the bootstrap script, so keeping the test after +/// removing that script would fail `flutter test` on the first run. +/// * `scripts/dev/check_issue_refs.sh` and `.github/workflows/issue-refs.yml` +/// - require `Refs koniz-dev/flutter-starter#N` in every commit. An +/// adopter's commits reference their own tracker, so the check would +/// reject every pull request they open. +/// +/// Deliberately NOT here, because each protects the adopter's app rather than +/// this repository's workflow: `tool/check_env_assets.dart` (stops secrets +/// shipping in the bundle), `tool/doc_signatures.dart`, `tool/doc_symbols.dart` +/// and `tool/check_docs.dart` (stop docs drifting from code), `ci.yml`, +/// `strip-smoke.yml`, the git hooks, and `scripts/test/run_acceptance.sh` +/// (a format/analyze/test/golden harness that takes whatever issue number the +/// adopter's own tracker gave them). +const _processOnlyPaths = [ + '.claude/agents', + '.github/workflows/issue-refs.yml', + 'docs/issue-workflow.md', + 'docs/verification', + 'scripts/bootstrap-issue-labels.sh', + 'scripts/dev/check_issue_refs.sh', + 'test/tooling/epic_coverage_test.dart', + 'tool/check_epic_coverage.dart', +]; + +/// Directories to delete once emptied by [_processOnlyPaths]. +const _processOnlyPrunedDirs = ['.claude']; + +/// Markers around a block of prose that documents the removed process only. +/// +/// Whole files are cheap to delete; a section inside a file an adopter keeps is +/// not, and hand-written path lists rot. These are HTML comments, so they are +/// invisible in rendered markdown and ignored by `tool/check_docs.dart`. +const _processMarkerStart = ''; +const _processMarkerEnd = ''; + +/// Directory names never walked when looking for markdown. +const _markdownWalkSkips = { + '.dart_tool', + '.git', + '.idea', + 'build', + 'node_modules', +}; + void main(List args) { if (!args.contains('--apply')) { stderr.writeln( @@ -64,7 +132,9 @@ void main(List args) { '[--remove-tasks] [--remove-feature-flags]\n' 'Removes lib/features/tasks, lib/features/feature_flags, related tests, ' 'and core FeatureFlagsManager. Rewires entrypoints from ' - 'tool/golden//. Keeps auth sample.', + 'tool/golden//. Keeps auth sample. Every variant also removes ' + "this repository's process-only artifacts (docs/verification/, the issue " + 'workflow, .claude/agents/ and the issue-loop guards).', ); exitCode = 1; return; @@ -111,6 +181,24 @@ void main(List args) { return; } + // Same discipline as the golden tree: an unbalanced marker pair is found + // before anything is deleted, so a bad edit leaves the working tree intact. + final markerProblems = _validateProcessMarkers(root); + if (markerProblems.isNotEmpty) { + stderr.writeln( + 'Process-only markers are unbalanced; nothing was deleted:\n' + '${markerProblems.join('\n')}', + ); + exitCode = 2; + return; + } + + // Process artifacts go first: `docs/verification/` is by far the largest + // thing removed, and deleting it up front means the later passes over + // `docs/` have thousands of evidence files fewer to read. + _removeProcessArtifacts(root); + _stripProcessMarkdownRegions(root); + if (removeTasks) { _deleteDir(Directory(p.join(root.path, 'lib/features/tasks'))); _deleteDir(Directory(p.join(root.path, 'test/features/tasks'))); @@ -244,6 +332,160 @@ void _deleteDir(Directory dir) { } } +/// Deletes every entry in [_processOnlyPaths], then prunes the directories +/// those deletions emptied. +/// +/// A missing entry is skipped rather than reported: a fork that already deleted +/// its own copy of one of these is not an error. +void _removeProcessArtifacts(Directory repoRoot) { + for (final relative in _processOnlyPaths) { + final path = p.join(repoRoot.path, relative); + final dir = Directory(path); + final file = File(path); + if (dir.existsSync()) { + dir.deleteSync(recursive: true); + } else if (file.existsSync()) { + file.deleteSync(); + } else { + continue; + } + stdout.writeln( + "Removed $relative: it serves this repository's issue loop, not the " + 'app a fork ships.', + ); + } + + for (final relative in _processOnlyPrunedDirs) { + final dir = Directory(p.join(repoRoot.path, relative)); + if (dir.existsSync() && dir.listSync().isEmpty) { + dir.deleteSync(); + stdout.writeln('Removed $relative: emptied by the removals above.'); + } + } +} + +/// Every markdown file that could carry a process-only marker. +/// +/// `docs/verification/` is skipped: the whole tree is deleted anyway, and its +/// files quote captured terminal output, so a marker-shaped string inside one +/// is a transcript rather than an instruction. +List _markdownFiles(Directory repoRoot) { + final found = []; + + void walk(Directory dir) { + if (!dir.existsSync()) { + return; + } + for (final entity in dir.listSync(followLinks: false)) { + final relative = p.split(p.relative(entity.path, from: repoRoot.path)); + if (entity is Directory) { + if (_markdownWalkSkips.contains(relative.last)) { + continue; + } + if (relative.length == 2 && + relative[0] == 'docs' && + relative[1] == 'verification') { + continue; + } + walk(entity); + } else if (entity is File && entity.path.endsWith('.md')) { + found.add(entity); + } + } + } + + walk(repoRoot); + return found; +} + +/// Reports markers that do not pair up, before anything has been deleted. +/// +/// An unclosed start marker would silently swallow the rest of a file an +/// adopter keeps - `CONTRIBUTING.md` from its issue callout to its last line, +/// say - and the only signal would be a shorter file nobody diffed. +List _validateProcessMarkers(Directory repoRoot) { + final problems = []; + for (final file in _markdownFiles(repoRoot)) { + final relative = p.relative(file.path, from: repoRoot.path); + var openedAt = 0; + var lineNumber = 0; + for (final line in file.readAsLinesSync()) { + lineNumber++; + final trimmed = line.trim(); + if (trimmed == _processMarkerStart) { + if (openedAt != 0) { + problems.add( + ' $relative:$lineNumber: start marker inside the region opened ' + 'at line $openedAt', + ); + } + openedAt = lineNumber; + } else if (trimmed == _processMarkerEnd) { + if (openedAt == 0) { + problems.add(' $relative:$lineNumber: end marker with no start'); + } + openedAt = 0; + } + } + if (openedAt != 0) { + problems.add(' $relative:$openedAt: start marker is never closed'); + } + } + return problems; +} + +/// Removes every `` region from markdown files +/// the strip keeps. +/// +/// Used where deleting the whole file would be wrong: `CLAUDE.md` still +/// describes the codebase after the issue loop is gone, `CONTRIBUTING.md` still +/// describes how to open a pull request, and `tool/README.md` still documents +/// the tools that survive. Only the marked block goes; the surrounding prose is +/// the file's own. +void _stripProcessMarkdownRegions(Directory repoRoot) { + for (final file in _markdownFiles(repoRoot)) { + final kept = []; + var dropping = false; + var justClosed = false; + var dropped = 0; + + for (final line in file.readAsLinesSync()) { + final trimmed = line.trim(); + if (trimmed == _processMarkerStart) { + dropping = true; + continue; + } + if (trimmed == _processMarkerEnd) { + dropping = false; + justClosed = true; + dropped++; + continue; + } + if (dropping) { + continue; + } + // A region is normally surrounded by blank lines; keeping both would + // leave a double blank where the section used to be. + if (justClosed) { + justClosed = false; + if (trimmed.isEmpty && (kept.isEmpty || kept.last.trim().isEmpty)) { + continue; + } + } + kept.add(line); + } + + if (dropped == 0) { + continue; + } + file.writeAsStringSync('${kept.join('\n')}\n'); + stdout.writeln( + 'Dropped $dropped process-only section(s) from ' + '${p.relative(file.path, from: repoRoot.path)}.', + ); + } +} + void _patchTestFixtures(String path) { final file = File(path); var s = file.readAsStringSync().replaceAll('\r\n', '\n');