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..896bb755 --- /dev/null +++ b/docs/UPSTREAM.md @@ -0,0 +1,174 @@ +# 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. + +## Releases are the default lens — and what that costs + +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. + +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 -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, once: + +```bash +git remote add upstream https://github.com/Kpa-clawbot/CoreScope.git +git remote set-url --push upstream DISABLED +``` + +**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 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 provenance lives in the history rather than in someone's memory. + +**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, 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. + + 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 `