From ddec94609374c12ec5b974d957a5b4ed76afb12e Mon Sep 17 00:00:00 2001 From: diffwireauto <330573775+diffwireauto@users.noreply.github.com> Date: Sun, 4 Oct 2026 00:59:54 -0400 Subject: [PATCH 1/2] docs(#163): how we take upstream changes now the fork link is gone CoreScope left the Kpa-clawbot fork network on 2026-10-03 (#158, #161). There is no Sync fork button and no automatic path for upstream code to arrive. This documents how a wanted upstream change gets taken deliberately instead. Decisions it records, each grounded rather than preferred: - Watch releases, not commits. Upstream runs ~90 commits/month but tags ~3 releases/month (v3.10.1, v3.11.0, v3.12.0 in the last 30 days). Reading the commit log at that volume reproduces the problem #152 created. - Cherry-pick with -x, one change at a time. This also settles attribution: cherry-pick preserves the upstream author's authorship; a squashed reimplementation silently reassigns credit to us. - A 7-item review checklist, every item traceable to something that actually went wrong: the config-contract break (#156), the exact-string required check context, the four-times-duplicated registry (#164), runtime reaches at the pre-handover repo, fork-relationship assumptions, goose migrations (#131), and the paths #144 removed. - An explicit never-take list: .squad/, .github/agents/, .github/instructions/, docs/agents/. Upstream still maintains all of them and last touched them 2026-09-17. Cadence and ownership are left EXPLICITLY unassigned - who reads upstream releases and how often is an owner and team decision, not this document's to make. Until settled, upstream is read on demand, which is stated as such rather than left to be inferred. AGENTS.md gets a pointer at the top, since a policy no tool loads is a policy nobody follows. Minimal insertion only; the AGENTS.md rewrite is #149's scope. Refs #163, #158 Co-Authored-By: Claude Opus 5 --- AGENTS.md | 2 ++ docs/UPSTREAM.md | 84 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 86 insertions(+) create mode 100644 docs/UPSTREAM.md diff --git a/AGENTS.md b/AGENTS.md index badc8208..93bc7888 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -2,6 +2,8 @@ Guide for AI agents working on this codebase. Read this before writing any code. +> **This repository is not a fork.** It left the `Kpa-clawbot/CoreScope` fork network on 2026-10-03. There is no Sync fork button and no automatic path for upstream code to arrive. Before taking anything from upstream — a commit, a release, a fix you saw in their notes — read **[docs/UPSTREAM.md](docs/UPSTREAM.md)**. It carries the mechanism, the review checklist, and the list of paths that must never come back. + ## Architecture Go backend + static frontend. No build step. No framework. No bundler. diff --git a/docs/UPSTREAM.md b/docs/UPSTREAM.md new file mode 100644 index 00000000..72484488 --- /dev/null +++ b/docs/UPSTREAM.md @@ -0,0 +1,84 @@ +# Taking changes from upstream + +`OKI-Mesh/CoreScope` was forked from `Kpa-clawbot/CoreScope` on 2026-06-24 and **left the fork network on 2026-10-03** (#158, #161). There is no Sync fork button, no shared object storage, and no automatic path for upstream code to arrive. + +That is deliberate. This file is how a wanted upstream change gets taken anyway, deliberately and one at a time. + +## Why the old way is not an option + +The fork link offered one button that merged everything upstream had done since the last sync. The last time it was used, #152, that was **583 files, 100+ commits, 20 authors** in a single PR. + +It carried a defect nobody caught: upstream had moved the Carto tile key from config field `carto.token` to `carto.key`. Our config still said `token`, so the new code appended no API key and the map went blank — on dev immediately, and on prod at its next deploy. Found by the owner noticing dead tiles, not by review and not by CI (#156). + +Upstream runs at roughly **90 commits a month**. A batch sync is unreviewable at that volume, and one unreviewed config-contract change is enough to break production. + +## Watch releases, not commits + +Upstream tags releases: `v3.12.0` 2026-09-26, `v3.11.0` 2026-09-16, `v3.10.1` 2026-09-04. Three in a month against ninety commits. + +**Read their release notes, not their commit log.** The notes name the changes worth considering; the commit log is noise at this volume. + +```bash +gh release list -R Kpa-clawbot/CoreScope --limit 5 +gh release view v3.12.0 -R Kpa-clawbot/CoreScope +``` + +Nothing about reading upstream requires a fork relationship — a public repo is readable by anyone, confirmed post-detach (#162). + +## Mechanism: a plain remote and a cherry-pick + +Add upstream as an ordinary read-only remote. Never push to it, never merge a branch of theirs wholesale. + +```bash +git remote add upstream https://github.com/Kpa-clawbot/CoreScope.git +git remote set-url --push upstream DISABLED +git fetch upstream +``` + +Take **one change at a time** by cherry-pick: + +```bash +git worktree add /c/Dev/.worktrees/corescope- -b feat/- origin/master +cd /c/Dev/.worktrees/corescope- +git cherry-pick -x +``` + +`-x` records the source commit in the message, so the provenance lives in the history rather than in someone's memory. + +**Cherry-pick, not reimplementation.** Cherry-pick preserves the original author's commit authorship, which is correct: it is their work. A squashed reimplementation silently reassigns credit to us. If a change genuinely cannot be cherry-picked and has to be rewritten, say so in the commit body and name the upstream commit it derives from. + +## Review checklist for an incoming upstream change + +Every item is here because something went wrong without it. + +1. **Does it change a config field our deployments supply?** This is the #156 class and the most dangerous one. Check both boxes' `config.json` against the fields the incoming code reads — including the frontend, where `map` and similar blocks are pass-through and no Go type references them. #157 tracks making this mechanical instead of remembered; until it lands, check by hand. +2. **Does it change a required status check's name?** `Protect Main` requires the context `✅ Go Build & Test` as an exact string, emoji included, matched against the `name:` of a job in `deploy.yml`. A rename on either side leaves PRs waiting forever on a context that never reports. +3. **Does it hardcode a registry, repo or org path?** The pre-handover registry survived in live CI for ten weeks in `release-fast-path.yml` because the value was written out four times and only two were updated (#164). Single-source anything like it. +4. **Does it reach `Kpa-clawbot` or `meshcore-analyzer` at runtime?** Issue links in comments are harmless. A User-Agent header, a release URL, a `gh run list -R`, or an image path is not. +5. **Does it assume a fork relationship?** Anything keyed on `github.event.pull_request.head.repo`, `.fork`, or a Sync-fork workflow is dead here. +6. **Does it touch the schema?** We run goose migrations (#131). An upstream schema change has to arrive as a numbered migration that applies cleanly to a baselined prod database, not as an in-place DDL edit. +7. **Does it bring back anything #144 removed?** See below. + +Run the pre-PR adversarial review as for any change. Note the reviewer tool is installed locally by the governance preflight and is deliberately not in the committed tree, so it will not be present in a clean checkout — if `scripts/llm-consult.py` is missing, run the preflight rather than writing a replacement. + +## What we do not take, ever + +Epic #144 removed an imported agent surface of ~143 files that upstream still maintains. Those paths must not return through this route: + +- `.squad/` — agent charters, casting, decision logs +- `.github/agents/`, `.github/instructions/` — Copilot and PR-reviewer agent definitions +- `docs/agents/` — imported runbooks and skills, including a release runbook that aimed release operations at the pre-handover repo (removed in #151) + +Upstream last modified all of these on 2026-09-17 and continues to. A cherry-pick that touches any of them is wrong by default. If an upstream change is entangled with those paths, take the part you want and drop the rest — that is the whole point of per-change over per-batch. + +## Cadence and ownership + +**Unassigned.** Who reads upstream releases, and how often, is an owner and team decision; this document does not settle it. Until it is settled, upstream is read **on demand** — when someone wants a specific fix, or when a release note mentions something that matters to us. + +The failure mode to avoid is a standing obligation nobody actually performs, which is how the imported agent surface sat untouched for four months while appearing to be maintained. + +## Recording the decision + +A change worth taking gets an issue first, like any other work, naming the upstream commit or release it comes from and why we want it. + +A change considered and **rejected** is worth one line on that issue too — otherwise the same evaluation gets redone by the next person. From 57a4621d4f27e93b3ef103cf29fbadf9d6796792 Mon Sep 17 00:00:00 2001 From: diffwireauto <330573775+diffwireauto@users.noreply.github.com> Date: Wed, 7 Oct 2026 14:23:50 -0400 Subject: [PATCH 2/2] docs(#163): close the gaps an adversarial review found in UPSTREAM.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The policy was drafted but never PR'd. Gemini 2.5 Pro reviewed it against #163's five required decisions. Changes below are its findings, not polish. Two instructions were wrong or unusable as written: - The worktree command hardcoded a Windows absolute path and fails anywhere else. Now uses $WORKTREES and says to set it. - Checklist item 1, the config-field check that exists because of #156, told the reader to compare against live config in a private repo and dead-ended there. It now gives a runnable alternative (/api/config/client reports the effective config) and says plainly that without box or instance access you cannot complete the item and must hand it off rather than skip it. Three gave false confidence: - The upstream-ref grep is presented as proof. It only finds the name spelled out; a path built at runtime passes it. That is exactly how #164 survived ten weeks. Now says to read how the change builds repo/registry strings. - The never-take list reads as a denylist. It is now explicitly examples subordinate to the question 'is this a file that tells an agent how to behave, that we did not write', with unlisted cases named. - Conflict resolution said resolve by hand but not how to learn why our side looks like that. Adds git log -L / -S and says to ask the owner of the local change rather than guess. Two omissions: - No rollback. The document was entirely preventative and prevention already failed once (#156 reached prod). Adds reverting a pick, finding taken commits via -x, and that redeploying the previous tag beats a revert when prod is affected. - No guidance for a change spanning several commits, which invited exactly the batch sync this policy exists to prevent. Adds the range pick and the test for when something is a batch. Dependency item 4 listed commands but no criteria, so it could not produce a decision. Adds what blocks a pick (reachable vulnerability, copyleft licence), what is the owner's call (new direct dependency), and what merely gets noted. Cadence is still deliberately unsettled. #163 asked for it to be decided and this does not decide it, because who reads upstream and how often is an owner and team decision rather than a session's. Flagged on the PR as the one open question. Also corrects the reviewer-tool path back to scripts/llm-consult.py, which the preflight installs (hooks/preflight.sh §5b). An earlier edit in this session changed it to a stale legacy copy at .llm-consult/. Refs #163, #158, #156, #164 Co-Authored-By: Claude Opus 5 --- docs/UPSTREAM.md | 136 +++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 113 insertions(+), 23 deletions(-) diff --git a/docs/UPSTREAM.md b/docs/UPSTREAM.md index 72484488..896bb755 100644 --- a/docs/UPSTREAM.md +++ b/docs/UPSTREAM.md @@ -12,70 +12,160 @@ It carried a defect nobody caught: upstream had moved the Carto tile key from co Upstream runs at roughly **90 commits a month**. A batch sync is unreviewable at that volume, and one unreviewed config-contract change is enough to break production. -## Watch releases, not commits +## Releases are the default lens — and what that costs -Upstream tags releases: `v3.12.0` 2026-09-26, `v3.11.0` 2026-09-16, `v3.10.1` 2026-09-04. Three in a month against ninety commits. +Upstream tags releases, and reading release notes rather than the commit log is what makes review tractable. Measured on 2026-10-07: **88 commits in the preceding thirty days against four releases** in the same window. Roughly twenty commits per release, so the release is the unit that a person can actually read. -**Read their release notes, not their commit log.** The notes name the changes worth considering; the commit log is noise at this volume. +Do not trust that ratio, or any tag named in this file, to still be current. It was four releases in thirty days when this was written and it will drift. Read it live: ```bash gh release list -R Kpa-clawbot/CoreScope --limit 5 -gh release view v3.12.0 -R Kpa-clawbot/CoreScope +gh release view -R Kpa-clawbot/CoreScope +``` + +**State the tradeoff rather than pretending there isn't one.** Watching releases means a fix that lands on upstream's `master` and is not yet tagged is invisible to us until it is released, or until someone goes looking. For an ordinary improvement that latency is fine. For a security fix or a crash fix it is not. + +So releases are the default, with one exception: **when we have reason to believe upstream has fixed something urgent, search their commits directly rather than waiting for a tag.** + +```bash +# targeted, not a 90-commit read +gh search commits --repo Kpa-clawbot/CoreScope "fix" --limit 20 +gh api repos/Kpa-clawbot/CoreScope/commits --paginate -q \ + '.[] | select(.commit.message | test("(?i)security|CVE|panic|data race|corrupt")) | "\(.sha[0:8]) \(.commit.message | split("\n")[0])"' +gh issue list -R Kpa-clawbot/CoreScope --label security --state all ``` Nothing about reading upstream requires a fork relationship — a public repo is readable by anyone, confirmed post-detach (#162). ## Mechanism: a plain remote and a cherry-pick -Add upstream as an ordinary read-only remote. Never push to it, never merge a branch of theirs wholesale. +Add upstream as an ordinary read-only remote, once: ```bash git remote add upstream https://github.com/Kpa-clawbot/CoreScope.git git remote set-url --push upstream DISABLED -git fetch upstream ``` -Take **one change at a time** by cherry-pick: +**Fetch before every pick.** A cherry-pick needs the commit object present locally; detaching did not change that, but it does mean nothing arrives until you fetch. A stale remote is the usual reason a pick fails with "bad object". ```bash -git worktree add /c/Dev/.worktrees/corescope- -b feat/- origin/master -cd /c/Dev/.worktrees/corescope- +git fetch upstream # every time, not just the first +# $WORKTREES is wherever you keep worktrees. On the owner's Windows box that is +# C:/Dev/.worktrees; set it to suit your machine rather than copying a path. +git worktree add "$WORKTREES/corescope-" -b feat/- origin/master +cd "$WORKTREES/corescope-" git cherry-pick -x ``` -`-x` records the source commit in the message, so the provenance lives in the history rather than in someone's memory. +`-x` records the source commit in the message, so provenance lives in the history rather than in someone's memory. -**Cherry-pick, not reimplementation.** Cherry-pick preserves the original author's commit authorship, which is correct: it is their work. A squashed reimplementation silently reassigns credit to us. If a change genuinely cannot be cherry-picked and has to be rewritten, say so in the commit body and name the upstream commit it derives from. +**Cherry-pick, not reimplementation.** Cherry-pick preserves the original author's authorship, which is correct: it is their work. A squashed reimplementation silently reassigns credit to us. + +### When the pick conflicts + +A conflict is information, not just an obstacle. Our tree has diverged a long way, so a conflict usually means the pick has landed in something we customised. + +- **Resolve by hand.** Do not take `-X ours` or `-X theirs` wholesale; both discard one side without reading it. +- **Treat a conflict in heavily-customised code as a reason to reconsider.** If resolving it means rewriting most of the change, the honest outcome may be to reimplement deliberately — or to decide we don't want it. +- **Say so in the commit.** `-x` alone implies a clean pick. If you resolved conflicts, add a line naming the files and what was preserved, e.g. *"conflict resolved in `cmd/server/routes.go` to keep our RLock snapshot from #141"*. A reader six months out needs to know the pick was not clean. +- **Find out why our side looks like that before you overwrite it.** Resolving by hand still needs you to understand both sides, and the local side usually exists for a reason that is not visible in the diff. `git log -L` on the conflicting range, or the issue named in the blamed commit, will say who changed it and why. If that reason is still load-bearing and you cannot tell, ask whoever owns it rather than guessing — a hand-resolved conflict that quietly drops a fix is worse than a pick we declined. + +```bash +git log -L ,: # history of just the conflicting lines +git log -1 --format='%h %an %s' -S'' -- +``` ## Review checklist for an incoming upstream change Every item is here because something went wrong without it. -1. **Does it change a config field our deployments supply?** This is the #156 class and the most dangerous one. Check both boxes' `config.json` against the fields the incoming code reads — including the frontend, where `map` and similar blocks are pass-through and no Go type references them. #157 tracks making this mechanical instead of remembered; until it lands, check by hand. -2. **Does it change a required status check's name?** `Protect Main` requires the context `✅ Go Build & Test` as an exact string, emoji included, matched against the `name:` of a job in `deploy.yml`. A rename on either side leaves PRs waiting forever on a context that never reports. -3. **Does it hardcode a registry, repo or org path?** The pre-handover registry survived in live CI for ten weeks in `release-fast-path.yml` because the value was written out four times and only two were updated (#164). Single-source anything like it. -4. **Does it reach `Kpa-clawbot` or `meshcore-analyzer` at runtime?** Issue links in comments are harmless. A User-Agent header, a release URL, a `gh run list -R`, or an image path is not. -5. **Does it assume a fork relationship?** Anything keyed on `github.event.pull_request.head.repo`, `.fork`, or a Sync-fork workflow is dead here. -6. **Does it touch the schema?** We run goose migrations (#131). An upstream schema change has to arrive as a numbered migration that applies cleanly to a baselined prod database, not as an in-place DDL edit. -7. **Does it bring back anything #144 removed?** See below. +1. **Does it change a config field our deployments supply?** This is the #156 class and the most dangerous one, because nothing in CI can catch it: every CI artifact derives from the code, so code and fixtures move together and stay self-consistent. The only disagreeing artifact is the config deployed on the boxes, which CI has never seen. Compare the fields the incoming code reads — including frontend code, where blocks like `map` are pass-through and no Go type references them — against the live prod and dev config. + + Those config files live on the boxes, outside this repo; the private `OKI-Mesh/roadmap` infrastructure inventory says which box and which path. You do not need that access to do the comparison, because a running instance reports its own effective config: + + ```bash + # what the server actually resolved, including defaults it filled in + curl -s /api/config/client | jq 'keys' + curl -s /api/config/client | jq '.' + ``` + + If you have neither box access nor a reachable instance, **you cannot complete this item** — say so on the issue and get someone who can. Do not merge an upstream change having skipped it. #157 tracks making this mechanical so it stops depending on who is holding it. +2. **Does it change a required status check's name?** Merge protection matches the check context as an exact string, emoji included, against a job `name:` in `deploy.yml`. A rename on either side leaves PRs waiting forever on a context that never reports. Do not trust the value written in any document, including this one — read the live rule: + ```bash + gh api repos/OKI-Mesh/CoreScope/rulesets -q '.[].id' \ + | xargs -I{} gh api repos/OKI-Mesh/CoreScope/rulesets/{} \ + -q '.rules[]|select(.type=="required_status_checks")|.parameters.required_status_checks[].context' + ``` +3. **Does it hardcode a registry, repo or org path?** The pre-handover registry survived in live CI for ten weeks because the value was written out four times and only two were updated (#164). Prove you checked: + ```bash + grep -rniE "kpa-clawbot|meshcore-analyzer" --exclude-dir=.git . + ``` + Anything found at runtime — a User-Agent header, a release URL, a `gh run list -R`, an image path — is a defect. An issue link in a comment is not. + + **A clean grep is not proof.** It only finds the name written out literally. A path assembled at runtime — `fmt.Sprintf("%s/%s", org, repo)`, an org held in a const or an env default, a workflow `${{ env.UPSTREAM }}` — passes this grep and still points at upstream. That is exactly how #164 survived: the value was written four times and only two were changed. So also read how the change *builds* any repo, registry or URL string, not just whether it spells the old name. +4. **Does it change Go dependencies?** An upstream change can move `go.mod` / `go.sum`, pulling a new module, a breaking API, a new vulnerability, or a licence we would not choose. Diff both files, and run `govulncheck ./...` if the module set changed. -Run the pre-PR adversarial review as for any change. Note the reviewer tool is installed locally by the governance preflight and is deliberately not in the committed tree, so it will not be present in a clean checkout — if `scripts/llm-consult.py` is missing, run the preflight rather than writing a replacement. + What to do with the answer, so this is a decision and not just a reading: a **new direct dependency** is the owner's call, not the reviewer's — say what it is and what it replaces. A **vulnerability `govulncheck` reports as affecting a path we call** blocks the pick until upstream or the module fixes it. A copyleft licence (GPL, AGPL, SSPL) on anything we link blocks it outright. Anything else — a transitive bump, a vuln in a code path we do not reach — goes in the PR description and proceeds. +5. **Does it change frontend assets or vendored libraries?** The frontend is vanilla JS with no bundler, so a vendored `*.min.js`, a new CDN URL, or a changed `