From 46bbd3df217e338760562ed5f43748a8628e092a Mon Sep 17 00:00:00 2001 From: Stoney Jackson Date: Tue, 13 Oct 2026 14:56:54 +0000 Subject: [PATCH 01/13] fix(check): require --root and check the repo that runs the hooks Co-Authored-By: Claude Opus 5.5 --- .githooks/pre-commit | 2 +- .githooks/pre-push | 2 +- bin/check.py | 7 ++++--- tests/test_check.py | 31 +++++++++++++++++++++++++++++++ 4 files changed, 37 insertions(+), 5 deletions(-) diff --git a/.githooks/pre-commit b/.githooks/pre-commit index fa0a3d4..1583be0 100755 --- a/.githooks/pre-commit +++ b/.githooks/pre-commit @@ -1,2 +1,2 @@ #!/bin/sh -exec python3 "$(git rev-parse --show-toplevel)/bin/check.py" +exec python3 "$(dirname "$0")/../bin/check.py" --root "$(git rev-parse --show-toplevel)" diff --git a/.githooks/pre-push b/.githooks/pre-push index d64a6be..e4e51bd 100755 --- a/.githooks/pre-push +++ b/.githooks/pre-push @@ -1,2 +1,2 @@ #!/bin/sh -exec python3 "$(git rev-parse --show-toplevel)/bin/check.py" --pre-push +exec python3 "$(dirname "$0")/../bin/check.py" --root "$(git rev-parse --show-toplevel)" --pre-push diff --git a/bin/check.py b/bin/check.py index 2ae4809..bbc0588 100755 --- a/bin/check.py +++ b/bin/check.py @@ -1,8 +1,8 @@ #!/usr/bin/env python3 """Consistency checks for the ourPLCC change-request tracker. -Standard library only: this runs on maintainers' hosts (pre-push hook) as -well as in containers, without Node or Backlog.md. +Standard library only: this runs from the hooks and in CI without Node or +Backlog.md. --root names the tracker repo (ourPLCC/change-requests). """ import argparse import re @@ -132,7 +132,8 @@ def rel(path): def main(argv=None): parser = argparse.ArgumentParser(description=__doc__.splitlines()[0]) - parser.add_argument("--root", default=str(Path(__file__).resolve().parent.parent)) + parser.add_argument("--root", required=True, + help="root of the tracker repo, which holds backlog/") parser.add_argument("--pre-push", action="store_true", help="also fail on uncommitted changes under backlog/") args = parser.parse_args(argv) diff --git a/tests/test_check.py b/tests/test_check.py index fc14e70..388f50b 100644 --- a/tests/test_check.py +++ b/tests/test_check.py @@ -142,6 +142,37 @@ def test_cli_exit_code_on_error(self): self.assertEqual(result.returncode, 1) self.assertIn("check: ", result.stderr) + def test_cli_requires_root(self): + result = subprocess.run( + [sys.executable, str(ROOT / "bin" / "check.py")], + capture_output=True, text=True) + self.assertEqual(result.returncode, 2) + self.assertIn("--root", result.stderr) + self.assertNotIn("Traceback", result.stderr) + + def hook(self, name): + git = ["git", "-c", "user.name=t", "-c", "user.email=t@t", "-C", str(self.root)] + subprocess.run(git + ["init", "-q"], check=True) + subprocess.run(git + ["add", "-A"], check=True) + subprocess.run(git + ["commit", "-q", "-m", "x"], check=True) + return subprocess.run(["sh", str(ROOT / ".githooks" / name)], + cwd=self.root, capture_output=True, text=True) + + def test_pre_commit_hook_checks_the_repo_that_runs_it(self): + self.add("tasks", "cr-1000 - A.md", record("CR-1000", project="nope")) + result = self.hook("pre-commit") + self.assertEqual(result.returncode, 1, result.stdout + result.stderr) + self.assertIn("project", result.stderr) + + def test_pre_push_hook_checks_the_repo_that_runs_it(self): + result = self.hook("pre-push") + self.assertEqual(result.returncode, 0, result.stderr) + self.add("tasks", "cr-1000 - A.md", record("CR-1000")) + result = subprocess.run(["sh", str(ROOT / ".githooks" / "pre-push")], + cwd=self.root, capture_output=True, text=True) + self.assertEqual(result.returncode, 1) + self.assertIn("uncommitted change under backlog/", result.stderr) + if __name__ == "__main__": unittest.main() From ee6bd378dcdebc745ba404ebeb3e1c235539eee8 Mon Sep 17 00:00:00 2001 From: Stoney Jackson Date: Tue, 13 Oct 2026 14:58:28 +0000 Subject: [PATCH 02/13] ci: check ourPLCC/change-requests from dev's CI Also say in the docs that the tracker data lives in change-requests. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/check.yml | 10 +++++++++- CONTRIBUTING.md | 3 ++- README.md | 10 ++++++---- dev-docs/tracker.md | 18 +++++++++++------- 4 files changed, 28 insertions(+), 13 deletions(-) diff --git a/.github/workflows/check.yml b/.github/workflows/check.yml index 38b5653..94aa9de 100644 --- a/.github/workflows/check.yml +++ b/.github/workflows/check.yml @@ -4,11 +4,18 @@ on: push: pull_request: +permissions: + contents: read + jobs: check: runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 + - uses: actions/checkout@v4 + with: + repository: ourPLCC/change-requests + path: change-requests - uses: actions/setup-python@v5 with: python-version: "3.12" @@ -22,6 +29,7 @@ jobs: python3 -m unittest discover -s tests -t . python3 -m unittest discover -s migrate/tests -t . - name: Consistency check - run: python3 bin/check.py --pre-push + run: python3 bin/check.py --pre-push --root change-requests - name: Backlog.md doctor + working-directory: change-requests run: backlog doctor diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 06791cd..c4a84b1 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -16,7 +16,8 @@ Repo docs link to this guide rather than restating it, even to agree with it. reports a problem or suggests an idea by opening an issue on the relevant code repo. - **Change requests (CRs)** are the work we commit to, tracked centrally in - this repo with [Backlog.md](https://github.com/MrLesk/Backlog.md). A CR is + [ourPLCC/change-requests](https://github.com/ourPLCC/change-requests) with + [Backlog.md](https://github.com/MrLesk/Backlog.md). A CR is work in exactly one repo with testable acceptance criteria, and has an ID `CR-N`. Work spanning repos is one CR per repo, linked by dependencies. - **Drafts** are ideas we have not committed to. Never cite one. diff --git a/README.md b/README.md index f5c4338..b84f563 100644 --- a/README.md +++ b/README.md @@ -1,13 +1,15 @@ -# ourPLCC developer docs and change requests +# ourPLCC developer docs How we build ourPLCC, for maintainers, their agents, and new developers: the developer guide shared by every ourPLCC repository, org-wide specs, agent -rules, and the central change-request tracker for work on plcc-ng, -languages-ng, plcc-ng-demo, plcc-ng-devcontainer, and course-materials-ng. +rules, and the tooling for the change-request tracker. The tracker itself, for +work on plcc-ng, languages-ng, plcc-ng-demo, plcc-ng-devcontainer, and +course-materials-ng, is +[ourPLCC/change-requests](https://github.com/ourPLCC/change-requests). **Reporting a problem or suggesting an idea?** Open a GitHub issue on the relevant code repository instead. GitHub Issues are where humans talk to us; -this repository is where maintainers track the resulting work. +the tracker is where maintainers track the resulting work. **Start with [CONTRIBUTING.md](CONTRIBUTING.md)**, the developer guide. diff --git a/dev-docs/tracker.md b/dev-docs/tracker.md index 40d2567..e8351af 100644 --- a/dev-docs/tracker.md +++ b/dev-docs/tracker.md @@ -2,7 +2,9 @@ The central tracker for work on the ourPLCC code repositories: plcc-ng, languages-ng, plcc-ng-demo, plcc-ng-devcontainer, and course-materials-ng. It -is managed with [Backlog.md](https://github.com/MrLesk/Backlog.md). This is +is managed with [Backlog.md](https://github.com/MrLesk/Backlog.md). Its data +lives in [ourPLCC/change-requests](https://github.com/ourPLCC/change-requests); +its checks and tooling live here, in `dev`. This is the full guide to using it; the [developer guide](../CONTRIBUTING.md) gives the short version and the workflow around it. @@ -59,7 +61,8 @@ this guide wins. decision that binds more than one repo); update the CR's acceptance criteria to match and add the spec as a reference: `backlog task edit CR-N --add-ref ../../../plcc-ng/dev-docs/specs/.md` - (for this repo: `../../dev-docs/specs/.md`). Specs do not cite CRs. + (for this repo: `../../../dev/dev-docs/specs/.md`). Specs do not cite + CRs. 2. **Plan and execute:** the plan lives in `.plans/` inside the worktree (gitignored) and is deleted with it. Plans are not kept. During automated execution, record discovered work in the CR's notes — @@ -102,7 +105,7 @@ dependency as satisfied, so review the CRs that depended on it. - **Demote** (`backlog task demote CR-N`) only when nothing cites the CR: ```bash - grep -rn 'CR-N\b' /workspaces/dev/backlog /workspaces/ --include='*.md' + grep -rn 'CR-N\b' /workspaces/change-requests/backlog /workspaces/ --include='*.md' git -C /workspaces/ log --all --oneline -i --grep 'cr-N\b' ``` @@ -124,7 +127,8 @@ version impact; its branch may carry other commit types too. Every CR has exactly one `project`: `plcc-ng`, `languages-ng`, `plcc-ng-demo`, `plcc-ng-devcontainer`, `course-materials-ng`, or `dev` (this repo: the org -docs and specs, the tracker's config, checks, CI, and migration tooling). Work spanning repos is one +docs and specs, and the tracker's checks, CI, and migration tooling; the +config is `change-requests`' `backlog/config.yml`). Work spanning repos is one CR per repo linked with `--dep` (no umbrella CRs); `backlog task list --ready` hides a CR until its dependencies are done. @@ -172,7 +176,7 @@ to renumber the unpushed CR, commit, and push. CI runs the same check plus ## Checks (`bin/check.py`) -Fails on: duplicate IDs (drafts included) or a filename that does not match its `id`; a +`bin/check.py --root ` fails on: duplicate IDs (drafts included) or a filename that does not match its `id`; a `DRAFT-N` cited outside its own file; a CR without exactly one configured `project` or `type`; an unconfigured label; anything in `archive/`; and, with `--pre-push`, uncommitted changes under `backlog/`. `CR-999` is exempt from @@ -222,8 +226,8 @@ checks run on hosts as well as in containers. Run what CI runs ```bash python3 -m unittest discover -s tests -t . python3 -m unittest discover -s migrate/tests -t . -python3 bin/check.py --pre-push -backlog doctor +python3 bin/check.py --pre-push --root /workspaces/change-requests +(cd /workspaces/change-requests && backlog doctor) ``` A new rule in `bin/check.py` gets a test in `tests/test_check.py` and a line From 44979c79734542127d5a2ef2a9e0952299b840be Mon Sep 17 00:00:00 2001 From: Stoney Jackson Date: Tue, 13 Oct 2026 15:06:13 +0000 Subject: [PATCH 03/13] fix(check): say so when --root holds no tracker Co-Authored-By: Claude Opus 5.5 --- bin/check.py | 2 ++ tests/test_check.py | 10 ++++++++++ 2 files changed, 12 insertions(+) diff --git a/bin/check.py b/bin/check.py index bbc0588..8866b18 100755 --- a/bin/check.py +++ b/bin/check.py @@ -71,6 +71,8 @@ def _as_list(value): def check(root, pre_push=False): root = Path(root) backlog = root / "backlog" + if not (backlog / "config.yml").is_file(): + return [f"{root}: no backlog/config.yml; --root must name the tracker repo"] config = load_config(backlog / "config.yml") projects = _as_list(config.get("projects")) types = _as_list(config.get("types")) diff --git a/tests/test_check.py b/tests/test_check.py index 388f50b..1f6a687 100644 --- a/tests/test_check.py +++ b/tests/test_check.py @@ -150,6 +150,16 @@ def test_cli_requires_root(self): self.assertIn("--root", result.stderr) self.assertNotIn("Traceback", result.stderr) + def test_root_without_tracker_is_a_clear_error(self): + empty = self.root / "not-a-tracker" + empty.mkdir() + result = subprocess.run( + [sys.executable, str(ROOT / "bin" / "check.py"), "--root", str(empty)], + capture_output=True, text=True) + self.assertEqual(result.returncode, 1) + self.assertIn("no backlog/config.yml", result.stderr) + self.assertNotIn("Traceback", result.stderr) + def hook(self, name): git = ["git", "-c", "user.name=t", "-c", "user.email=t@t", "-C", str(self.root)] subprocess.run(git + ["init", "-q"], check=True) From fd30f01f6ac76a4aa2cbbf77c3e06b8ad6ee1bf6 Mon Sep 17 00:00:00 2001 From: Stoney Jackson Date: Tue, 13 Oct 2026 17:10:25 +0000 Subject: [PATCH 04/13] feat(push): push change-requests under the shared lock after checks Co-Authored-By: Claude Opus 5.5 --- bin/push.bash | 60 +++++++++++++ tests/test_push.py | 207 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 267 insertions(+) create mode 100755 bin/push.bash create mode 100644 tests/test_push.py diff --git a/bin/push.bash b/bin/push.bash new file mode 100755 index 0000000..cd81d45 --- /dev/null +++ b/bin/push.bash @@ -0,0 +1,60 @@ +#!/usr/bin/env bash +# Push the change-request tracker (ourPLCC/change-requests): fetch, rebase +# onto origin/main, pass bin/check.py --pre-push, push. Holds a lock on this +# repo's root, which in a devcontainer is the ourplcc-dev volume's mount +# point, the same lock the code repos' bin/install/shared-clones.bash takes. +set -euo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +DEV_ROOT="$(dirname "${SCRIPT_DIR}")" +TRACKER="${OURPLCC_CHANGE_REQUESTS:-/workspaces/change-requests}" + +die() { + echo "push.bash: $*" >&2 + exit 1 +} + +git -C "${TRACKER}" rev-parse --git-dir >/dev/null 2>&1 \ + || die "${TRACKER} is not a git clone of ourPLCC/change-requests" + +# Re-run under the lock. -o keeps the lock's descriptor in flock alone, so a +# child that outlives this script (git maintenance, detached by fetch) does +# not hold it; -w turns a stuck holder into an error. +# OURPLCC_LOCK_WAIT shortens the wait, for tests. +if [ -z "${OURPLCC_PUSH_LOCKED:-}" ]; then + status=0 + OURPLCC_PUSH_LOCKED=1 flock -o -w "${OURPLCC_LOCK_WAIT:-120}" -E 75 \ + "${DEV_ROOT}" "$0" "$@" || status=$? + [ "${status}" -ne 75 ] \ + || die "timed out waiting for the lock on ${DEV_ROOT};" \ + "another push or clone holds it" + exit "${status}" +fi +unset OURPLCC_PUSH_LOCKED + +cd "${TRACKER}" + +[ "$(git symbolic-ref --quiet --short HEAD || true)" = main ] \ + || die "${TRACKER} is not on main; check out main first" +[ -z "$(git status --porcelain)" ] \ + || die "${TRACKER} has uncommitted changes; commit or discard them first" + +git fetch --quiet origin +if ! git rebase --quiet origin/main; then + git rebase --abort + die "rebase onto origin/main conflicted; aborted, local commits kept." \ + "Resolve with 'git pull --rebase' in ${TRACKER}, then run this again" +fi + +python3 "${DEV_ROOT}/bin/check.py" --pre-push --root "${TRACKER}" \ + || die "bin/check.py failed; nothing pushed. For a duplicate id," \ + "run 'backlog doctor --fix', commit, and run this again" + +if [ "$(git rev-list --count origin/main..HEAD)" -eq 0 ]; then + echo "push.bash: nothing to push" + exit 0 +fi + +git push --quiet origin main \ + || die "push rejected; if another push landed since the fetch, run this again" +echo "push.bash: pushed" diff --git a/tests/test_push.py b/tests/test_push.py new file mode 100644 index 0000000..5738bf6 --- /dev/null +++ b/tests/test_push.py @@ -0,0 +1,207 @@ +import fcntl +import os +import subprocess +import tempfile +import time +import unittest +from pathlib import Path + +from tests.test_check import CONFIG, record + +ROOT = Path(__file__).resolve().parent.parent +PUSH = ROOT / "bin" / "push.bash" + + +class PushTest(unittest.TestCase): + def setUp(self): + tmp = tempfile.TemporaryDirectory() + self.addCleanup(tmp.cleanup) + self.tmp = Path(tmp.name) + self.origin = self.tmp / "origin.git" + self.tracker = self.tmp / "tracker" + self.other = self.tmp / "other" + # Isolate from the user's git config (signing, helpers, hooksPath). + self.env = {k: v for k, v in os.environ.items() if not k.startswith("GIT_")} + self.env.update( + GIT_CONFIG_GLOBAL=os.devnull, + GIT_CONFIG_NOSYSTEM="1", + GIT_AUTHOR_NAME="Test", + GIT_AUTHOR_EMAIL="test@example.com", + GIT_COMMITTER_NAME="Test", + GIT_COMMITTER_EMAIL="test@example.com", + OURPLCC_CHANGE_REQUESTS=str(self.tracker), + ) + self.git("init", "-q", "--bare", "-b", "main", str(self.origin)) + seed = self.tmp / "seed" + self.git("init", "-q", "-b", "main", str(seed)) + self.write(seed, "backlog/config.yml", CONFIG) + self.write(seed, "backlog/completed/cr-999 - Seed.md", + record("CR-999", project=None, type_=None)) + self.commit(seed, "Seed") + self.git("-C", str(seed), "push", "-q", str(self.origin), "main") + self.git("clone", "-q", str(self.origin), str(self.tracker)) + self.git("clone", "-q", str(self.origin), str(self.other)) + + def git(self, *args): + return subprocess.run(["git", *args], env=self.env, check=True, + capture_output=True, text=True).stdout + + def write(self, repo, rel, text): + path = repo / rel + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(text) + + def commit(self, repo, message): + self.git("-C", str(repo), "add", "-A") + self.git("-C", str(repo), "commit", "-q", "-m", message) + + def add_cr(self, repo, n, title="Title"): + self.write(repo, f"backlog/tasks/cr-{n} - {title}.md", record(f"CR-{n}")) + self.commit(repo, f"Create task CR-{n} {title}") + + def push_other(self): + self.git("-C", str(self.other), "push", "-q", "origin", "main") + + def run_push(self): + return subprocess.run([str(PUSH)], env=self.env, capture_output=True, text=True) + + def origin_log(self): + return self.git("--git-dir", str(self.origin), "log", "--format=%s", "main") + + def test_pushes_local_commits(self): + self.add_cr(self.tracker, 1000) + result = self.run_push() + self.assertEqual(result.returncode, 0, result.stderr) + self.assertIn("Create task CR-1000 Title", self.origin_log()) + + def test_rebases_onto_commits_pushed_elsewhere(self): + self.add_cr(self.other, 1001) + self.push_other() + self.add_cr(self.tracker, 1000) + result = self.run_push() + self.assertEqual(result.returncode, 0, result.stderr) + log = self.origin_log() + self.assertIn("Create task CR-1000 Title", log) + self.assertIn("Create task CR-1001 Title", log) + self.assertEqual(self.git("-C", str(self.tracker), "rev-parse", "HEAD"), + self.git("--git-dir", str(self.origin), "rev-parse", "main")) + + def test_nothing_to_push_exits_zero(self): + result = self.run_push() + self.assertEqual(result.returncode, 0, result.stderr) + self.assertIn("nothing to push", result.stdout) + + def test_refuses_uncommitted_changes(self): + self.add_cr(self.tracker, 1000) + self.write(self.tracker, "backlog/tasks/cr-1001 - Title.md", record("CR-1001")) + result = self.run_push() + self.assertNotEqual(result.returncode, 0) + self.assertIn("uncommitted", result.stderr) + self.assertNotIn("CR-1000", self.origin_log()) + + def test_rebase_conflict_aborts_and_keeps_local_commits(self): + self.write(self.other, "backlog/config.yml", CONFIG + "statuses: [a]\n") + self.commit(self.other, "Other config") + self.push_other() + self.write(self.tracker, "backlog/config.yml", CONFIG + "statuses: [b]\n") + self.commit(self.tracker, "Tracker config") + result = self.run_push() + self.assertNotEqual(result.returncode, 0) + self.assertIn("conflicted", result.stderr) + self.assertFalse((self.tracker / ".git" / "rebase-merge").exists()) + self.assertFalse((self.tracker / ".git" / "rebase-apply").exists()) + self.assertEqual( + self.git("-C", str(self.tracker), "log", "-1", "--format=%s").strip(), + "Tracker config") + self.assertNotIn("Tracker config", self.origin_log()) + + def test_duplicate_id_after_rebase_blocks_push(self): + self.add_cr(self.other, 1000, title="Other") + self.push_other() + self.add_cr(self.tracker, 1000) + result = self.run_push() + self.assertNotEqual(result.returncode, 0) + self.assertIn("duplicate id CR-1000", result.stderr) + self.assertIn("backlog doctor --fix", result.stderr) + self.assertNotIn("CR-1000 Title", self.origin_log()) + + def test_rejected_push_says_run_again(self): + hook = self.origin / "hooks" / "pre-receive" + hook.parent.mkdir(exist_ok=True) + hook.write_text("#!/bin/sh\nexit 1\n") + hook.chmod(0o755) + self.add_cr(self.tracker, 1000) + result = self.run_push() + self.assertNotEqual(result.returncode, 0) + self.assertIn("run this again", result.stderr) + + def test_refuses_branch_other_than_main(self): + self.git("-C", str(self.tracker), "switch", "-q", "-c", "topic") + self.add_cr(self.tracker, 1000) + result = self.run_push() + self.assertNotEqual(result.returncode, 0) + self.assertIn("not on main", result.stderr) + self.assertNotIn("CR-1000", self.origin_log()) + + def test_refuses_detached_head(self): + self.git("-C", str(self.tracker), "switch", "-q", "--detach") + result = self.run_push() + self.assertNotEqual(result.returncode, 0) + self.assertIn("not on main", result.stderr) + + def test_missing_tracker_clone_explains(self): + self.env["OURPLCC_CHANGE_REQUESTS"] = str(self.tmp / "missing") + result = self.run_push() + self.assertNotEqual(result.returncode, 0) + self.assertIn("not a git clone", result.stderr) + + def test_waits_for_the_lock(self): + self.add_cr(self.tracker, 1000) + fd = os.open(ROOT, os.O_RDONLY) + try: + fcntl.flock(fd, fcntl.LOCK_EX) + proc = subprocess.Popen([str(PUSH)], env=self.env, + stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True) + time.sleep(0.5) + self.assertIsNone(proc.poll(), "push.bash ran while the lock was held") + self.assertNotIn("CR-1000", self.origin_log()) + finally: + fcntl.flock(fd, fcntl.LOCK_UN) + os.close(fd) + _, stderr = proc.communicate(timeout=30) + self.assertEqual(proc.returncode, 0, stderr) + self.assertIn("Create task CR-1000 Title", self.origin_log()) + + def test_gives_up_on_a_stuck_lock(self): + self.add_cr(self.tracker, 1000) + self.env["OURPLCC_LOCK_WAIT"] = "1" + fd = os.open(ROOT, os.O_RDONLY) + try: + fcntl.flock(fd, fcntl.LOCK_EX) + result = self.run_push() + finally: + fcntl.flock(fd, fcntl.LOCK_UN) + os.close(fd) + self.assertNotEqual(result.returncode, 0) + self.assertIn("timed out waiting for the lock", result.stderr) + self.assertNotIn("CR-1000", self.origin_log()) + + def test_child_that_outlives_it_does_not_keep_the_lock(self): + # Stands in for the git maintenance a fetch detaches. + hook = self.tracker / ".git" / "hooks" / "pre-push" + hook.parent.mkdir(exist_ok=True) + hook.write_text("#!/bin/sh\n(sleep 5 >/dev/null 2>&1 Date: Tue, 13 Oct 2026 17:10:41 +0000 Subject: [PATCH 05/13] build: check out every file with LF line endings Co-Authored-By: Claude Opus 5.5 --- .gitattributes | 1 + 1 file changed, 1 insertion(+) create mode 100644 .gitattributes diff --git a/.gitattributes b/.gitattributes new file mode 100644 index 0000000..6313b56 --- /dev/null +++ b/.gitattributes @@ -0,0 +1 @@ +* text=auto eol=lf From e3c31aca7b1216336be1548af6626a343a413c33 Mon Sep 17 00:00:00 2001 From: Stoney Jackson Date: Tue, 13 Oct 2026 17:11:25 +0000 Subject: [PATCH 06/13] docs(agents): forbid merging, gh auth, and pushing the tracker directly Co-Authored-By: Claude Opus 5.5 --- ORG-AGENTS.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/ORG-AGENTS.md b/ORG-AGENTS.md index 0a22b6d..38242a8 100644 --- a/ORG-AGENTS.md +++ b/ORG-AGENTS.md @@ -19,3 +19,8 @@ because the reader is an agent. (`backlog task edit CR-N --append-notes "…"`), never as new CRs. - When closing the CR, triage those notes with the human: new draft, new CR, fold into an existing CR, or drop. +- Never merge a pull request. +- Never run `gh auth login`, `gh auth refresh`, or `gh auth token`; if `gh` + is not logged in, stop and tell the human. +- Push `change-requests` only through `dev`'s `bin/push.bash`, and only to + publish tracker changes the human approved. From eb01c118966cfff775008f6f1217f1f2ffbc661c Mon Sep 17 00:00:00 2001 From: Stoney Jackson Date: Tue, 13 Oct 2026 17:12:01 +0000 Subject: [PATCH 07/13] docs(tracker): push from the container with bin/push.bash Co-Authored-By: Claude Opus 5.5 --- dev-docs/tracker.md | 70 +++++++++++++++++++++++++++++++++------------ 1 file changed, 52 insertions(+), 18 deletions(-) diff --git a/dev-docs/tracker.md b/dev-docs/tracker.md index e8351af..3556c17 100644 --- a/dev-docs/tracker.md +++ b/dev-docs/tracker.md @@ -14,17 +14,21 @@ this guide wins. ## Setup -- Keep the ourPLCC repos as siblings on your host, with this repo cloned - beside them as `dev/`. Each code repo's devcontainer clones it there if - missing, bind-mounts it at `/workspaces/dev`, sets - `BACKLOG_CWD=/workspaces/dev`, and installs the pinned - `backlog` CLI. -- Commit inside the container; **push from the host**. Backlog.md commits - every change automatically (`auto_commit: true`). +- Each code repo's devcontainer clones this repo into the shared volume + `ourplcc-dev`, mounted at `/workspaces/dev`, and the tracker data, + [ourPLCC/change-requests](https://github.com/ourPLCC/change-requests), + into `ourplcc-change-requests`, mounted at `/workspaces/change-requests`. + Every ourPLCC container on a machine shares both. It sets + `BACKLOG_CWD=/workspaces/change-requests` and installs the pinned `backlog` + CLI, and on start fast-forwards `dev`'s `main` when it is checked out and + clean. +- Commit and push inside the container, with `/workspaces/dev/bin/push.bash`, + after every tracker change. Backlog.md commits every change automatically + (`auto_commit: true`); the volume is not a backup. - Hooks in `.githooks/` run `bin/check.py` (Python 3, standard library) on - commit and push; `core.hooksPath` is set by the devcontainer's - `initializeCommand`. -- Hosts: Linux or macOS. + commit and push; the devcontainer sets `change-requests`' `core.hooksPath` + to `/workspaces/dev/.githooks`, so the hooks come from reviewed code. +- Hosts: Linux, macOS, or Windows; or a Codespace. ## Vocabulary @@ -168,11 +172,34 @@ projects before filing. ## Pushing and collisions -Push from the host. If the push is rejected, `git pull --rebase`, then -`python3 bin/check.py`. If it reports a duplicate ID (two people allocated the -same number before either pushed), run `backlog doctor --fix` in a container -to renumber the unpushed CR, commit, and push. CI runs the same check plus -`backlog doctor` on every push; it can only alert, so fix problems by revert. +Push with `/workspaces/dev/bin/push.bash`. It takes a lock that every ourPLCC +container on the machine shares, fetches, rebases your tracker commits onto +`origin/main`, runs `bin/check.py --pre-push`, and pushes. It stops without +pushing, and says why, when: + +- `change-requests` has uncommitted changes: commit or discard them. +- The rebase conflicts: it aborts the rebase and keeps your commits. Run + `git pull --rebase` in `/workspaces/change-requests`, resolve, and run it + again. +- `bin/check.py` fails. A duplicate ID means two people allocated the same + number before either pushed: run `backlog doctor --fix` to renumber the + unpushed CR, commit, and run it again. +- The push is rejected because another push landed after the fetch: run it + again. +- It waited two minutes for the lock: another push or a container start + holds it. Run it again; if it keeps timing out, look for a stuck + `bin/push.bash` or `shared-clones.bash` in your other containers. + +Do not edit the tracker from another container while a push runs. Backlog.md +does not take the lock, so its commit can land in the middle of the rebase +and be dropped. + +Locally, every container shares one tracker tree, so one person's sessions +never allocate the same ID. Two Codespaces, or a Codespace and a local +container, are two trees and can collide like two people. Nothing on GitHub +checks a direct push to `change-requests`; the next `bin/push.bash` fails on +a bad one and names the problem, and `dev`'s CI checks `change-requests` on +every push to `dev`. Fix problems by revert. ## Checks (`bin/check.py`) @@ -191,7 +218,9 @@ the project and type rules. 3. Fold relevant changes into this guide (and, where they touch agent rules, into [ORG-AGENTS.md](../ORG-AGENTS.md)) and update the version it was checked against — in the same commit. -4. Rebuild devcontainers to pick up the new version. +4. Once the change merges, rebuild every ourPLCC container to pick up the new + version; each warns on start while its `backlog` differs from + `.backlog-version`. ## Migration tooling @@ -220,8 +249,10 @@ that call `backlog` set `BACKLOG_CWD` to their target explicitly. ## Working on the tracker tooling `bin/check.py` and `migrate/` use only the Python 3 standard library, so the -checks run on hosts as well as in containers. Run what CI runs -(`.github/workflows/check.yml`) before pushing a tooling change: +hooks run in every container without extra installs. Changes to `dev` go +through a pull request; its `main` takes no direct pushes. In a container, +run what CI runs (`.github/workflows/check.yml`) before pushing a tooling +change: ```bash python3 -m unittest discover -s tests -t . @@ -232,3 +263,6 @@ python3 bin/check.py --pre-push --root /workspaces/change-requests A new rule in `bin/check.py` gets a test in `tests/test_check.py` and a line in [Checks](#checks-bincheckpy) above. + +`bin/push.bash`'s tests in `tests/test_push.py` build throwaway repos; change +them with the script. From 0cc5e71db25fd8f03ecfb7d9186a6256ef203c89 Mon Sep 17 00:00:00 2001 From: Stoney Jackson Date: Tue, 13 Oct 2026 17:12:19 +0000 Subject: [PATCH 08/13] docs(contributing): explain working in a devcontainer Co-Authored-By: Claude Opus 5.5 --- CONTRIBUTING.md | 70 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 70 insertions(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index c4a84b1..fc023b9 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -59,3 +59,73 @@ guide — filing, statuses, IDs, references, checks — is in the repo's git log. A CR's type is the commit type of the change that decides its version impact (`fix` → patch, `feat` → minor); its branch may carry other commit types too. + +## Working in a devcontainer + +Each ourPLCC code repo has a devcontainer. In it, you and any agent you run +act on GitHub as you, through one token of yours, and GitHub's settings, not +the agent's instructions, set the limits. The design and its reasons are in +[the devcontainer permissions spec](dev-docs/specs/2026-10-13-devcontainer-permissions-design.md). + +### Your token + +1. On GitHub, create a fine-grained personal access token (Settings → + Developer settings → Personal access tokens → Fine-grained tokens): + - Resource owner: `ourPLCC`. Repository access: all repositories. + - Repository permissions: Contents read and write, and Pull requests read + and write. Nothing else. + - Expiration: 90 days at most. +2. Wait for an org owner to approve it; until then it cannot reach ourPLCC. +3. In any ourPLCC container, run `gh auth login --with-token` and paste the + token. Every ourPLCC container on the machine shares the login, and it + survives rebuilds. + +Never use plain `gh auth login`: its browser flow yields a token for every +repo you can reach. When the token expires, make a new one, have it approved, +and load it again. To revoke it, delete it on GitHub; to forget it on one +machine, run `docker volume rm ourplcc-gh` on the host. + +### What the container cannot do + +From the container you can push branches, open and update pull requests, and +push tracker changes with `/workspaces/dev/bin/push.bash`. You cannot push to +any repo's `main` except `change-requests`', to `gh-pages`, or tags; push a +change to `.github/workflows/`; run or re-run workflows; or change settings, +rulesets, environments, or secrets. Do those in GitHub's web UI, or on the +host with your own credentials. + +Merging a pull request is yours alone: in a code repo, merging to `main` is +the release. Agents never merge. + +### Approving a release + +A release reaches PyPI only after you approve it. Open the release's workflow +run in the repo's Actions tab, choose **Review deployments**, select `pypi`, +and approve. The token cannot approve, so no agent can. + +### Codespaces + +A Codespace uses its own GitHub token; do not run `gh auth login` there. When +you create one, GitHub asks you to authorize access to `ourPLCC/dev` and +`ourPLCC/change-requests`: authorize it, or both stay read-only and +`bin/push.bash` fails. A full rebuild of a Codespace deletes its volumes and +any tracker commits you have not pushed, so push right away. + +### Moving from the host clone + +Before the devcontainers cloned `dev` into a volume, it lived on the host +beside the code repos, and tracker changes were pushed from there. Move once: + +1. In the host's `../dev`, check that `git status`, + `git log --branches --not --remotes`, `git worktree list`, and + `git stash list` come back clean. `dev`'s `main` no longer takes direct + pushes, so re-enter any tracker change they show by hand, in the + container, after step 3. +2. Create your token and wait for its approval. +3. Rebuild one ourPLCC container; it clones `dev` and `change-requests` into + their volumes. Run `gh auth login --with-token`, then push your next + tracker change with `/workspaces/dev/bin/push.bash`. +4. On a Windows host, check out each code repo again + (`git rm -r --cached -q . && git reset --hard`), or clone it fresh. +5. Delete the host's `../dev`. Do not check out a branch of any repo from + before the move, or its devcontainer recreates `../dev`. From 98d4696ce059402bf00a6237f406a9a53d661088 Mon Sep 17 00:00:00 2001 From: Stoney Jackson Date: Tue, 13 Oct 2026 17:18:22 +0000 Subject: [PATCH 09/13] fix(push): lock /workspaces/dev and explain every failure in one line Lock the volume's mount point rather than the script's own repo, so a worktree's copy still serializes with other containers; tests lock a temp dir through OURPLCC_LOCK_DIR. Reject an unfinished clone, a rebase already in progress, and a rebase that fails without a conflict with a push.bash: message instead of raw git errors, never leaving a rebase half done. Point a failed push at git's message and the token. Warn before CONTRIBUTING's reset --hard. Co-Authored-By: Claude Opus 5.5 --- CONTRIBUTING.md | 7 ++++-- bin/push.bash | 38 ++++++++++++++++++++-------- tests/test_push.py | 63 +++++++++++++++++++++++++++++++++++++++++++--- 3 files changed, 92 insertions(+), 16 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index fc023b9..e7d19c9 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -125,7 +125,10 @@ beside the code repos, and tracker changes were pushed from there. Move once: 3. Rebuild one ourPLCC container; it clones `dev` and `change-requests` into their volumes. Run `gh auth login --with-token`, then push your next tracker change with `/workspaces/dev/bin/push.bash`. -4. On a Windows host, check out each code repo again - (`git rm -r --cached -q . && git reset --hard`), or clone it fresh. +4. On a Windows host, once each code repo's branch has the + `* text=auto eol=lf` line in its `.gitattributes`, commit or stash your + work there, since the next command discards uncommitted changes. Then + check the repo out again (`git rm -r --cached -q . && git reset --hard`), + or clone it fresh. 5. Delete the host's `../dev`. Do not check out a branch of any repo from before the move, or its devcontainer recreates `../dev`. diff --git a/bin/push.bash b/bin/push.bash index cd81d45..77fc240 100755 --- a/bin/push.bash +++ b/bin/push.bash @@ -1,32 +1,35 @@ #!/usr/bin/env bash # Push the change-request tracker (ourPLCC/change-requests): fetch, rebase -# onto origin/main, pass bin/check.py --pre-push, push. Holds a lock on this -# repo's root, which in a devcontainer is the ourplcc-dev volume's mount -# point, the same lock the code repos' bin/install/shared-clones.bash takes. +# onto origin/main, pass bin/check.py --pre-push, push. Holds a lock on the +# ourplcc-dev volume's mount point, /workspaces/dev, the same lock the code +# repos' bin/install/shared-clones.bash takes. set -euo pipefail SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" DEV_ROOT="$(dirname "${SCRIPT_DIR}")" TRACKER="${OURPLCC_CHANGE_REQUESTS:-/workspaces/change-requests}" +LOCK_DIR="${OURPLCC_LOCK_DIR:-/workspaces/dev}" die() { echo "push.bash: $*" >&2 exit 1 } -git -C "${TRACKER}" rev-parse --git-dir >/dev/null 2>&1 \ - || die "${TRACKER} is not a git clone of ourPLCC/change-requests" +git -C "${TRACKER}" rev-parse --verify --quiet HEAD >/dev/null 2>&1 \ + || die "${TRACKER} is not a git clone of ourPLCC/change-requests" \ + "(missing, empty, or an unfinished clone);" \ + "rerun bin/install/shared-clones.bash" # Re-run under the lock. -o keeps the lock's descriptor in flock alone, so a # child that outlives this script (git maintenance, detached by fetch) does # not hold it; -w turns a stuck holder into an error. -# OURPLCC_LOCK_WAIT shortens the wait, for tests. +# OURPLCC_LOCK_DIR and OURPLCC_LOCK_WAIT are for tests. if [ -z "${OURPLCC_PUSH_LOCKED:-}" ]; then status=0 OURPLCC_PUSH_LOCKED=1 flock -o -w "${OURPLCC_LOCK_WAIT:-120}" -E 75 \ - "${DEV_ROOT}" "$0" "$@" || status=$? + "${LOCK_DIR}" bash "${BASH_SOURCE[0]}" "$@" || status=$? [ "${status}" -ne 75 ] \ - || die "timed out waiting for the lock on ${DEV_ROOT};" \ + || die "timed out waiting for the lock on ${LOCK_DIR};" \ "another push or clone holds it" exit "${status}" fi @@ -34,6 +37,14 @@ unset OURPLCC_PUSH_LOCKED cd "${TRACKER}" +rebase_in_progress() { + [ -d "$(git rev-parse --git-path rebase-merge)" ] \ + || [ -d "$(git rev-parse --git-path rebase-apply)" ] +} + +! rebase_in_progress \ + || die "a rebase is in progress in ${TRACKER}; finish it" \ + "(git rebase --continue) or run 'git rebase --abort', then run this again" [ "$(git symbolic-ref --quiet --short HEAD || true)" = main ] \ || die "${TRACKER} is not on main; check out main first" [ -z "$(git status --porcelain)" ] \ @@ -41,7 +52,12 @@ cd "${TRACKER}" git fetch --quiet origin if ! git rebase --quiet origin/main; then - git rebase --abort + rebase_in_progress \ + || die "rebase onto origin/main failed (see git's message above);" \ + "nothing changed; run this again" + git rebase --abort \ + || die "rebase onto origin/main conflicted and could not be aborted;" \ + "run 'git rebase --abort' in ${TRACKER}" die "rebase onto origin/main conflicted; aborted, local commits kept." \ "Resolve with 'git pull --rebase' in ${TRACKER}, then run this again" fi @@ -56,5 +72,7 @@ if [ "$(git rev-list --count origin/main..HEAD)" -eq 0 ]; then fi git push --quiet origin main \ - || die "push rejected; if another push landed since the fetch, run this again" + || die "push failed (see git's message above). If another push landed" \ + "since the fetch, run this again; if it is an authentication error," \ + "see 'Your token' in dev's CONTRIBUTING.md" echo "push.bash: pushed" diff --git a/tests/test_push.py b/tests/test_push.py index 5738bf6..b226070 100644 --- a/tests/test_push.py +++ b/tests/test_push.py @@ -21,7 +21,10 @@ def setUp(self): self.tracker = self.tmp / "tracker" self.other = self.tmp / "other" # Isolate from the user's git config (signing, helpers, hooksPath). - self.env = {k: v for k, v in os.environ.items() if not k.startswith("GIT_")} + self.lock_dir = self.tmp / "lock" + self.lock_dir.mkdir() + self.env = {k: v for k, v in os.environ.items() + if not k.startswith(("GIT_", "OURPLCC_"))} self.env.update( GIT_CONFIG_GLOBAL=os.devnull, GIT_CONFIG_NOSYSTEM="1", @@ -30,6 +33,7 @@ def setUp(self): GIT_COMMITTER_NAME="Test", GIT_COMMITTER_EMAIL="test@example.com", OURPLCC_CHANGE_REQUESTS=str(self.tracker), + OURPLCC_LOCK_DIR=str(self.lock_dir), ) self.git("init", "-q", "--bare", "-b", "main", str(self.origin)) seed = self.tmp / "seed" @@ -155,13 +159,64 @@ def test_missing_tracker_clone_explains(self): self.assertNotEqual(result.returncode, 0) self.assertIn("not a git clone", result.stderr) + def test_empty_tracker_dir_explains(self): + self.env["OURPLCC_CHANGE_REQUESTS"] = str(self.lock_dir) + result = self.run_push() + self.assertNotEqual(result.returncode, 0) + self.assertIn("not a git clone", result.stderr) + + def test_unfinished_clone_explains(self): + partial = self.tmp / "partial" + self.git("init", "-q", "-b", "main", str(partial)) + self.git("-C", str(partial), "remote", "add", "origin", str(self.origin)) + self.env["OURPLCC_CHANGE_REQUESTS"] = str(partial) + result = self.run_push() + self.assertNotEqual(result.returncode, 0) + self.assertIn("not a git clone", result.stderr) + + def test_rebase_failing_without_conflict_explains(self): + self.add_cr(self.other, 1001) + self.push_other() + self.add_cr(self.tracker, 1000) + (self.tracker / ".git" / "index.lock").write_text("") + result = self.run_push() + self.assertNotEqual(result.returncode, 0) + self.assertIn("push.bash: rebase onto origin/main failed", result.stderr) + self.assertFalse((self.tracker / ".git" / "rebase-merge").exists()) + self.assertNotIn("CR-1000", self.origin_log()) + + def test_rebase_in_progress_says_finish_it(self): + self.write(self.other, "backlog/config.yml", CONFIG + "statuses: [a]\n") + self.commit(self.other, "Other config") + self.push_other() + self.write(self.tracker, "backlog/config.yml", CONFIG + "statuses: [b]\n") + self.commit(self.tracker, "Tracker config") + self.git("-C", str(self.tracker), "fetch", "-q") + subprocess.run(["git", "-C", str(self.tracker), "rebase", "origin/main"], + env=self.env, capture_output=True) + result = self.run_push() + self.assertNotEqual(result.returncode, 0) + self.assertIn("a rebase is in progress", result.stderr) + self.assertTrue((self.tracker / ".git" / "rebase-merge").exists()) + + def test_failed_push_points_at_git_and_the_token(self): + hook = self.origin / "hooks" / "pre-receive" + hook.parent.mkdir(exist_ok=True) + hook.write_text("#!/bin/sh\nexit 1\n") + hook.chmod(0o755) + self.add_cr(self.tracker, 1000) + result = self.run_push() + self.assertIn("see git's message above", result.stderr) + self.assertIn("authentication", result.stderr) + def test_waits_for_the_lock(self): self.add_cr(self.tracker, 1000) - fd = os.open(ROOT, os.O_RDONLY) + fd = os.open(self.lock_dir, os.O_RDONLY) try: fcntl.flock(fd, fcntl.LOCK_EX) proc = subprocess.Popen([str(PUSH)], env=self.env, stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True) + self.addCleanup(proc.kill) time.sleep(0.5) self.assertIsNone(proc.poll(), "push.bash ran while the lock was held") self.assertNotIn("CR-1000", self.origin_log()) @@ -175,7 +230,7 @@ def test_waits_for_the_lock(self): def test_gives_up_on_a_stuck_lock(self): self.add_cr(self.tracker, 1000) self.env["OURPLCC_LOCK_WAIT"] = "1" - fd = os.open(ROOT, os.O_RDONLY) + fd = os.open(self.lock_dir, os.O_RDONLY) try: fcntl.flock(fd, fcntl.LOCK_EX) result = self.run_push() @@ -195,7 +250,7 @@ def test_child_that_outlives_it_does_not_keep_the_lock(self): self.add_cr(self.tracker, 1000) result = self.run_push() self.assertEqual(result.returncode, 0, result.stderr) - fd = os.open(ROOT, os.O_RDONLY) + fd = os.open(self.lock_dir, os.O_RDONLY) try: fcntl.flock(fd, fcntl.LOCK_EX | fcntl.LOCK_NB) # raises if still held fcntl.flock(fd, fcntl.LOCK_UN) From 662e85e28448d8907d5831c2563efb7d78377dd0 Mon Sep 17 00:00:00 2001 From: Stoney Jackson Date: Tue, 27 Oct 2026 12:10:52 +0000 Subject: [PATCH 10/13] docs(specs): record the devcontainer open-check results Settled on plcc-ng's branch: the forwarded SSH agent can only be hidden, so the boundary is keeping no GitHub key in the host agent; the Codespace token can create draft releases; its token expires about 12 hours after start and Codespaces' own helper is no fresher, so the fallback helper is dropped; volumes survive a Codespace restart; the lock crosses containers. Verification narrows to a smoke test for repos after plcc-ng. Co-Authored-By: Claude Opus 5.5 --- CONTRIBUTING.md | 9 +++ ...6-10-13-devcontainer-permissions-design.md | 70 ++++++++++++++----- 2 files changed, 61 insertions(+), 18 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index e7d19c9..307814d 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -85,6 +85,12 @@ repo you can reach. When the token expires, make a new one, have it approved, and load it again. To revoke it, delete it on GitHub; to forget it on one machine, run `docker volume rm ourplcc-gh` on the host. +Keep no SSH key that GitHub accepts loaded in your host's SSH agent; on the +host, use HTTPS with Git Credential Manager instead. VS Code forwards that +agent into every container, no setting turns it off, and the devcontainer can +only hide it, not block it, so a key there would let the container push +anywhere you can. + ### What the container cannot do From the container you can push branches, open and update pull requests, and @@ -111,6 +117,9 @@ you create one, GitHub asks you to authorize access to `ourPLCC/dev` and `bin/push.bash` fails. A full rebuild of a Codespace deletes its volumes and any tracker commits you have not pushed, so push right away. +A Codespace's token expires about 12 hours after the Codespace starts. If a +push or `gh` command then fails to authenticate, restart the Codespace. + ### Moving from the host clone Before the devcontainers cloned `dev` into a volume, it lived on the host diff --git a/dev-docs/specs/2026-10-13-devcontainer-permissions-design.md b/dev-docs/specs/2026-10-13-devcontainer-permissions-design.md index 548f69d..95e065d 100644 --- a/dev-docs/specs/2026-10-13-devcontainer-permissions-design.md +++ b/dev-docs/specs/2026-10-13-devcontainer-permissions-design.md @@ -98,6 +98,7 @@ below narrows it. | Force-push or delete `change-requests`' `main` | no | no | branch ruleset | | Push to any other repo's `main` (`dev` included), or to `gh-pages` | no | no | branch rulesets | | Push, move, or delete a tag | no | no | tag rulesets | +| Create, edit, or delete a draft release | yes | yes | — (publishing one creates its tag, which the tag rulesets block) | | Open, update, comment on, or close a PR | yes | yes | — | | Merge a PR | yes | **no** | ORG-AGENTS.md and Claude Code deny rules only | | Publish a package | no | no | `pypi` environment's required reviewer | @@ -273,9 +274,21 @@ Codespaces GPG verification for ourPLCC repos, and are unsigned otherwise. `"git.terminalAuthentication": false` and `"github.gitAuthentication": false` in `customizations.vscode.settings`. A push with no token fails instead of borrowing an unscoped one. -- Does not expose the host's SSH agent: SSH remotes would push with the host's - keys, unscoped. The likely mechanism is `"remoteEnv": {"SSH_AUTH_SOCK": ""}`; - see "Open checks". +- Hides the host's SSH agent, which SSH remotes would push through with the + host's keys, unscoped. VS Code forwards the agent into every container as + `/tmp/vscode-ssh-auth-.sock`, recreates the socket on every window + reload, and has no setting that turns this off. So the devcontainer sets + `"remoteEnv": {"SSH_AUTH_SOCK": ""}`, which stops `git` and `ssh` finding + the agent by default, and + `"postAttachCommand": "rm -f /tmp/vscode-ssh-auth-*.sock"`, which removes + the socket after each attach. That leaves the socket reachable while + `postStartCommand` runs, on every start (Dev Containers runs + `postAttachCommand` only after it), in the moment between an attach and + the removal, and to a process that survives a window reload. Hiding the + agent is therefore not the boundary. The rule is: keep no SSH key that + GitHub accepts loaded in the host's agent, and use HTTPS with Git + Credential Manager on the host. That rests on the contributor, and + `CONTRIBUTING.md` says so. - Uses one `remoteUser`, `vscode`, with UID 1000, like every other ourPLCC devcontainer, because the shared volumes keep real UIDs. @@ -309,7 +322,19 @@ same token. (for branches and PRs to `dev`); the Codespace's own repo is granted by default. GitHub asks for this grant when the Codespace is created. Choosing to continue without it leaves both read-only, so the docs say to authorize - it. A change to the grant reaches only Codespaces created afterwards. + it. A change to the grant reaches only Codespaces created afterwards. The + grant names a repo that must exist: before the split, GitHub lists + `ourPLCC/change-requests` as not available, and does not follow its + redirect to `dev`. +- The token is fixed in each process's environment when the process starts, + and it expires about 12 hours after the Codespace starts; a restart issues + a new one. Codespaces' own credential helper reads the same environment + variable, so a stock Codespace behaves the same way, and deferring to it + would gain nothing. So once a Codespace has run about 12 hours, pushes and + `gh` may fail to authenticate until it restarts. Whether a new terminal + gets a fresh token sooner is untested. A Codespace stops after its idle + timeout, and a stop ends every process, so this takes a Codespace kept + active that long. - So in a Codespace the scope is that repo plus `dev` and `change-requests`, narrower than local. The table above still holds within that scope. @@ -513,9 +538,13 @@ describes the volumes in place of the `../dev` clone, and drops plcc-ng's ## Verification -Each code repo's CR passes this checklist in a local container on a Linux or -macOS host, in a local container on a Windows host, and in a newly created -Codespace. +The first repo to adopt the shared setup, plcc-ng, passes this checklist in a +local container on a Linux or macOS host, in a local container on a Windows +host, and in a newly created Codespace. The other repos' devcontainer changes +copy plcc-ng's, so each of their CRs instead passes a smoke test in a rebuilt +local container: push a branch and open a PR with `gh`, list CRs with +`backlog task list`, push a tracker change with `bin/push.bash`, and see a +direct push to `main` rejected. Must succeed: @@ -547,17 +576,22 @@ Must hold: ## Open checks -Settled during step 1 of "Rollout" or the first repo's CR. A result that -contradicts this spec is fixed here, before the spec is frozen. - -- `remoteEnv` with an empty `SSH_AUTH_SOCK` stops git and `ssh` from reaching - the forwarded agent; if not, another way to stop VS Code forwarding it. -- The Codespace token cannot push workflow changes, create releases, or push - `gh-pages`. If it can, the table records the difference for Codespaces. -- A long-running process in a Codespace keeps working after the Codespace's - `GITHUB_TOKEN` is refreshed. If not, the helper becomes a small script that - defers to Codespaces' own helper when `$CODESPACES` is set. -- Codespaces honors `type=volume` mounts under `/workspaces`. +All settled on plcc-ng's branch before the split, on a macOS host and in a +Codespace; the sections above already reflect each result. + +- An empty `SSH_AUTH_SOCK` hides the forwarded agent but does not block it: + the socket stays in `/tmp`, and pointing `ssh-add` at it reaches the host. + Removing the socket in `postAttachCommand` closes that path except for the + gaps "Every devcontainer" lists, so the boundary is the rule about the + host's agent. +- The Codespace token can push a change to `.github/workflows/` and create a + draft release; it cannot push a tag or push `gh-pages`, which the rulesets + reject. Publishing a release was not tried; Verification step 7 covers it. +- A process in a Codespace keeps the token it started with, which expires + about 12 hours after the Codespace starts. Codespaces' own helper reads the + same variable, so the planned fallback to it is dropped. +- Codespaces honors `type=volume` mounts under `/workspaces`, and they + survive a restart. - `flock /workspaces/dev` in one local container blocks it in another container that mounts the same volume. From 26bf1aa5119621beb97ed91bdeac44ce9b3b81e3 Mon Sep 17 00:00:00 2001 From: Stoney Jackson Date: Tue, 27 Oct 2026 12:24:38 +0000 Subject: [PATCH 11/13] docs(specs): record that a Codespace can push workflow changes A workflow's permissions block can raise the read-only GITHUB_TOKEN default, so a workflow pushed from a Codespace runs with write access to its repo. Accepted; CONTRIBUTING advises running agents regularly in a local container and only watched one-offs in a Codespace. Co-Authored-By: Claude Opus 5.5 --- CONTRIBUTING.md | 9 +++++++- ...6-10-13-devcontainer-permissions-design.md | 22 +++++++++++++++---- 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 307814d..a9ff144 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -96,7 +96,8 @@ anywhere you can. From the container you can push branches, open and update pull requests, and push tracker changes with `/workspaces/dev/bin/push.bash`. You cannot push to any repo's `main` except `change-requests`', to `gh-pages`, or tags; push a -change to `.github/workflows/`; run or re-run workflows; or change settings, +change to `.github/workflows/` (except from a Codespace; see below); run or +re-run workflows; or change settings, rulesets, environments, or secrets. Do those in GitHub's web UI, or on the host with your own credentials. @@ -117,6 +118,12 @@ you create one, GitHub asks you to authorize access to `ourPLCC/dev` and `bin/push.bash` fails. A full rebuild of a Codespace deletes its volumes and any tracker commits you have not pushed, so push right away. +If you use an agent regularly, use it in a local container, not a Codespace. +A Codespace's token can push changes to `.github/workflows/`, which your own +token cannot, so an agent misled by text it reads could push a workflow that +runs with write access to the repo. An agent for a one-off task in a Codespace +is fine; watch what it pushes. + A Codespace's token expires about 12 hours after the Codespace starts. If a push or `gh` command then fails to authenticate, restart the Codespace. diff --git a/dev-docs/specs/2026-10-13-devcontainer-permissions-design.md b/dev-docs/specs/2026-10-13-devcontainer-permissions-design.md index 95e065d..0c002dc 100644 --- a/dev-docs/specs/2026-10-13-devcontainer-permissions-design.md +++ b/dev-docs/specs/2026-10-13-devcontainer-permissions-design.md @@ -89,7 +89,7 @@ permission rules, the spec says so. ## What a container may do The same in a local container and in a Codespace, except where "Codespaces" -below narrows it. +below narrows or widens it. | Operation | Human | Agent | Enforced by | | --- | --- | --- | --- | @@ -103,7 +103,7 @@ below narrows it. | Merge a PR | yes | **no** | ORG-AGENTS.md and Claude Code deny rules only | | Publish a package | no | no | `pypi` environment's required reviewer | | Run code in CI by pushing a branch or opening a PR | yes | yes | — (CI tokens are read-only; see "Workflows") | -| Push a change to `.github/workflows/` | no | no | token lacks Workflows | +| Push a change to `.github/workflows/` | no (yes in a Codespace) | no (yes in a Codespace) | token lacks Workflows; see "Codespaces" | | Dispatch or re-run workflows; change settings, rulesets, environments, or secrets | no | no | token lacks Actions, Administration, Environments, Secrets | | Anything in a repo outside ourPLCC | no | no | token's resource owner | | Log `gh` in with another credential | no | no | ORG-AGENTS.md and Claude Code deny rules only | @@ -336,7 +336,20 @@ same token. timeout, and a stop ends every process, so this takes a Codespace kept active that long. - So in a Codespace the scope is that repo plus `dev` and `change-requests`, - narrower than local. The table above still holds within that scope. + narrower than local. The table above holds within that scope, with one + exception: the Codespace's token can push a change to `.github/workflows/` + in its own repo, and no setting narrows it. The org's read-only + `GITHUB_TOKEN` is only a default, which a workflow's own `permissions:` + block can raise, so a workflow pushed on a branch can run with write access + to that repo. That reaches its unprotected branches, issues, and PRs, which + the token reaches anyway; its packages, which matters where people pull + them, as with plcc-ng-devcontainer's images; and `actions: write`, which can + start workflows on `main`, running reviewed code with the `release` + environment's secrets. The rulesets, the `release` environment's refusal of + other branches, and the `pypi` reviewer still hold. This is accepted, and + limited by advice rather than enforcement: `CONTRIBUTING.md` says to run + agents regularly in a local container, and to use one in a Codespace only + for a one-off task, watched. ## Where the shared clones live @@ -559,7 +572,8 @@ Must fail: 4. A direct push to the repo's `main`, to `dev`'s `main`, and to `gh-pages` if the repo has one. 5. A force-push to `change-requests`' `main`. -6. A push of a commit touching `.github/workflows/`. +6. A push of a commit touching `.github/workflows/`, locally. In a Codespace + it succeeds; see "Codespaces". 7. Pushing any tag; `gh release create` for a new tag. 8. `gh workflow run` on any workflow. From a1172aea78a8aab463478d3ff75f33f1212b24b6 Mon Sep 17 00:00:00 2001 From: Stoney Jackson Date: Tue, 27 Oct 2026 12:46:25 +0000 Subject: [PATCH 12/13] docs(specs): stop naming the maintainer who bypasses a ruleset Naming them tells a phisher whose token is worth taking; the spec needs only the exception, not the person. Co-Authored-By: Claude Opus 5.5 --- .../2026-10-13-devcontainer-permissions-design.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/dev-docs/specs/2026-10-13-devcontainer-permissions-design.md b/dev-docs/specs/2026-10-13-devcontainer-permissions-design.md index 0c002dc..94f1d67 100644 --- a/dev-docs/specs/2026-10-13-devcontainer-permissions-design.md +++ b/dev-docs/specs/2026-10-13-devcontainer-permissions-design.md @@ -190,11 +190,11 @@ Per repo: org rulesets need GitHub Team, which ourPLCC does not have. "Apply the rulesets" is therefore a step in creating any ourPLCC repo. The release Apps are the only bypass actors; nobody else bypasses, org owners included, with one exception: course-materials-ng's `main` also lets its -maintainer Alain Kägi (`alainkaegi`) bypass, because he maintains it by -pushing to `main` directly. Its `main` still needs the ruleset for everyone -else: `deploy.yml` runs on every push to `main` with the release App's token, -which reaches every repo in the App's installation. The remaining risk is -that one contributor's token. +maintainer bypass, because they maintain it by pushing to `main` directly. +Its `main` still needs the ruleset for everyone else: `deploy.yml` runs on +every push to `main` with the release App's token, which reaches every repo +in the App's installation. The remaining risk is that one contributor's +token. - **`main`**, every repo except `change-requests` (`dev` included): require a pull request, block force-pushes, block deletion. From fad78d0311b964d66314bf49b6bc70348aa935da Mon Sep 17 00:00:00 2001 From: Stoney Jackson Date: Wed, 28 Oct 2026 19:29:01 +0000 Subject: [PATCH 13/13] docs(specs): keep OAuth app access restricted, approving only GCM The spec assumed OAuth app access was unrestricted so that hosts could push through Git Credential Manager. It was already restricted, which cut GCM off and also stops gh's browser-login token reaching ourPLCC. Keep the restriction, approve GCM alone, and add the host's switch to HTTPS through GCM to the migration. Co-Authored-By: Claude Opus 5.5 --- CONTRIBUTING.md | 8 +++++++ ...6-10-13-devcontainer-permissions-design.md | 22 ++++++++++++++----- 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index a9ff144..29ee8c7 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -148,3 +148,11 @@ beside the code repos, and tracker changes were pushed from there. Move once: or clone it fresh. 5. Delete the host's `../dev`. Do not check out a branch of any repo from before the move, or its devcontainer recreates `../dev`. +6. On the host, push over HTTPS with Git Credential Manager. It comes with + Git for Windows; on macOS, `brew install --cask git-credential-manager`. + ourPLCC admits only approved apps, and GCM is the one approved for + pushing, so a token another app stored fails with a 403. Erase it with + `printf 'protocol=https\nhost=github.com\n\n' | git credential reject`, + then push again and sign in through the browser. If your host clones use + SSH remotes, switch them to HTTPS and remove your GitHub key from the SSH + agent (`ssh-add -d `). diff --git a/dev-docs/specs/2026-10-13-devcontainer-permissions-design.md b/dev-docs/specs/2026-10-13-devcontainer-permissions-design.md index 94f1d67..3fdc16a 100644 --- a/dev-docs/specs/2026-10-13-devcontainer-permissions-design.md +++ b/dev-docs/specs/2026-10-13-devcontainer-permissions-design.md @@ -157,11 +157,17 @@ Set by an org owner, before any container gets push access. an org owner approves each one. Maximum lifetime: 90 days. - Classic personal access tokens are blocked for the org, so a classic token pasted into `gh` cannot reach ourPLCC. -- OAuth app access is left as it is. Restricting it would also cut off the - host's own Git Credential Manager, which the host needs for the work the - container cannot do. The cost: a `gh auth login` taken through the browser - default yields an unscoped token. The `--with-token`-only instruction, the - deny rules, and Verification step 9 stand in for that restriction. +- Third-party (OAuth) application access is **restricted**, as it already + was on 2026-10-27: only approved apps reach ourPLCC's data beyond what is + public. Git Credential Manager is approved, so a host pushes over HTTPS + with GCM's browser sign-in for the work the container cannot do; an org + owner grants it the first time they sign in with it. No other app is + approved for pushing, the GitHub CLI's included, so a `gh auth login` + taken through the browser default yields a token that cannot push to + ourPLCC. The `--with-token`-only instruction, the deny rules, and + Verification step 9 remain as a backstop. A token another app left in a + host's keychain (VS Code's, an old GCM's) fails the same way, and the + contributor erases it and signs in with GCM again. ### Workflows @@ -548,6 +554,10 @@ describes the volumes in place of the `../dev` clone, and drops plcc-ng's (`git rm -r --cached -q . && git reset --hard`) or clone it fresh. 5. Delete the host's `../dev`. Do not check out a pre-migration branch of any repo afterwards, or its devcontainer recreates it. +6. On the host, push over HTTPS through Git Credential Manager, erasing any + token another app stored (see "Tokens"). Switch SSH remotes to HTTPS and + remove any GitHub key from the host's SSH agent (see "Every + devcontainer"). ## Verification @@ -628,7 +638,7 @@ Codespace; the sections above already reflect each result. | Token in a host environment variable via `${localEnv:…}` | VS Code launched from a GUI often does not see the shell's environment, and the setup differs on every host OS | | `gh auth setup-git` | Writes `~/.gitconfig`, which VS Code replaces with the host's copy | | Token with the Workflows permission | A workflow on a pushed branch runs before review; workflow edits are rare enough to do outside the container | -| Restrict OAuth app access for the org | Also blocks the host's Git Credential Manager, which the host needs for what the container cannot do | +| Lift the org's restriction on OAuth app access | Any app a contributor authorizes, the GitHub CLI's browser login included, would reach ourPLCC with an unscoped token; approving Git Credential Manager alone gives hosts what they need | | Keep tracker data in `dev`, restricting direct pushes to `backlog/` with a push ruleset | Push rulesets are not available to ourPLCC | | Keep tracker data in `dev`, open to direct pushes, guarded by `bin/push.bash` | A rule an injected agent can skip, on the path that sets every agent's instructions and every container's hooks | | Tracker data on an orphan branch of `dev`, checked out as a worktree | Same protection as a separate repo, but orphan branches and worktrees are unfamiliar to new contributors, and GitHub shows `main` by default |