Skip to content

DES-374: Make lotus-audit-ios output portable (gitignored repo path + LOTUS_AUDIT_MIRROR)#2

Merged
OJBoon merged 3 commits into
mainfrom
feat/lotus-audit-ios-output-paths
May 7, 2026
Merged

DES-374: Make lotus-audit-ios output portable (gitignored repo path + LOTUS_AUDIT_MIRROR)#2
OJBoon merged 3 commits into
mainfrom
feat/lotus-audit-ios-output-paths

Conversation

@OJBoon
Copy link
Copy Markdown
Collaborator

@OJBoon OJBoon commented May 7, 2026

Jira: DES-374 — Lotus Design System (Epic)

Summary

The previous default wrote audit reports to a path inside Olly's Obsidian vault — the skill was unrunnable for anyone else who clones the repo. This PR moves to the standard pattern that tools like pytest (--junitxml), cargo (target/), and npm run build (dist/) use: a gitignored repo-relative default + environment-variable mirror for users who want a personal copy elsewhere.

What changed

Output resolution (priority order):

  1. Primary path<lotus-repo>/audits/ios/YYYY-MM-DD.md by default, overridable with --out <path>. The audits/ directory is gitignored so weekly runs don't bloat git history. Milestone snapshots can be committed deliberately with git add -f.
  2. Mirror copy — if LOTUS_AUDIT_MIRROR is set, also writes a copy there. Olly sets this in his ~/.zshrc to his vault path; team members don't, so no vault writes happen for them.
  3. Vault index update — only triggered when the mirror lands inside Olly's Obsidian Lotus Audits folder. Other mirror destinations (Notion, Slack drop folders, etc.) handle their own indexing.

Files touched:

  • .gitignore — adds audits/ with a comment explaining the convention.
  • .claude/skills/lotus-audit-ios/SKILL.md:
    • Description rewritten — no longer claims vault-output as the default.
    • New ## Output destinations section near the top so contributors see the path resolution before they read any phase.
    • Phase 5 rewritten with explicit shell snippets for primary + mirror + index update.
    • Important rules updated — default must be team-portable; [[wikilinks]] only when the output is going to a vault.

Why this pattern

Industry survey of how tools handle generated reports:

Pattern Examples Fit
stdout, user redirects npm audit, eslint, clippy Too unstructured for a multi-page report
Repo-local, gitignored coverage/, dist/, .pytest_cache/, target/ Strong fit. Predictable + portable + no accidental commits
CI artifact only Lighthouse CI, SonarQube Wrong fit — this is human-triggered
GitHub Issue body Renovate Dependency Dashboard Heavyweight; loses git-history drift tracking
Committed milestones Trail of Bits style audits/ dirs Right for some runs, not weekly
External KB (Notion/Confluence/Obsidian) Most internal tools Personal — not portable

This PR adopts pattern #2 as the default with a hook into #6 for users who want it.

Test plan

  • Re-run node .claude/skills/lotus-audit-ios/scripts/fetch-figma-tokens.mjs /tmp/test.json — still works (verified locally).
  • Confirm audits/ is gitignored.
  • Once parkhub/ios is cloned locally, run /lotus-audit-ios — verify the report lands at audits/ios/YYYY-MM-DD.md AND (with LOTUS_AUDIT_MIRROR set) at the mirror path.

Notes

  • v1 only handles iOS audits. When lotus-audit-android and lotus-audit-web ship later, the same LOTUS_AUDIT_MIRROR convention extends naturally — audits/<platform>/ paths.
  • Users who want every audit committed can override gitignore per-file (git add -f path/to.md) on the runs they care about. Default is still "don't commit."

🤖 Generated with Claude Code

The previous default wrote to a path inside Olly's Obsidian vault, which
made the skill unrunnable for anyone who clones this repo. Move to the
industry-standard pattern (gitignored repo-local + env-var mirror):

Output resolution (priority order):

1. Primary path — `<lotus-repo>/audits/ios/YYYY-MM-DD.md` by default,
   overridable with `--out <path>`. The `audits/` dir is gitignored so
   weekly runs don't pollute git history. Milestone snapshots can be
   committed deliberately with `git add -f`.

2. Mirror copy — if `LOTUS_AUDIT_MIRROR` is set, also writes a copy
   there. This is how individual users (e.g. Olly's vault) opt in to a
   personal knowledge-base destination without baking that path into
   the skill.

3. Vault index — only updated when the mirror lands inside Olly's
   Obsidian Lotus folder. Other mirror destinations handle their own
   indexing.

Same pattern as `pytest`'s `--junitxml`, `npm run build`'s `dist/`,
`cargo`'s `target/`. Default works for anyone; users opt into
extra destinations.

Files:
- .gitignore — add `audits/` (with note explaining the convention)
- .claude/skills/lotus-audit-ios/SKILL.md
  - Description rewritten — no longer claims vault-output as the default
  - New `## Output destinations` section near the top
  - Phase 5 rewritten with explicit path-resolution shell snippets
  - Important rules updated — default must be team-portable;
    Obsidian-only syntax (`[[wikilinks]]`) only when output is in vault

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@gitstream-cm
Copy link
Copy Markdown

gitstream-cm Bot commented May 7, 2026

🚨 Warning: Approaching Monthly Automation Limit

Monthly PRs automated: 244/250

Your organization has used over 90% of its monthly quota for gitStream automations. Once the quota is reached, new pull requests for this month will not trigger gitStream automations and will be marked as “Skipped”.

To avoid interruptions, consider optimizing your automation usage or upgrading your plan by contacting LinearB.

Copy link
Copy Markdown

@gitstream-cm gitstream-cm Bot left a comment

Choose a reason for hiding this comment

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

✨ PR Review

The PR successfully makes the output path portable and team-friendly. However, there's a logic bug in the file suffix increment code that will create malformed filenames when duplicate dates exist.

1 issues detected:

🐞 Bug - The ${PRIMARY%.md} operation inside the loop strips the extension from an already-modified path, causing suffixes to accumulate incorrectly 🛠️

Details: The file suffix increment logic has a bug that repeatedly modifies the same variable, creating malformed filenames. When a file like 'audits/ios/2026-05-07.md' exists and its '-2' variant also exists, the code produces 'audits/ios/2026-05-07-2-3.md' instead of 'audits/ios/2026-05-07-3.md'. This affects both the primary path (lines 333-338) and mirror path (lines 348-356) implementations.

File: .claude/skills/lotus-audit-ios/SKILL.md (334-336)
🛠️ A suggested code correction is included in the review comments.

Generated by LinearB AI and added by gitStream.
AI-generated content may contain inaccuracies. Please verify before using.
💡 Tip: You can customize your AI Review using Guidelines Learn how

Comment thread .claude/skills/lotus-audit-ios/SKILL.md Outdated
Comment on lines +334 to +336
i=2
while [ -e "$PRIMARY" ]; do
PRIMARY="${PRIMARY%.md}-$i.md"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🐞 Bug - Incorrect Suffix Logic: Store the base path (without .md extension) before entering the while loop. Inside the loop, construct PRIMARY from the base path plus the current suffix: BASE="${PRIMARY%.md}"; i=2; while [ -e "$PRIMARY" ]; do PRIMARY="$BASE-$i.md"; i=$((i+1)); done. Apply the same fix to the mirror path logic.

Suggested change
i=2
while [ -e "$PRIMARY" ]; do
PRIMARY="${PRIMARY%.md}-$i.md"
BASE="${PRIMARY%.md}"
i=2
while [ -e "$PRIMARY" ]; do
PRIMARY="$BASE-$i.md"
Is this review accurate? Use 👍 or 👎 to rate it

If you want to tell us more, use /gs feedback e.g. /gs feedback this review doesn't make sense, I disagree, and it keeps repeating over and over

Capture the base path (sans `.md` extension) once before the
collision-check loop. The previous version reassigned PRIMARY/MIRROR
each iteration via `${PRIMARY%.md}-$i.md`, so a third iteration would
produce `2026-05-07-2-3.md` instead of `2026-05-07-3.md`.

Apply the same fix to the mirror-path loop.

Caught by LinearB AI review on #2.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@OJBoon
Copy link
Copy Markdown
Collaborator Author

OJBoon commented May 7, 2026

Addressed PR comments

LinearB AI review — bug in suffix loop — ✅ fixed in 31bbc22.

The reviewer correctly spotted that PRIMARY="${PRIMARY%.md}-$i.md" mutates PRIMARY each iteration, so a third collision produced 2026-05-07-2-3.md instead of 2026-05-07-3.md. Captured the base path once before the loop and append the suffix to it on each iteration. Same fix applied to the mirror-path loop.

gitstream-cm quota warning — informational only; no action required from this PR.

Copy link
Copy Markdown

@gitstream-cm gitstream-cm Bot left a comment

Choose a reason for hiding this comment

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

✨ PR Review

The PR successfully implements portable output paths with proper gitignore handling and environment variable support. The previously reported suffix increment bug has been resolved. One functional issue remains in the vault index update logic.

1 issues detected:

🐞 Bug - The wikilink format doesn't match the actual filename when suffixes are applied. 🛠️

Details: When multiple audits run on the same day, the mirror file receives a suffix (e.g., 2026-05-07-2.md), but the vault index update instruction hardcodes the format [[Audits/YYYY-MM-DD]] without accounting for the suffix, creating a broken wikilink that won't resolve to the actual file.

File: .claude/skills/lotus-audit-ios/SKILL.md (370-370)
🛠️ A suggested code correction is included in the review comments.

Generated by LinearB AI and added by gitStream.
AI-generated content may contain inaccuracies. Please verify before using.
💡 Tip: You can customize your AI Review using Guidelines Learn how

Comment thread .claude/skills/lotus-audit-ios/SKILL.md Outdated
bullet to that vault's `_Index.md` under `## Audits`:

```
- [[Audits/YYYY-MM-DD]] — auto-generated
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🐞 Bug - Broken Wikilink: Extract the actual filename from the $MIRROR variable (using basename and stripping the .md extension) and use it in the wikilink instead of the hardcoded YYYY-MM-DD format. For example: MIRROR_NAME=$(basename "$MIRROR" .md) then use [[Audits/$MIRROR_NAME]].

Suggested change
- [[Audits/YYYY-MM-DD]] — auto-generated
- [[Audits/$(basename "$MIRROR" .md)]] — auto-generated
Is this review accurate? Use 👍 or 👎 to rate it

If you want to tell us more, use /gs feedback e.g. /gs feedback this review doesn't make sense, I disagree, and it keeps repeating over and over

The vault `_Index.md` update hardcoded `[[Audits/YYYY-MM-DD]]` which
breaks if the same-day collision loop appended a suffix (e.g.
`2026-05-07-2.md`). The wikilink would point at a non-existent note.

Use `basename "$MIRROR" .md` to get the actual filename so the link
always resolves.

Caught by LinearB AI review on #2.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@OJBoon
Copy link
Copy Markdown
Collaborator Author

OJBoon commented May 7, 2026

LinearB AI review — broken wikilink with suffix — ✅ fixed in latest commit.

Replaced the hardcoded [[Audits/YYYY-MM-DD]] template with [[Audits/$(basename "$MIRROR" .md)]] so when a same-day collision produces e.g. 2026-05-07-2.md, the wikilink resolves correctly.

Copy link
Copy Markdown

@gitstream-cm gitstream-cm Bot left a comment

Choose a reason for hiding this comment

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

✨ PR Review

LGTM

Generated by LinearB AI and added by gitStream.
AI-generated content may contain inaccuracies. Please verify before using.
💡 Tip: You can customize your AI Review using Guidelines Learn how

@OJBoon OJBoon merged commit bd54cc3 into main May 7, 2026
11 checks passed
@OJBoon OJBoon deleted the feat/lotus-audit-ios-output-paths branch May 7, 2026 15:18
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.

1 participant