cleanup(legibility): repo hygiene — proofshot retention, ignore .agent/.worktrees #415

Closed
opened 2026-05-04 16:17:53 +02:00 by marcel · 9 comments
Owner

Context

Part of Epic #411 — Cleanup. This is CLEANUP-4: address the repo-root hygiene findings from AUDIT-5 (#392). A stranger walking into the repo today sees proofshot-artifacts/, .agent/, .claude/worktrees/, frontend/.svelte-kit.old/, frontend/test-results.locked/ and asks "what are these?". That violates C1.5.

Per the Legibility Rubric, this addresses C1.5 (Minor) but high-leverage for first impression.

Required changes

1. proofshot-artifacts/

  • Decide a retention policy. Options:
    • Move out of repo entirely. Don't commit artifact runs; store them on disk or in a separate artifact bucket.
    • Keep but cap. Add a script (or pre-commit hook) that prunes folders older than N days.
    • Document and accept. If the artifacts ARE intentional deliverables, document this in README.md and docs/.
  • Whichever path, document the decision in docs/CONTRIBUTING.md (DOC-4 / #398) so future contributors know.

2. .agent/ and .claude/worktrees/ and .claude/scheduled_tasks.lock

  • These are LLM/dev-tooling state, not application or deliverable code.
  • Add to .gitignore if not already there.
  • Verify by running git status — these directories should not appear in the staged-changes list.

3. .claude/personas/ and .claude/skills/ and .claude/settings.json

  • These ARE deliverables (custom personas, skills, settings the team relies on).
  • Should be tracked in git. Verify they are.
  • Document their purpose in CONTRIBUTING.md (DOC-4) so contributors know to maintain them.

4. frontend/.svelte-kit.old/

  • Remove. This is a leftover from a previous build/migration. Verify build still works after deletion.

5. frontend/test-results.locked/

  • Investigate. If it's a Playwright artifact directory, add to .gitignore. If something else, document or remove.

6. node_modules/ at repo root

  • Investigate why there's a top-level package.json and package-lock.json (saw it in git status). Is this intentional? Likely a workspace setup or a tooling install. Document or remove.

7. frontend/e2e/.auth/user.json (showing as modified in git status)

  • This is a Playwright auth state. Should NOT be tracked. Add to .gitignore.

Acceptance criteria

  • proofshot-artifacts/ decision made and implemented; documented in CONTRIBUTING.md
  • .gitignore updated to exclude all dev-tooling state directories
  • frontend/.svelte-kit.old/ deleted; build verified working
  • frontend/test-results.locked/ resolved (gitignore'd or removed)
  • Top-level node_modules/ and package.json resolved (justified or removed)
  • frontend/e2e/.auth/user.json removed from tracking; gitignore'd
  • After this PR merges, git status on a fresh clone shows zero unexpected items
  • Audit re-run (CLEANUP-5) confirms C1.5 PASS

Dependency

Soft dependency on AUDIT-5 (#392) — that audit's §7 will surface anything I missed above.

Definition of Done

PR merged; closing comment shows git status output of a fresh clone with the cleanup applied.

## Context Part of **Epic #411** — Cleanup. This is **CLEANUP-4**: address the repo-root hygiene findings from AUDIT-5 (#392). A stranger walking into the repo today sees `proofshot-artifacts/`, `.agent/`, `.claude/worktrees/`, `frontend/.svelte-kit.old/`, `frontend/test-results.locked/` and asks "what are these?". That violates C1.5. Per the Legibility Rubric, this addresses **C1.5 (Minor)** but high-leverage for first impression. ## Required changes ### 1. `proofshot-artifacts/` - Decide a retention policy. Options: - **Move out of repo entirely.** Don't commit artifact runs; store them on disk or in a separate artifact bucket. - **Keep but cap.** Add a script (or pre-commit hook) that prunes folders older than N days. - **Document and accept.** If the artifacts ARE intentional deliverables, document this in `README.md` and `docs/`. - Whichever path, document the decision in `docs/CONTRIBUTING.md` (DOC-4 / #398) so future contributors know. ### 2. `.agent/` and `.claude/worktrees/` and `.claude/scheduled_tasks.lock` - These are LLM/dev-tooling state, not application or deliverable code. - Add to `.gitignore` if not already there. - Verify by running `git status` — these directories should not appear in the staged-changes list. ### 3. `.claude/personas/` and `.claude/skills/` and `.claude/settings.json` - These ARE deliverables (custom personas, skills, settings the team relies on). - Should be tracked in git. Verify they are. - Document their purpose in `CONTRIBUTING.md` (DOC-4) so contributors know to maintain them. ### 4. `frontend/.svelte-kit.old/` - Remove. This is a leftover from a previous build/migration. Verify build still works after deletion. ### 5. `frontend/test-results.locked/` - Investigate. If it's a Playwright artifact directory, add to `.gitignore`. If something else, document or remove. ### 6. `node_modules/` at repo root - Investigate why there's a top-level `package.json` and `package-lock.json` (saw it in git status). Is this intentional? Likely a workspace setup or a tooling install. Document or remove. ### 7. `frontend/e2e/.auth/user.json` (showing as modified in git status) - This is a Playwright auth state. Should NOT be tracked. Add to `.gitignore`. ## Acceptance criteria - [ ] `proofshot-artifacts/` decision made and implemented; documented in CONTRIBUTING.md - [ ] `.gitignore` updated to exclude all dev-tooling state directories - [ ] `frontend/.svelte-kit.old/` deleted; build verified working - [ ] `frontend/test-results.locked/` resolved (gitignore'd or removed) - [ ] Top-level `node_modules/` and `package.json` resolved (justified or removed) - [ ] `frontend/e2e/.auth/user.json` removed from tracking; gitignore'd - [ ] After this PR merges, `git status` on a fresh clone shows zero unexpected items - [ ] Audit re-run (CLEANUP-5) confirms C1.5 PASS ## Dependency Soft dependency on AUDIT-5 (#392) — that audit's §7 will surface anything I missed above. ## Definition of Done PR merged; closing comment shows `git status` output of a fresh clone with the cleanup applied.
marcel added this to the Codebase Legibility milestone 2026-05-04 16:17:53 +02:00
marcel added the P3-latercleanuplegibility labels 2026-05-04 16:18:46 +02:00
Author
Owner

🔐 Nora "NullX" Steiner — Application Security Engineer

Observations

  • frontend/e2e/.auth/user.json is tracked in git and contains a live credential. I decoded it: the auth_token cookie value is Basic%20YWRtaW5AZmFt... — a Base64-encoded HTTP Basic Auth token. Anyone who clones this repo has the admin credential. The frontend/.gitignore already contains e2e/.auth/ as a rule, but the file was committed before the rule existed — .gitignore does not untrack already-tracked files. This is a P0 security item that should be addressed before any other item in this issue.
  • .agent/, .claude/worktrees/, .claude/scheduled_tasks.lock contain LLM process metadata (PIDs, session IDs) — not credentials, but they reveal internal tool internals and shouldn't be in a repo clone.
  • .claude/settings.json likely contains tool permissions and hooks — review whether it contains any tokens or secrets before confirming it should be tracked.

Recommendations

  1. Immediately rotate the admin credential that is encoded in frontend/e2e/.auth/user.json. Even after git rm, the token is in git history and must be revoked.
  2. Run git rm --cached frontend/e2e/.auth/user.json to untrack the file without deleting it from disk. The .gitignore rule e2e/.auth/ already covers it — this just needs the file removed from tracking.
  3. Consider a git history rewrite (git filter-branch or BFG Repo Cleaner) if the token has meaningful access scope. For a self-hosted family project the blast radius is limited, but the principle stands.
  4. Add a pre-commit hook or CI check that blocks any file under e2e/.auth/ from being committed, as a permanent guard against re-occurrence.
  5. Review .claude/settings.json for secrets before confirming it as a tracked deliverable.

Open Decisions (omit this section entirely if none)

  • Credential rotation scope: The auth_token appears to be the dev admin credential (admin@familyarchive.local / admin123 per project memory). If this is the same password used in production, rotation is urgent. If it is dev-only, rotation is still good hygiene — but priority differs. Only you know the answer.
## 🔐 Nora "NullX" Steiner — Application Security Engineer ### Observations - **`frontend/e2e/.auth/user.json` is tracked in git and contains a live credential.** I decoded it: the `auth_token` cookie value is `Basic%20YWRtaW5AZmFt...` — a Base64-encoded HTTP Basic Auth token. Anyone who clones this repo has the admin credential. The `frontend/.gitignore` already contains `e2e/.auth/` as a rule, but the file was committed before the rule existed — `.gitignore` does not untrack already-tracked files. This is a **P0 security item that should be addressed before any other item in this issue.** - `.agent/`, `.claude/worktrees/`, `.claude/scheduled_tasks.lock` contain LLM process metadata (PIDs, session IDs) — not credentials, but they reveal internal tool internals and shouldn't be in a repo clone. - `.claude/settings.json` likely contains tool permissions and hooks — review whether it contains any tokens or secrets before confirming it should be tracked. ### Recommendations 1. **Immediately rotate the admin credential** that is encoded in `frontend/e2e/.auth/user.json`. Even after `git rm`, the token is in git history and must be revoked. 2. **Run `git rm --cached frontend/e2e/.auth/user.json`** to untrack the file without deleting it from disk. The `.gitignore` rule `e2e/.auth/` already covers it — this just needs the file removed from tracking. 3. Consider a **git history rewrite** (`git filter-branch` or BFG Repo Cleaner) if the token has meaningful access scope. For a self-hosted family project the blast radius is limited, but the principle stands. 4. Add a **pre-commit hook or CI check** that blocks any file under `e2e/.auth/` from being committed, as a permanent guard against re-occurrence. 5. Review `.claude/settings.json` for secrets before confirming it as a tracked deliverable. ### Open Decisions _(omit this section entirely if none)_ - **Credential rotation scope**: The `auth_token` appears to be the dev admin credential (`admin@familyarchive.local / admin123` per project memory). If this is the same password used in production, rotation is urgent. If it is dev-only, rotation is still good hygiene — but priority differs. Only you know the answer.
Author
Owner

🏗️ Markus Keller — Senior Application Architect

Observations

  • The root .gitignore is missing entries for every item this issue addresses. Confirmed by git check-ignore: .agent/, .claude/worktrees/, .claude/scheduled_tasks.lock, proofshot-artifacts/, frontend/test-results.locked/, and frontend/e2e/.auth/user.json all have no gitignore match at the repo root level. The frontend/.gitignore has e2e/.auth/ but the file is already tracked, so the rule has no effect.
  • The root .gitignore does cover .worktrees/ (with a leading dot) and .superpowers/ — but .claude/worktrees/ (nested under .claude/) is a different path and is not covered.
  • proofshot-artifacts/ (7.6 MB) is untracked but not ignored. It is a run artifact from the proofshot skill, not application code. It does not belong in the repository regardless of retention policy.
  • The root package.json contains only three dev dependencies (@testing-library/jest-dom, @testing-library/svelte, jsdom) — this appears to be a workspace-level shim for component testing dependencies, separate from frontend/package.json. Whether this is intentional or a leftover deserves one line of documentation.
  • .claude/personas/, .claude/skills/, .claude/settings.json are correctly identified as deliverables — these are the team's tooling infrastructure and should be tracked.

Recommendations

  1. Add to root .gitignore in a single PR (grouped by category):
    # Claude Code / LLM tooling state — not application code
    .agent/
    .claude/worktrees/
    .claude/scheduled_tasks.lock
    
    # Run artifacts from verification tooling
    proofshot-artifacts/
    
    # Playwright test artifacts
    frontend/test-results.locked/
    
  2. Do not change the root package.json yet — document the purpose in a comment in the file itself or in CONTRIBUTING.md. The file is small and has a clear function; the mystery is why it lives at root rather than contributing its deps to frontend/package.json.
  3. Document .claude/ directory structure in CONTRIBUTING.md (DOC-4, #398) so contributors understand which subdirs are deliverables and which are ephemeral state.
  4. The frontend/.svelte-kit.old/ directory — verified to contain only types/ and src/ — is safe to delete. Run npm run check after deletion to confirm no path references it.

No open decisions — all items have a clear resolution path.

## 🏗️ Markus Keller — Senior Application Architect ### Observations - The root `.gitignore` is missing entries for every item this issue addresses. Confirmed by `git check-ignore`: `.agent/`, `.claude/worktrees/`, `.claude/scheduled_tasks.lock`, `proofshot-artifacts/`, `frontend/test-results.locked/`, and `frontend/e2e/.auth/user.json` all have **no gitignore match at the repo root level**. The `frontend/.gitignore` has `e2e/.auth/` but the file is already tracked, so the rule has no effect. - The root `.gitignore` does cover `.worktrees/` (with a leading dot) and `.superpowers/` — but `.claude/worktrees/` (nested under `.claude/`) is a different path and is not covered. - `proofshot-artifacts/` (7.6 MB) is untracked but not ignored. It is a run artifact from the proofshot skill, not application code. It does not belong in the repository regardless of retention policy. - The root `package.json` contains only three dev dependencies (`@testing-library/jest-dom`, `@testing-library/svelte`, `jsdom`) — this appears to be a workspace-level shim for component testing dependencies, separate from `frontend/package.json`. Whether this is intentional or a leftover deserves one line of documentation. - `.claude/personas/`, `.claude/skills/`, `.claude/settings.json` are correctly identified as deliverables — these are the team's tooling infrastructure and should be tracked. ### Recommendations 1. **Add to root `.gitignore` in a single PR** (grouped by category): ``` # Claude Code / LLM tooling state — not application code .agent/ .claude/worktrees/ .claude/scheduled_tasks.lock # Run artifacts from verification tooling proofshot-artifacts/ # Playwright test artifacts frontend/test-results.locked/ ``` 2. **Do not change the root `package.json`** yet — document the purpose in a comment in the file itself or in `CONTRIBUTING.md`. The file is small and has a clear function; the mystery is why it lives at root rather than contributing its deps to `frontend/package.json`. 3. **Document `.claude/` directory structure** in `CONTRIBUTING.md` (DOC-4, #398) so contributors understand which subdirs are deliverables and which are ephemeral state. 4. The `frontend/.svelte-kit.old/` directory — verified to contain only `types/` and `src/` — is safe to delete. Run `npm run check` after deletion to confirm no path references it. ### No open decisions — all items have a clear resolution path.
Author
Owner

⚙️ Tobias Wendt — DevOps & Platform Engineer

Observations

  • None of the items in this issue are covered by the root .gitignore — I ran git check-ignore against all of them and every check returned a miss. The .gitignore covers .worktrees/ (bare path) but not .claude/worktrees/ (nested path). These are different entries.
  • proofshot-artifacts/ (7.6 MB) is a Playwright-screenshot artifact directory generated by the proofshot skill. It should never be committed. The frontend/.gitignore does have proofshot-artifacts/ but that only covers a frontend/proofshot-artifacts/ path. The actual directory lives at the repo root and needs a root-level entry.
  • .claude/scheduled_tasks.lock contains a JSON object with a pid, sessionId, and acquiredAt timestamp — it's a live-process lock file, not configuration. This must be gitignored, not committed.
  • .claude/worktrees/ has 19 entries. These are ephemeral agent workspaces. Committing them would balloon the repository with duplicated working-tree content.
  • frontend/test-results.locked/ (20 KB) contains Playwright E2E test results — clearly a run artifact. The root .gitignore has **/test-results/ but the locked suffix bypasses that glob. Either rename the directory to test-results (which would be caught) or add an explicit entry.
  • frontend/e2e/.auth/user.json is already tracked by git even though frontend/.gitignore has e2e/.auth/. The .gitignore rule is correct but arrived after the file was committed — git rm --cached is required to untrack it.

Recommendations

  1. Add to root .gitignore (the entries below fix every miss confirmed by git check-ignore):
    # LLM tooling state
    .agent/
    .claude/worktrees/
    .claude/scheduled_tasks.lock
    
    # Run artifacts
    proofshot-artifacts/
    frontend/test-results.locked/
    
  2. Untrack the auth file: git rm --cached frontend/e2e/.auth/user.json. The existing .gitignore rule in frontend/ will then take effect.
  3. For frontend/test-results.locked/ — the locked suffix looks like it was renamed to prevent gitignore from catching it (e.g. during debugging). Rename it back to test-results/ and the existing **/test-results/ root rule covers it. Or just add an explicit entry.
  4. The CI E2E setup should generate e2e/.auth/user.json fresh at the start of each run via auth.setup.ts — confirm that the Gitea Actions workflow does this, so removing the tracked file does not break CI.

Open Decisions (omit this section entirely if none)

  • frontend/test-results.locked/ naming: Was this renamed intentionally (to preserve a specific run's output for inspection) or accidentally? If intentional, the "locked" convention needs a documented meaning. If accidental, rename to test-results and the existing glob catches it.
## ⚙️ Tobias Wendt — DevOps & Platform Engineer ### Observations - **None of the items in this issue are covered by the root `.gitignore`** — I ran `git check-ignore` against all of them and every check returned a miss. The `.gitignore` covers `.worktrees/` (bare path) but not `.claude/worktrees/` (nested path). These are different entries. - `proofshot-artifacts/` (7.6 MB) is a Playwright-screenshot artifact directory generated by the proofshot skill. It should never be committed. The `frontend/.gitignore` does have `proofshot-artifacts/` but that only covers a `frontend/proofshot-artifacts/` path. The actual directory lives at the repo root and needs a root-level entry. - `.claude/scheduled_tasks.lock` contains a JSON object with a `pid`, `sessionId`, and `acquiredAt` timestamp — it's a live-process lock file, not configuration. This must be gitignored, not committed. - `.claude/worktrees/` has 19 entries. These are ephemeral agent workspaces. Committing them would balloon the repository with duplicated working-tree content. - `frontend/test-results.locked/` (20 KB) contains Playwright E2E test results — clearly a run artifact. The root `.gitignore` has `**/test-results/` but the `locked` suffix bypasses that glob. Either rename the directory to `test-results` (which would be caught) or add an explicit entry. - `frontend/e2e/.auth/user.json` is **already tracked** by git even though `frontend/.gitignore` has `e2e/.auth/`. The `.gitignore` rule is correct but arrived after the file was committed — `git rm --cached` is required to untrack it. ### Recommendations 1. Add to root `.gitignore` (the entries below fix every miss confirmed by `git check-ignore`): ```gitignore # LLM tooling state .agent/ .claude/worktrees/ .claude/scheduled_tasks.lock # Run artifacts proofshot-artifacts/ frontend/test-results.locked/ ``` 2. Untrack the auth file: `git rm --cached frontend/e2e/.auth/user.json`. The existing `.gitignore` rule in `frontend/` will then take effect. 3. For `frontend/test-results.locked/` — the `locked` suffix looks like it was renamed to prevent gitignore from catching it (e.g. during debugging). Rename it back to `test-results/` and the existing `**/test-results/` root rule covers it. Or just add an explicit entry. 4. The CI E2E setup should generate `e2e/.auth/user.json` fresh at the start of each run via `auth.setup.ts` — confirm that the Gitea Actions workflow does this, so removing the tracked file does not break CI. ### Open Decisions _(omit this section entirely if none)_ - **`frontend/test-results.locked/` naming**: Was this renamed intentionally (to preserve a specific run's output for inspection) or accidentally? If intentional, the "locked" convention needs a documented meaning. If accidental, rename to `test-results` and the existing glob catches it.
Author
Owner

👨‍💻 Felix Brandt — Senior Fullstack Developer

Observations

  • The issue is well-scoped and the acceptance criteria are concrete. Each item has a clear resolution path — this is a straightforward PR.
  • The root package.json is the most interesting unknown. It contains only @testing-library/jest-dom, @testing-library/svelte, and jsdom as dev dependencies. These look like they were installed at the repo root for use by a component test runner that operates outside the frontend/ workspace. This may have been a one-off experiment or a setup artifact. The lock file (package-lock.json) at root creates ambiguity: npm install at the repo root would now install things into a root node_modules/, separate from frontend/node_modules/.
  • frontend/.svelte-kit.old/ contains types/src — this is a Vite/SvelteKit generated directory from before a migration. It is safe to delete without touching any source file.
  • frontend/test-results.locked/ contains Playwright E2E result artifacts. The locked suffix prevented the **/test-results/ gitignore glob from matching it.

Recommendations

  1. For frontend/.svelte-kit.old/: simply rm -rf frontend/.svelte-kit.old/ and run npm run check and npm run build to confirm nothing references it. Add .svelte-kit.old/ to .gitignore as a precaution against a future rebuild creating the same directory.
  2. For the root package.json: make a decision and document it as a comment in the file itself. Two options:
    • If the three testing libraries are still needed at root, keep the file and add a comment explaining why they live there and not in frontend/.
    • If they were experimental, delete package.json, package-lock.json, and node_modules/ at root.
  3. For the node_modules/ at root: add to .gitignore unconditionally — even if package.json stays, node_modules/ is never committed.
  4. In the acceptance criteria, the DoD says "git status on a fresh clone shows zero unexpected items." Add a CI check that runs git status --short after checkout and asserts empty output — this prevents the issue from silently regressing.

No open decisions — all items have a clear path forward pending the package.json decision.

## 👨‍💻 Felix Brandt — Senior Fullstack Developer ### Observations - The issue is well-scoped and the acceptance criteria are concrete. Each item has a clear resolution path — this is a straightforward PR. - The root `package.json` is the most interesting unknown. It contains only `@testing-library/jest-dom`, `@testing-library/svelte`, and `jsdom` as dev dependencies. These look like they were installed at the repo root for use by a component test runner that operates outside the `frontend/` workspace. This may have been a one-off experiment or a setup artifact. The lock file (`package-lock.json`) at root creates ambiguity: `npm install` at the repo root would now install things into a root `node_modules/`, separate from `frontend/node_modules/`. - `frontend/.svelte-kit.old/` contains `types/src` — this is a Vite/SvelteKit generated directory from before a migration. It is safe to delete without touching any source file. - `frontend/test-results.locked/` contains Playwright E2E result artifacts. The `locked` suffix prevented the `**/test-results/` gitignore glob from matching it. ### Recommendations 1. **For `frontend/.svelte-kit.old/`**: simply `rm -rf frontend/.svelte-kit.old/` and run `npm run check` and `npm run build` to confirm nothing references it. Add `.svelte-kit.old/` to `.gitignore` as a precaution against a future rebuild creating the same directory. 2. **For the root `package.json`**: make a decision and document it as a comment in the file itself. Two options: - If the three testing libraries are still needed at root, keep the file and add a comment explaining why they live there and not in `frontend/`. - If they were experimental, delete `package.json`, `package-lock.json`, and `node_modules/` at root. 3. **For the `node_modules/` at root**: add to `.gitignore` unconditionally — even if `package.json` stays, `node_modules/` is never committed. 4. In the acceptance criteria, the DoD says "git status on a fresh clone shows zero unexpected items." Add a CI check that runs `git status --short` after checkout and asserts empty output — this prevents the issue from silently regressing. ### No open decisions — all items have a clear path forward pending the `package.json` decision.
Author
Owner

🧪 Sara Holt — QA Engineer & Test Strategist

Observations

  • frontend/e2e/.auth/user.json is tracked in git. This is the Playwright auth state file that gets generated by auth.setup.ts. It contains a live session token. If it's committed, every CI run that regenerates it will show it as "modified" in git status — which is exactly what the current git status output shows (M frontend/e2e/.auth/user.json). This is a test infrastructure reliability issue: if this file drifts between local and CI, E2E tests will fail with authentication errors rather than meaningful test failures.
  • frontend/test-results.locked/ (20 KB) appears to be a Playwright artifact dir from a previous run that was renamed. The root .gitignore has **/test-results/ but the locked suffix bypasses the glob. This means test artifacts from a specific run were preserved and are now in git status as untracked.
  • The acceptance criterion "git status on a fresh clone shows zero unexpected items" is correct and sufficient as a DoD signal. I'd add one more: CI should fail if git status --short is non-empty after checkout — otherwise the DoD only gets verified manually at merge time.

Recommendations

  1. For frontend/e2e/.auth/user.json: git rm --cached frontend/e2e/.auth/user.json. The frontend/.gitignore already has e2e/.auth/ so this single command is the entire fix. Verify by running git status — the file should no longer appear as tracked.
  2. Confirm CI auth setup: the Playwright auth.setup.ts should run before all E2E tests and write frontend/e2e/.auth/user.json fresh each time. Check the Gitea Actions workflow to confirm this is configured, so untracking the file does not break CI.
  3. For frontend/test-results.locked/: determine whether this is a one-off preserved run (in which case delete it and add **/test-results.locked/ to .gitignore) or a recurring pattern (in which case rename to test-results/ and the existing glob covers it). Either way, add an explicit .gitignore entry.
  4. Add a CI lint step: git diff --exit-code && git status --short | grep -q '^?' && exit 1 || exit 0 — this fails if any untracked file or modification appears after the workflow checkout. It's a lightweight guard that enforces the DoD automatically.

Open Decisions (omit this section entirely if none)

  • test-results.locked/ origin: Was this directory renamed intentionally to preserve a specific Playwright run for debugging, or is it an accidental naming artifact? The answer determines whether to rename-and-use-existing-glob or add a new gitignore entry.
## 🧪 Sara Holt — QA Engineer & Test Strategist ### Observations - **`frontend/e2e/.auth/user.json` is tracked in git.** This is the Playwright auth state file that gets generated by `auth.setup.ts`. It contains a live session token. If it's committed, every CI run that regenerates it will show it as "modified" in git status — which is exactly what the current `git status` output shows (`M frontend/e2e/.auth/user.json`). This is a test infrastructure reliability issue: if this file drifts between local and CI, E2E tests will fail with authentication errors rather than meaningful test failures. - `frontend/test-results.locked/` (20 KB) appears to be a Playwright artifact dir from a previous run that was renamed. The root `.gitignore` has `**/test-results/` but the `locked` suffix bypasses the glob. This means test artifacts from a specific run were preserved and are now in `git status` as untracked. - The acceptance criterion "git status on a fresh clone shows zero unexpected items" is correct and sufficient as a DoD signal. I'd add one more: **CI should fail if `git status --short` is non-empty after checkout** — otherwise the DoD only gets verified manually at merge time. ### Recommendations 1. **For `frontend/e2e/.auth/user.json`**: `git rm --cached frontend/e2e/.auth/user.json`. The `frontend/.gitignore` already has `e2e/.auth/` so this single command is the entire fix. Verify by running `git status` — the file should no longer appear as tracked. 2. **Confirm CI auth setup**: the Playwright `auth.setup.ts` should run before all E2E tests and write `frontend/e2e/.auth/user.json` fresh each time. Check the Gitea Actions workflow to confirm this is configured, so untracking the file does not break CI. 3. **For `frontend/test-results.locked/`**: determine whether this is a one-off preserved run (in which case delete it and add `**/test-results.locked/` to `.gitignore`) or a recurring pattern (in which case rename to `test-results/` and the existing glob covers it). Either way, add an explicit `.gitignore` entry. 4. **Add a CI lint step**: `git diff --exit-code && git status --short | grep -q '^?' && exit 1 || exit 0` — this fails if any untracked file or modification appears after the workflow checkout. It's a lightweight guard that enforces the DoD automatically. ### Open Decisions _(omit this section entirely if none)_ - **`test-results.locked/` origin**: Was this directory renamed intentionally to preserve a specific Playwright run for debugging, or is it an accidental naming artifact? The answer determines whether to rename-and-use-existing-glob or add a new gitignore entry.
Author
Owner

🎨 Leonie Voss — UI/UX Design Lead

Observations

This issue is infrastructure hygiene with no direct UI impact. I checked everything I can comment on from a design perspective:

  • No UI surfaces are affected — none of the files in scope (proofshot-artifacts/, .agent/, .claude/worktrees/, frontend/.svelte-kit.old/, frontend/test-results.locked/) touch any Svelte component, CSS token, or design system file.
  • proofshot-artifacts/ contains Playwright screenshots from the proofshot skill — these are visual verification artifacts, not design assets. They are not part of the design system and do not belong in the repo.
  • frontend/.svelte-kit.old/ — the types/src directory inside this is generated TypeScript. It has no bearing on the UI.

Recommendations

  • No design system work required for this issue.
  • When proofshot-artifacts/ is excluded from git, consider whether the screenshots it produces are useful for visual regression baselining. If the team wants to do visual regression testing, the right tool is Playwright's built-in screenshot comparison (expect(page).toHaveScreenshot()), not ad-hoc proofshot runs committed to the repo. That's a separate issue, but worth noting before discarding the directory's output entirely.

No concerns from my angle on the hygiene changes themselves.

## 🎨 Leonie Voss — UI/UX Design Lead ### Observations This issue is infrastructure hygiene with no direct UI impact. I checked everything I can comment on from a design perspective: - **No UI surfaces are affected** — none of the files in scope (`proofshot-artifacts/`, `.agent/`, `.claude/worktrees/`, `frontend/.svelte-kit.old/`, `frontend/test-results.locked/`) touch any Svelte component, CSS token, or design system file. - `proofshot-artifacts/` contains Playwright screenshots from the proofshot skill — these are visual verification artifacts, not design assets. They are not part of the design system and do not belong in the repo. - `frontend/.svelte-kit.old/` — the `types/src` directory inside this is generated TypeScript. It has no bearing on the UI. ### Recommendations - No design system work required for this issue. - When `proofshot-artifacts/` is excluded from git, consider whether the **screenshots it produces are useful for visual regression baselining**. If the team wants to do visual regression testing, the right tool is Playwright's built-in screenshot comparison (`expect(page).toHaveScreenshot()`), not ad-hoc proofshot runs committed to the repo. That's a separate issue, but worth noting before discarding the directory's output entirely. ### No concerns from my angle on the hygiene changes themselves.
Author
Owner

📋 Elicit (Requirements Engineer)

Observations

  • The issue is well-structured with a clear scope, grouped by item, and concrete acceptance criteria. It meets the Definition of Ready for most items.
  • One ambiguity in the AC: "After this PR merges, git status on a fresh clone shows zero unexpected items" — the word "unexpected" is untestable as written. Either this means "zero untracked files in git status --short" (which is testable) or it means something narrower. The intent is clearly the former; the AC should say so.
  • Proofshot retention policy (item 1) is the only open design question in the issue — three options are listed (move out, cap, document+accept) but no decision is made. This is a genuine product decision, not a technical one.
  • Item 6 (root node_modules/) is listed as "Investigate why..." but investigation has a clear answer already: the root package.json contains only three testing library dev dependencies. The question is whether to keep or remove them, not whether to investigate. Reframing as a decision item would make the AC cleaner.
  • The issue references DOC-4 / #398 for CONTRIBUTING.md documentation — confirm that issue is still open and in scope before adding more work to it.

Recommendations

  1. Tighten the AC for the "fresh clone" criterion: replace "zero unexpected items" with "running git status --short on a fresh clone produces no output."
  2. Make a decision on proofshot retention before starting the PR, not during it — the AC blocks on this decision. The fastest resolution: add proofshot-artifacts/ to root .gitignore (the directories already exist on disk, the issue is just that they are not ignored). A retention policy for the on-disk artifacts is an ops concern, not a code concern.
  3. Confirm DOC-4 (#398) is active before adding work items to it. If #398 is stale or blocked, the CONTRIBUTING.md documentation items in this issue need their own acceptance criteria path.

Open Decisions (omit this section entirely if none)

  • Proofshot artifacts: Keep-and-cap, move-out-of-repo, or document-as-intentional? The gitignore fix is the same regardless of which path is chosen, but the CONTRIBUTING.md note differs. Decide before writing the PR description.
  • Root package.json: Keep (with documentation) or delete? Both are valid. The choice determines whether node_modules/ at root is a permanent gitignore entry or a one-time cleanup.
## 📋 Elicit (Requirements Engineer) ### Observations - The issue is well-structured with a clear scope, grouped by item, and concrete acceptance criteria. It meets the Definition of Ready for most items. - **One ambiguity in the AC**: "After this PR merges, `git status` on a fresh clone shows zero unexpected items" — the word "unexpected" is untestable as written. Either this means "zero untracked files in `git status --short`" (which is testable) or it means something narrower. The intent is clearly the former; the AC should say so. - **Proofshot retention policy (item 1) is the only open design question** in the issue — three options are listed (move out, cap, document+accept) but no decision is made. This is a genuine product decision, not a technical one. - **Item 6 (root `node_modules/`)** is listed as "Investigate why..." but investigation has a clear answer already: the root `package.json` contains only three testing library dev dependencies. The question is whether to keep or remove them, not whether to investigate. Reframing as a decision item would make the AC cleaner. - The issue references DOC-4 / #398 for CONTRIBUTING.md documentation — confirm that issue is still open and in scope before adding more work to it. ### Recommendations 1. **Tighten the AC** for the "fresh clone" criterion: replace "zero unexpected items" with "running `git status --short` on a fresh clone produces no output." 2. **Make a decision on proofshot retention before starting the PR**, not during it — the AC blocks on this decision. The fastest resolution: add `proofshot-artifacts/` to root `.gitignore` (the directories already exist on disk, the issue is just that they are not ignored). A retention policy for the on-disk artifacts is an ops concern, not a code concern. 3. **Confirm DOC-4 (#398) is active** before adding work items to it. If #398 is stale or blocked, the CONTRIBUTING.md documentation items in this issue need their own acceptance criteria path. ### Open Decisions _(omit this section entirely if none)_ - **Proofshot artifacts**: Keep-and-cap, move-out-of-repo, or document-as-intentional? The gitignore fix is the same regardless of which path is chosen, but the CONTRIBUTING.md note differs. Decide before writing the PR description. - **Root `package.json`**: Keep (with documentation) or delete? Both are valid. The choice determines whether `node_modules/` at root is a permanent gitignore entry or a one-time cleanup.
Author
Owner

🗳️ Decision Queue — Action Required

4 decisions need your input before implementation starts.

Security

  • Credential rotation scope — The auth_token in the tracked frontend/e2e/.auth/user.json decodes to the dev admin Basic Auth credential. Is this the same password used in production? If yes, rotate immediately; if dev-only, rotate as good hygiene but at lower urgency. Either way, git rm --cached the file first. (Raised by: Nora)

Infrastructure / Repository

  • proofshot-artifacts/ retention policy — Three options exist: (A) add to root .gitignore only (on-disk artifacts are yours to manage), (B) add a prune script capping to N days, or (C) document as intentional deliverables. The gitignore fix is the same in all cases. The CONTRIBUTING.md entry differs. Fastest resolution: choose A (gitignore + delete local), note it in CONTRIBUTING.md as "run artifacts, never commit." (Raised by: Elicit, Markus)

  • Root package.json — Contains only @testing-library/jest-dom, @testing-library/svelte, jsdom. Keep (with a one-line comment explaining why they live at root rather than inside frontend/) or delete (and clean up package-lock.json + node_modules/). Both are valid; the answer determines whether root-level node_modules/ needs a permanent .gitignore entry or is a one-time cleanup. (Raised by: Felix, Elicit)

Test Infrastructure

  • frontend/test-results.locked/ naming — Was this directory renamed from test-results/ intentionally (to preserve a specific Playwright run for debugging) or accidentally? If intentional, document the "locked" convention and add **/test-results.locked/ to .gitignore. If accidental, rename back to test-results/ — the existing root **/test-results/ glob will cover it. (Raised by: Tobias, Sara)
## 🗳️ Decision Queue — Action Required _4 decisions need your input before implementation starts._ ### Security - **Credential rotation scope** — The `auth_token` in the tracked `frontend/e2e/.auth/user.json` decodes to the dev admin Basic Auth credential. Is this the same password used in production? If yes, rotate immediately; if dev-only, rotate as good hygiene but at lower urgency. Either way, `git rm --cached` the file first. _(Raised by: Nora)_ ### Infrastructure / Repository - **`proofshot-artifacts/` retention policy** — Three options exist: (A) add to root `.gitignore` only (on-disk artifacts are yours to manage), (B) add a prune script capping to N days, or (C) document as intentional deliverables. The gitignore fix is the same in all cases. The CONTRIBUTING.md entry differs. Fastest resolution: choose A (gitignore + delete local), note it in CONTRIBUTING.md as "run artifacts, never commit." _(Raised by: Elicit, Markus)_ - **Root `package.json`** — Contains only `@testing-library/jest-dom`, `@testing-library/svelte`, `jsdom`. Keep (with a one-line comment explaining why they live at root rather than inside `frontend/`) or delete (and clean up `package-lock.json` + `node_modules/`). Both are valid; the answer determines whether root-level `node_modules/` needs a permanent `.gitignore` entry or is a one-time cleanup. _(Raised by: Felix, Elicit)_ ### Test Infrastructure - **`frontend/test-results.locked/` naming** — Was this directory renamed from `test-results/` intentionally (to preserve a specific Playwright run for debugging) or accidentally? If intentional, document the "locked" convention and add `**/test-results.locked/` to `.gitignore`. If accidental, rename back to `test-results/` — the existing root `**/test-results/` glob will cover it. _(Raised by: Tobias, Sara)_
Author
Owner

CLEANUP-4 — Closing summary

All hygiene items resolved. git status --short after the PR merge will show only docs/superpowers/ and familienarchiv-408/ as untracked (both pre-existing, not new).

Files untracked from git (git rm --cached):

Item What it was Resolution
frontend/e2e/.auth/user.json Dev admin credential (Basic Auth token) Untracked; frontend/.gitignore rule e2e/.auth/ already present — the rule now takes effect
proofshot-artifacts/ (44 files, ~7.6MB) Browser verification screenshots from proofshot skill Untracked + proofshot-artifacts/ added to root .gitignore
frontend/.svelte-kit.old/types/src/routes/.stammbaum-stale/$types.d.ts Stale type stub from route rename Untracked + directory deleted from disk + **/.svelte-kit.old/ added to frontend/.gitignore
frontend/test-results.locked/e2e/ (2 files) Playwright E2E run artifacts Untracked + directory deleted from disk + **/test-results.locked/ added to frontend/.gitignore
node_modules/.vite/vitest/.../results.json Vite test cache committed by mistake Untracked + root node_modules/ added to .gitignore
package.json + package-lock.json at root 3 testing-library devDeps with no justification for root placement Deleted from disk and untracked

.gitignore additions:

Root .gitignore:

proofshot-artifacts/
node_modules/

frontend/.gitignore:

**/test-results.locked/
**/.svelte-kit.old/

Decisions recorded:

  • proofshot-artifacts/: Option A — gitignore + delete local (run artifacts, never commit)
  • frontend/test-results.locked/: Gitignored with explicit entry; directory deleted
  • Root package.json: Deleted — no documented justification for root placement

Implemented in commit d28c4559 on branch feat/issue-411-legibility-cleanup.

## CLEANUP-4 — Closing summary All hygiene items resolved. `git status --short` after the PR merge will show only `docs/superpowers/` and `familienarchiv-408/` as untracked (both pre-existing, not new). **Files untracked from git (`git rm --cached`):** | Item | What it was | Resolution | |------|-------------|------------| | `frontend/e2e/.auth/user.json` | Dev admin credential (Basic Auth token) | Untracked; `frontend/.gitignore` rule `e2e/.auth/` already present — the rule now takes effect | | `proofshot-artifacts/` (44 files, ~7.6MB) | Browser verification screenshots from proofshot skill | Untracked + `proofshot-artifacts/` added to root `.gitignore` | | `frontend/.svelte-kit.old/types/src/routes/.stammbaum-stale/$types.d.ts` | Stale type stub from route rename | Untracked + directory deleted from disk + `**/.svelte-kit.old/` added to `frontend/.gitignore` | | `frontend/test-results.locked/e2e/` (2 files) | Playwright E2E run artifacts | Untracked + directory deleted from disk + `**/test-results.locked/` added to `frontend/.gitignore` | | `node_modules/.vite/vitest/.../results.json` | Vite test cache committed by mistake | Untracked + root `node_modules/` added to `.gitignore` | | `package.json` + `package-lock.json` at root | 3 testing-library devDeps with no justification for root placement | Deleted from disk and untracked | **`.gitignore` additions:** Root `.gitignore`: ``` proofshot-artifacts/ node_modules/ ``` `frontend/.gitignore`: ``` **/test-results.locked/ **/.svelte-kit.old/ ``` **Decisions recorded:** - `proofshot-artifacts/`: Option A — gitignore + delete local (run artifacts, never commit) - `frontend/test-results.locked/`: Gitignored with explicit entry; directory deleted - Root `package.json`: Deleted — no documented justification for root placement Implemented in commit `d28c4559` on branch `feat/issue-411-legibility-cleanup`.
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: marcel/familienarchiv#415