Skip to content

ci: stop docs-only PRs hanging on required checks - #34

Merged
psimaker merged 2 commits into
mainfrom
docs/readme-architecture-diagram
Jun 3, 2026
Merged

ci: stop docs-only PRs hanging on required checks#34
psimaker merged 2 commits into
mainfrom
docs/readme-architecture-diagram

Conversation

@psimaker

@psimaker psimaker commented Jun 3, 2026

Copy link
Copy Markdown
Owner

What & why

Two related changes — the docs change surfaced a latent CI problem, so both are fixed here.

Docs — architecture diagram

Replace the Mermaid flowchart in the README with two hand-authored, theme-aware SVGs
(docs/images/vaultsync-architecture-{dark,light}.svg), served via <picture> + prefers-color-scheme. Crisper
rendering, proper dark/light variants, and a full aria-label/alt description.

CI — stop docs-only PRs hanging on required checks

The required status checks Go Tests and Notify Tests live in ci.yml, which was skipped entirely on
docs/markdown-only changes via a top-level paths-ignore. Once those jobs became required in branch protection, such
PRs hung forever on "Expected — waiting for status to be reported" (a path-filtered workflow never reports a
status). This PR itself hit that.

Fix:

  • Drop paths-ignore so the workflow always triggers; the cheap ubuntu required checks (Go Tests, Notify Tests)
    now always report green — no reliance on skipped-check semantics.
  • Gate only the ~10x-priced macOS Build & Test job behind a dorny/paths-filter code detector (go/**,
    ios/**, notify/**, .github/**). Docs-only changes still skip the costly build; it is not a required check, so
    skipping never blocks a PR.
  • The plain if: expression (no always()) keeps the implicit "all needs succeeded" gate, so a failed test job still
    skips the build as before.

Notes

Because this branch touches .github/**, the code filter is true for this PR, so the macOS build runs once here
to validate the CI change itself.

@coderabbitai

coderabbitai Bot commented Jun 3, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The PR updates README.md's "How it works" section by replacing an embedded Mermaid flowchart with a centered HTML <picture> element. The new implementation serves a dark-mode SVG variant when available and falls back to a light-mode SVG, with explicit alt text for accessibility.

Changes

Documentation: Dark/Light Architecture Diagram

Layer / File(s) Summary
Architecture diagram visualization with dark/light SVG variants
README.md
The "How it works" Mermaid flowchart is replaced with a <picture> element that loads a dark-themed SVG on dark backgrounds and falls back to light-themed SVG, including alt text for accessibility.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

  • psimaker/vaultsync#3: Both PRs modify the architecture diagram in README.md; this PR replaces the Mermaid diagram with dark/light SVG variants while the related PR aligns the ASCII diagram.

Poem

🌓 The diagram now sees both day and night,
Dark SVGs blend seamlessly with light.
One picture holds two worlds in perfect grace,
Accessibility shines in every space.
From Mermaid's flow to vectors bright,
The architecture speaks in dark and light. ✨

🚥 Pre-merge checks | ✅ 6 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Title check ⚠️ Warning The PR title uses 'ci:' prefix but describes documentation changes (README diagram replacement), which should use 'docs:' prefix per conventional commit style. Change the title to 'docs: replace Mermaid architecture diagram with themed SVGs' to align the conventional commit prefix with the actual nature of the changes.
✅ Passed checks (6 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
No Private Note Leakage ✅ Passed PR replaces Mermaid diagram with SVG images in README.md. SVG files contain no credentials, API keys, vault paths, tokens, or sensitive data. No code changes affect logging or network requests.
Bounded Ios Background Work ✅ Passed This PR is documentation-only (README.md diagram update). The custom check for iOS background work changes is not applicable—no iOS code was modified.
Bridge Contract Compatibility ✅ Passed PR only modifies README.md documentation (replaces Mermaid diagram with SVG images). No Go bridge, Swift bridge-service code, or bridge contract changes detected; check applies only to bridge changes.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch docs/readme-architecture-diagram

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
README.md (1)

43-46: ⚡ Quick win

Align alt text with the SVG aria-labels for consistency.

The alt text on the <img> differs slightly from the aria-label attributes embedded in both SVG files. Screen reader users may hear different descriptions depending on whether the browser announces the alt or the SVG's aria-label.

Current <img> alt:

  • "your server with Syncthing"
  • "writes notes into your Obsidian vault"

SVG aria-label (from both dark/light variants):

  • "your server running Syncthing"
  • "places notes into your Obsidian vault"
  • "silent push wake-ups to the app" (slightly more complete)
♻️ Suggested fix to match the SVG aria-labels
-  <img alt="VaultSync architecture: your server with Syncthing syncs peer-to-peer with the VaultSync iOS app, which writes notes into your Obsidian vault. An optional Cloud Relay sends silent push wake-ups." src="docs/images/vaultsync-architecture-light.svg" width="100%">
+  <img alt="VaultSync architecture: your server running Syncthing syncs peer-to-peer with the VaultSync iOS app, which places notes into your Obsidian vault. An optional Cloud Relay sends silent push wake-ups to the app." src="docs/images/vaultsync-architecture-light.svg" width="100%">
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@README.md` around lines 43 - 46, Update the <img> alt text so it exactly
matches the aria-labels embedded in both SVGs: change the description to
"VaultSync architecture: your server running Syncthing syncs peer-to-peer with
the VaultSync iOS app, which places notes into your Obsidian vault. An optional
Cloud Relay sends silent push wake-ups to the app." Locate the <picture> block
and the <img> element and replace the existing alt string to ensure screen
readers hear the same text regardless of whether the browser exposes the <img>
alt or the SVG aria-label.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@README.md`:
- Around line 43-46: Update the <img> alt text so it exactly matches the
aria-labels embedded in both SVGs: change the description to "VaultSync
architecture: your server running Syncthing syncs peer-to-peer with the
VaultSync iOS app, which places notes into your Obsidian vault. An optional
Cloud Relay sends silent push wake-ups to the app." Locate the <picture> block
and the <img> element and replace the existing alt string to ensure screen
readers hear the same text regardless of whether the browser exposes the <img>
alt or the SVG aria-label.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 7cacdbd5-9950-4209-acc5-2ece3b753f8c

📥 Commits

Reviewing files that changed from the base of the PR and between 6c41df3 and 659dfc9.

⛔ Files ignored due to path filters (2)
  • docs/images/vaultsync-architecture-dark.svg is excluded by !**/*.svg, !**/*.svg and included by **/*
  • docs/images/vaultsync-architecture-light.svg is excluded by !**/*.svg, !**/*.svg and included by **/*
📒 Files selected for processing (1)
  • README.md
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
**/*

⚙️ CodeRabbit configuration file

**/*: VaultSync syncs private Obsidian notes through Syncthing. Treat data loss,
privacy leaks, security regressions, and broken sync behavior as high priority.
Do not nitpick formatting unless it affects maintainability, correctness, or public API clarity.
Flag any accidental logging, telemetry, crash reporting, or network transfer of note contents,
vault paths, filenames with private context, API keys, APNs tokens, relay keys, or security-scoped bookmark data.

Files:

  • README.md
**/*.md

⚙️ CodeRabbit configuration file

**/*.md: Review public documentation for technical accuracy, privacy/security claims, App Store-facing wording,
setup correctness, and consistency with the free app plus optional Cloud Relay subscription model.

Files:

  • README.md
🧠 Learnings (5)
📓 Common learnings
Learnt from: CR
Repo: psimaker/vaultsync

Timestamp: 2026-06-03T13:06:57.552Z
Learning: License all code under MPL-2.0 (Mozilla Public License 2.0)
Learnt from: CR
Repo: psimaker/vaultsync

Timestamp: 2026-06-03T13:06:57.552Z
Learning: Ensure the relay server only sees the Device ID and push token, never notes, file names, folder names, or vault structure
📚 Learning: 2026-05-31T08:18:03.446Z
Learnt from: CR
Repo: psimaker/vaultsync PR: 0
File: docs/architecture.md:0-0
Timestamp: 2026-05-31T08:18:03.446Z
Learning: VaultSync background sync is intentionally asymmetric: Server→iPhone sync is accelerated via Cloud Relay push notifications (server-side file watch + APNs); iPhone→Server sync relies on foreground execution in the VaultSync app, with background refresh as opportunistic only—not guaranteed real-time.

Applied to files:

  • README.md
📚 Learning: 2026-05-31T08:18:03.446Z
Learnt from: CR
Repo: psimaker/vaultsync PR: 0
File: docs/architecture.md:0-0
Timestamp: 2026-05-31T08:18:03.446Z
Learning: Applies to docs/notify/** : Cloud Relay is an optional Docker sidecar service that watches Syncthing on the homeserver for outgoing changes and sends APNs silent-push wake-ups to the iPhone. Refer to relay-spec.md for protocol details and troubleshooting.md for end-user issues.

Applied to files:

  • README.md
📚 Learning: 2026-05-31T08:19:08.076Z
Learnt from: CR
Repo: psimaker/vaultsync PR: 0
File: docs/troubleshooting.md:0-0
Timestamp: 2026-05-31T08:19:08.076Z
Learning: Install and open Obsidian at least once on the iOS device before attempting to connect the Obsidian folder in VaultSync

Applied to files:

  • README.md
📚 Learning: 2026-05-31T08:19:08.076Z
Learnt from: CR
Repo: psimaker/vaultsync PR: 0
File: docs/troubleshooting.md:0-0
Timestamp: 2026-05-31T08:19:08.076Z
Learning: Capture VaultSync version, iOS version, **Sync Issues** screenshot, **Relay Diagnostics** screenshot, and `vaultsync-notify --doctor` output when filing bug reports

Applied to files:

  • README.md

The required status checks Go Tests and Notify Tests live in ci.yml, which
was skipped entirely on docs/markdown-only changes via top-level paths-ignore.
Once those jobs became required in branch protection, such PRs hung forever on
"Expected — waiting for status" because a path-filtered workflow never reports.

Drop paths-ignore so the workflow always runs; the cheap ubuntu required checks
now always report green. Gate only the ~10x-priced macOS Build & Test job behind
a dorny/paths-filter "code" detector, so docs-only changes still skip the costly
build (it is not a required check, so skipping never blocks).
@psimaker psimaker changed the title docs: replace Mermaid architecture diagram with themed SVGs docs+ci: themed architecture diagram & fix docs-only required checks Jun 3, 2026
@psimaker psimaker changed the title docs+ci: themed architecture diagram & fix docs-only required checks ci: stop docs-only PRs hanging on required checks Jun 3, 2026
@psimaker
psimaker merged commit 0857e6d into main Jun 3, 2026
13 of 14 checks passed
@psimaker
psimaker deleted the docs/readme-architecture-diagram branch June 3, 2026 13:38
psimaker added a commit that referenced this pull request Jun 3, 2026
The Detect changes job ran dorny/paths-filter without a checkout. On
pull_request events that is fine — it lists changed files via the GitHub
API. But on push events (the merge of #34 to main) there is no PR to
query, so paths-filter falls back to a local git diff and needs a working
copy. With no .git present its first git command failed:

  fatal: not a git repository
  The process '/usr/bin/git' failed with exit code 128

This surfaced only after merge, since #34 introduced the job itself and
push-to-main was the first push event to exercise it. Add actions/checkout
as the first step so the job works on both push and pull_request.

Co-authored-by: psimaker <psimaker@users.noreply.github.com>
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