Skip to content

chore: remove the public boundary check from the repo - #54

Merged
gabchess merged 2 commits into
mainfrom
chore/cut-boundary-check
Sep 26, 2026
Merged

gabchess merged 2 commits into
mainfrom
chore/cut-boundary-check

Conversation

@gabchess

@gabchess gabchess commented Sep 26, 2026 •

Copy link
Copy Markdown
Owner

What changed

Removes the public repository boundary check from the repository:

  • Deleted scripts/check-public-boundary.sh and scripts/test-public-boundary.sh.
  • Removed the "Test public repository boundary" and "Check public repository boundary" steps from .github/workflows/ci.yml.
  • Removed the public:check and public:test npm scripts from package.json.
  • Deleted .github/pull_request_template.md.
  • Removed the "Public repository boundary" section from CONTRIBUTING.md, and the closing sentence about it in the pull request bar section.
  • Updated a comment in app/revoke-demo-lib.ts that referenced the deleted script. No functional line in that file changed.
  • Added a ### Removed entry under Unreleased in CHANGELOG.md.

.github/ now holds only workflows/ci.yml.

Why

The check and its script are removed from the repository. Contributors no longer need to run it, and CI no longer runs it.

How to run it

yarn install --frozen-lockfile
yarn sdk:build
yarn consult:build
npm ci --ignore-scripts --prefix mcp
yarn mcp:build
npm ci --prefix app

yarn consult:test   # 500 passing
yarn mcp:test       # 165 passing
npm --prefix app test
./node_modules/.bin/tsc -p app/tsconfig.json --noEmit
yarn lint

Verification

  • yarn consult:test: 500 passing.
  • yarn mcp:test: 165 passing.
  • npm --prefix app test: 51 passing, 0 failing.
  • App typecheck (tsc -p app/tsconfig.json --noEmit): clean.
  • yarn lint (Prettier check): clean.
  • git diff main -- app/revoke-demo-lib.ts touches comment lines only; PERSONAL_NOTES_VAULT_DIR_NAME and every other line are unchanged.
  • .github/workflows/ci.yml still parses as valid YAML; package.json still parses as valid JSON.
  • A repository-wide search for the removed script names and npm script names, after this change, only matches the new CHANGELOG line.

No changes to consult/src, mcp/src, or any tagged release artifact.

Summary by CodeRabbit

  • Chores
    • Removed the public repository boundary checks from the pull request template, contributor guidance, and CI workflow.
    • Removed the related npm scripts and checker tests.

@coderabbitai

coderabbitai Bot commented Sep 26, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 05f6d707-ba73-4397-a02b-0f98b5137d3c

📥 Commits

Reviewing files that changed from the base of the PR and between e0864b5 and c213c1c.

📒 Files selected for processing (8)
  • .github/pull_request_template.md
  • .github/workflows/ci.yml
  • CHANGELOG.md
  • CONTRIBUTING.md
  • app/revoke-demo-lib.ts
  • package.json
  • scripts/check-public-boundary.sh
  • scripts/test-public-boundary.sh
💤 Files with no reviewable changes (5)
  • .github/pull_request_template.md
  • scripts/test-public-boundary.sh
  • .github/workflows/ci.yml
  • package.json
  • scripts/check-public-boundary.sh

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 6 remain after this review.

📜 Recent review details
⏰ Context from checks skipped due to timeout. (2)
  • GitHub Check: test
  • GitHub Check: test
🔇 Additional comments (3)
CONTRIBUTING.md (1)

60-60: LGTM!

CHANGELOG.md (1)

10-10: LGTM!

app/revoke-demo-lib.ts (1)

98-100: LGTM!


📝 Walkthrough

Walkthrough

The pull request removes the public-boundary checker and test, their npm scripts and CI steps, and related contributor guidance and references. The runtime directory-checking logic remains unchanged.

Changes

Public-boundary check removal

Layer / File(s) Summary
Remove checker, test, and execution wiring
scripts/*, package.json, .github/workflows/ci.yml
The checker and its test are deleted. The npm scripts and CI steps that ran them are removed.
Remove guidance and references
.github/pull_request_template.md, CONTRIBUTING.md, CHANGELOG.md, app/revoke-demo-lib.ts
The template and contributor guide no longer include public-boundary requirements. The changelog records the removal, and a code comment no longer refers to the check.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Other

Merge Risk: ⚪ Minimal · up to c213c

This change removes the public-boundary checks and their associated guidance and CI steps. The remaining requirement to keep README and threat-model claims aligned with public guarantees is unaffected; no concrete merge-blocking issue is identified.

Architecture Summary

Architecture risk: 🔵 Low · up to c213c

The change affects 5 systems.

Changed systems: scripts, app, CHANGELOG.md, CONTRIBUTING.md, package.json

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — scripts (service) was modified; 2 changed files map to changed impact.
  • observed — app (service) was modified; 1 changed file maps to changed impact.
  • observed — CHANGELOG.md (service) was modified; 1 changed file maps to changed impact.
  • observed — CONTRIBUTING.md (service) was modified; 1 changed file maps to changed impact.

Before / after behavior

  • observed — Modified behavior in CHANGELOG.md: The Unreleased “Removed” entry now includes the public:check and public:test npm scripts and their CI steps.
  • observed — Modified behavior in CONTRIBUTING.md: The “Public repository boundary” section and its content rules, public-evidence requirements, and yarn public:check instructions were removed. The pull request bar remains, but its instruction to confirm the boundary was deleted.
  • observed — Modified behavior in app/revoke-demo-lib.ts: The comment now describes the denied directory as a personal notes-vault app and omits the prior reference to the repository’s public-boundary check and its script; executable code is unchanged.
  • observed — Modified behavior in package.json: The public:check and public:test script entries were removed.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely identifies the main change: removing the public boundary check from the repository.
Description check ✅ Passed The description is complete and relevant. It explains what changed, why it changed, how to run validation, and which tests and checks passed. The removed public repository check template is not reprod…
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. (2 skipped: 2 …
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.
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

A rabbit hops past the checklist gate
The scripts are gone; the docs grow light
No boundary test runs in the CI
The vault-check code stays as before
A quiet thump, then off to nibble greens

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

@gabchess
gabchess merged commit 9d1382d into main Sep 26, 2026
5 checks passed
@gabchess
gabchess deleted the chore/cut-boundary-check branch September 26, 2026 18:09
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