Skip to content

Tweak flags language - #123

Merged
mislav merged 4 commits into
masterfrom
flags-language
Nov 27, 2019
Merged

mislav merged 4 commits into
masterfrom
flags-language

Conversation

@mislav

@mislav mislav commented Nov 27, 2019

Copy link
Copy Markdown
Contributor

Ref. #68

@mislav
mislav merged commit 92c9ab1 into master Nov 27, 2019
@mislav
mislav deleted the flags-language branch November 27, 2019 22:53
nobe4 added a commit that referenced this pull request Mar 1, 2024
Various places during the `gh pr merge` flow show the PR number and
title. Those places are updated to also show the owner/repo.

E.g.
Before:
  Pull request #123 (title) is ready to be merged
After:
  Pull request owner/repo#123 (title) is ready to be merged

There are other places, where only the number is displayed. Those were
intentionally left as is. It made sense to show the owner/repo only when
the extra context of the title was present.

It also fixes the related tests.

cc #8777
cchristous pushed a commit to cchristous/cli that referenced this pull request Feb 28, 2026
…e_switch

archive switch in snapshot cretate
BlakeHastings added a commit to BlakeHastings/b-fac that referenced this pull request Sep 25, 2026
… ones (#212)

Closes #202

The shipped skill and the lines its assets print cited this repository's
ADRs and issues by bare number. A host repository has its own
`docs/architecture/decisions/` from 0001, and GitHub resolves `#N`
against the repository it is read in, so those citations named the
host's decisions and issues. This qualifies every one in the payload and
adds `check:citations` so they stay qualified.

## The form, and why

- **ADRs: `b-fac ADR 0021`.** Six characters more per citation, and it
reads in a terminal. A full URL to the decisions directory is about 90
characters, and check-setup prints several of these in a report whose
lines are already about 85 wide. ADRs have no autolink on GitHub either,
so a URL would buy a click and cost the line.
- **Issues: `#122`.** This is GitHub's own
cross-repository reference. It renders as a link to the right issue
wherever GitHub renders markdown, and it reads unambiguously in a
terminal. The shorter `b-fac #122` was rejected because GitHub would
still autolink the `#122` in it to the *host's* issue 122, which is the
bug this fixes.

## Counts, before and after, by file (lint findings)

| File | Before | After |
| --- | --- | --- |
| `SKILL.md` | 3 | 0 |
| `references/backlog-port.md` | 4 | 0 |
| `references/beads-backlog.md` | 2 | 0 |
| `references/continuity.md` | 8 | 0 |
| `references/enforcement.md` | 16 | 0 (plus 1 illustrative, exempt) |
| `references/host-checks.md` | 12 | 0 |
| `references/parallelism.md` | 3 | 0 |
| `references/refinement.md` | 8 | 0 |
| `references/reviewing.md` | 2 | 0 |
| `assets/check-setup.mjs` (strings) | 15 | 0 |
| `assets/discover-checks.mjs` (strings) | 4 | 0 |
| `assets/check-outward-writes.mjs` (strings) | 1 | 0 |
| `assets/guard-guest-writes.mjs` (strings) | 8 | 8, **pending**, see
Not done |
| **Total** | **86** | **8, all pending** |

`references/first-run.md` is not counted. Its opening paragraph already
says every commit, issue, PR and ADR it names is b-fac's, at a URL it
gives, so it is exempt by name.

## Functionality

- `npm run check`: 996 tests pass, and the new step prints `Citation
check passed across 24 payload files` with `guard-guest-writes.mjs: 8
bare, pending` above it.
- **Failure path:** I put `ADR 0022` back bare in
`references/host-checks.md:46` and appended `see #150.` to line 158.
`node scripts/check-citations.mjs` exited 1 and named both, each with
its file:line, the line text, and the two qualified forms to use. After
I reverted them it exited 0.
- `check:version` (0.54.6 to 0.54.9), `check:plugin` and
`check:plugin-load` pass.
- check-setup's tests drive the real script in scratch repositories.
Four of them assert on the legacy-install lines, and they now match
`before #122`. One of the four was a `doesNotMatch`
that would have passed vacuously if left alone, so I updated it too.

## Code

`scripts/check-citations.mjs` scans everything under `.agents/skills/`.
The mirror is left to `check:sync`, as `check-reference-tables.mjs`
does.

**What counts as bare:**
- `ADR NNNN` or `ADRs NNNN` not preceded by `b-fac` and whitespace,
where the whitespace can be a newline so wrapped prose passes.
- `#N` with no `owner/repo` in front of it. `#122` is
ours and `cli/cli#123` is someone else's that says so.
- In `.mjs`, `.js` and `.py`, a line that is wholly a comment (`//`,
`/*`, `*`, or `#` for Python) is blanked before the scan. Every other
line is scanned, so string literals count. The brief decided that
comments stay bare, and the check holds exactly that and nothing more.

**What it must NOT flag**, all covered by tests:
- a qualified citation
- another repository's `owner/repo#N`
- a path like `docs/architecture/decisions/0018`
- `{`, `file.md#L12`, markdown headings
- a comment in an asset
- **placeholder numbers in examples**, meant to be read as the host's
own. These are the status-line template in SKILL.md (`#41`, `#42`,
`#43`, `#52`), the quoted clause `"I'll start #122 unless…"`, the other
project's brief in `briefing.md` (`#29`, `#59`), the `gh issue edit 78
--add-blocked-by 61` example in `github-backlog.md` (`#61`), and the
host's hypothetical `A branch adding ADR 0009` in `enforcement.md`.

Those placeholders are exempted by **exact phrase, not by number**. So
`#122` cited for real elsewhere in SKILL.md is still a finding, and a
test shows it. An exemption whose phrase has disappeared fails the
check, and so does a `PENDING` file with nothing left to fix, so neither
kind of exemption can outlive its reason.

## Architecture

- This is a sibling of `check-vocabulary.mjs`, not an extension of it.
Vocabulary scans the whole repo for banned words. This scans only the
payload and needs comment-awareness and per-phrase exemptions that
vocabulary does not.
- It is a new step inside the `Checks` job, not a new job, so no new
required check context is created (ADR 0009). It is also added to `npm
run check`, AGENTS.md's run table, review.md's Gate 0 list and
working-an-issue.md.
- The entry-point test is copied from `post-body.mjs`. No new
dependencies.
- This repository's own docs and ADRs stay bare, as decided.
- Only citations changed in the prose. Nothing was reflowed, so a few
lines now run a few characters longer. One sentence changed its wording:
in `enforcement.md`, "deleting the layer in its ADR 0001" became "in
b-fac ADR 0001", because "its" was the only thing qualifying that
citation.

## Not done

- **`assets/guard-guest-writes.mjs` still has 8 bare citations in
printed strings and records**, at lines 725, 1032, 1056, 1067 (`ADR
0021` and `#94`), 1111, 1211 and 1232. The brief put this file
off-limits because other agents are working in it. It is listed in
`PENDING` in the check, which reports its count on every run and goes
red once it has none, so the follow-up that fixes it has to delete the
entry.
- `guard-merge.mjs` and `merge-pr.mjs` were not touched. Neither has a
bare citation outside a comment, so there is nothing to follow up there.
- **Comments in the assets are left bare, by decision.** The issue
points out that a host operator reads them too (#185). If that is judged
worth fixing, it is a larger follow-up, roughly 100 comment lines across
the assets including the three off-limits files, and it needs one line
removed from the check.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 5.5 <[email protected]>
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.

2 participants