Repository navigation
refactor(server): remove the dead POST /api/packets endpoint (#223) - #231
Merged
Merged
Conversation
handlePostPacket INSERTed into transmissions, observers and observations on the server's mode=ro handle (Kpa-clawbot#1283), so in production every call returned 500 with the raw SQLite error. Nothing in the frontend used it; ingest is MQTT -> cmd/ingestor. Removed: the route and handler, PacketIngestResponse, the OpenAPI description, the proto request/response messages, the api-spec section, the FAQ claim, the BUILD_PLAN lines, tools/generate-packets.js (its only job was to POST here), the writable-DB round-trip test, and routes.go from knownServerWriteSQL. POST /api/packets now gets 405 from the API router; with main.go's catch-all SPA handler it falls through to index.html like any other unmatched path. Both are pinned against an OpenDB (mode=ro) store, with row counts checked. New guards: no INSERT/REPLACE into the packet tables anywhere in cmd/server, and every OpenAPI description must name a registered route. Relates to #223 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018mVL1VB4jtY7caZQnMWJpW
4 tasks
Collaborator
Review — CS-Macmini PR#231 remove-post-packets — head dbc0426Dom: APPROVE med nits. The removal is correct and complete in behaviour. The branch must be synced with master before merge (F1); F2 and F3 are small and fit in that sync commit. Evidence tags: [T] = tested/run by me, [A] = read/analysed in source, [K] = taken from the author's work log or PR text, not verified by me. There is no author report on this PR. This review is based on issue #223, finding N9 from my #222 review, the PR description and the diff. Items from the author's work log are tagged [K]. Setup: Findings
1. Only
|
| Request | master | merged |
|---|---|---|
POST /api/packets + valid key |
500 {"error":"transmission insert: attempt to write a readonly database (8)"} |
200 text/html (index.html, SPA catch-all) |
POST /api/packets no key |
401 | 200 text/html |
GET /api/packets?limit=5, ?groupByHash=true, /api/packets/timestamps, GET /api/packets/{hash}, POST /api/packets/observations, POST /api/decode |
200 | 200, byte-identical bodies |
GET /api/spec |
— | identical except that ('/api/packets','post') is gone |
| DB file sha256 / row counts (tx/obs/observers) | unchanged, 499/500/31 | unchanged, 499/500/31 |
- What the PR pins, and on which router: [A]
TestPostPacketsRemovedReturns405OnReadOnlyDBuses the plain API router (RegisterRoutesonly) and expects 405.TestPostPacketsRemovedFallsThroughToSPAInProductionRouteradds the samePathPrefix("/")+wsOrStatic(spaHandler)wiring asmain.goand expects 200text/html.- Both use an
OpenDB(mode=ro) store and check row counts. The PR text says plainly that production returns 200 index.html, not 405, and calls that the existing fallback. My live probe confirms it. [T] - The fallback for unmatched
/api/*is tracked as fix(server): unknown /api/* paths and wrong methods return 200 index.html instead of a JSON 404/405 #233 and is out of scope here. [K]
- Route survival:
TestPacketsRoutesSurviveRemovalchecks routing only, not handler output, and says so. My probe above covers the handler output. [A]/[T]
2. Everything that belonged only to the endpoint is gone
-
Removed:
- handler and route;
PacketIngestResponse(types.go);- the OpenAPI description;
- the
packetpathimport in routes.go (it is still used elsewhere); TestPostPacketPersistsV3Schema;PacketIngestRequest/PacketIngestResponsefromproto/packet.proto, and the comment inproto/decoded.proto;tools/generate-packets.js;routes.gofromknownServerWriteSQL.
[A]
-
Leftovers I grepped for:
- Grep for
handlePostPacket,PacketIngest,generate-packets, "manual injection" and POST-to-/api/packetsshapes across the repo, excludingfirmware/andnode_modules. The only hits are the new tests, the new comment and the stale master comment (F2). [T] PayloadJSONis left over (F3).ComputeContentHash(used byhash_migrate.go) andsetupTestServerWithAPIKey(still used by tests) are still live. [T]package.jsondoes not reference the deleted tool. [T]
- Grep for
-
Proto:
- Correct: the removed messages were referenced nowhere else.
DecodedResultis still used byDecodeResponseandWSPacketData. [A] - Validator:
tools/validate-protos.pygives the same result on master and merged: 38 fixtures, 0 errors, 3 warnings. The only change is 145 → 143 parsed messages. [T] - Necessary and harmless: the repo has no codegen and no generated
.pb.go; the.protofiles are documentation. Whole messages are removed, so no field numbers needreserved. An external client built from these messages could never have worked against a production (mode=ro) server, so nothing working can break. [A] - External clients: I found no MeshViewLive or other client reference in this repo, and I cannot check consumers outside it. [A]
- Correct: the removed messages were referenced nowhere else.
3. Guard and the mode=ro probe
- The new guard:
TestServerHasNoPacketTableInsertscovers INSERT,INSERT OR …and REPLACE ontransmissions,observations,observersanddropped_packets. It runs on every non-testcmd/server/*.gowith no exceptions, which is minimal.cmd/serverhas no Go subpackages; onlytestdata/. Its companion sensitivity test covers the three removed statements, plus quoted and multi-line forms. [A]/[T] - Generic INSERT guard: INSERTs into any table are still guarded by
TestServerSourceHasNoNewWriteSQL. On the merged tree it allows onlybackup.go: 1(VACUUM INTO),openapi.go: 1(prose) andping_score_history.go: 15(its own DB).routes.goandhash_migrate.goare both at 0 now. [T] - Remaining write SQL:
- No write-SQL literal is left outside those exceptions. [T]
- The non-test
Exec/Begincalls are:backup.go(VACUUM INTO),ping_score_history.go(own DB),reach_rank.go:349(read-only tx, rollback only) anddb.go:248(PRAGMA wal_checkpointon Close). The last two are pre-existing and out of scope. [A]
mode=roprobe repeated: see the table in item 1. Master gives 500 with SQLite text; merged gives no DB access and an unchanged file hash. [T]
4. Orphan check (TestOpenAPIDescriptionsHaveRoutes)
- Robust. It builds
METHOD path-templatekeys fromrouter.Walk, exactly asbuildOpenAPISpeclooks descriptions up. So it can only flag a description that the spec builder would silently ignore anyway. [A] RegisterRouteshas no conditional registration (97.Methods(...)calls, noif/for), so config cannot hide a route from the test. [A]- Edge case: a description for a route registered outside
RegisterRoutes(inmain.go) would fail. Today that is only/ws, which is outside/api/and has no description. Acceptable. [A] - The check is a test, not code in
openapi.go(F4). Mutant M7 shows it works. [T]
5. Docs
-
Updated:
docs/api-spec.md: the section and its TOC entry are both gone.docs/user-guide/faq.mdQ8: now says MQTT + ingestor only, and that the server is read-only.BUILD_PLAN.md: the injection line, the "manual packet injection" data line and the tool in the tree listing are removed.
[A]
-
Other mentions: a grep of all
*.md,*.jsonand*.ymlfinds no other "POST /api/packets" or injection mention.config.example.jsonandconfiguration.mddescribeapiKeygenerically ("POST/PUT routes"), which is still accurate. [T] -
CHANGELOG: not touched. The reason is in the PR text. [K]
6. Diff contains only intended files
-
Files:
git diff --stat origin/master...dbc04260shows 13 files (+256/−563): one added (the new test), one deleted (tools/generate-packets.js), 11 modified. Every file matches the PR description. [T] -
Hygiene:
- one commit on top of
7697a826; git diff --checkis clean;- no conflict markers, no empty blobs in the tree, no mode changes;
- the touched Go files are
gofmt-clean.
[T]
- one commit on top of
-
No trace of the
git stashepisode. [K] for the episode itself, [T] for the clean diff.
7. Rules
cmd/serverread-only: the PR removes the last INSERTs on the shared DB. [T]- No new
map[string]interface{}: there are none. The diff removes two, one in the handler and one in the deleted test. [T] - Fork guards:
github.repository ==appears 9 times indeploy.ymland once inrelease-fast-path.yml, unchanged. [T] - No closing keywords in the title, body or commit message; they use "Relates to POST /api/packets INSERTs on the server's read-only handle and always returns 500 #223". [T]
- Commit author and committer: both are
dborup <kontakt@meshview.dk>. [T] - Test count: top-level
func Testincmd/servergoes 2114 → 2120 by my count (base vs head) and 2135 → 2141 on master vs merged. The delta of +6 (−1 / +7) matches the PR text; the PR's absolute numbers differ by 1 because of the counting method. [T]
Tests
- CI on head: Go Build & Test, Playwright E2E and Docker all pass; Deploy, Badges and Release were skipped. This ran on the old base (
7697a826), not on current master. [T] (gh pr checks) cd cmd/server && go test -race -count=1 -timeout 30m ./...on the merged tree (aff158c7+ head, F1 resolved as above): 1 failure,TestIssue1008_HandlerReturns503WhileSubpathIndexLoading(status 200, want 503).- This is the known flake test: TestIssue1008_HandlerReturns503WhileSubpathIndexLoading races the background index build under load (-race flake) #227. The run was under load, with mutants and the probe running in parallel.
- Rerun in isolation with
-race -count=5: 5/5 pass. - The
rebuild panic: intentional test panicstack trace in the log is the expected output ofTestNeighborGraphCacheRebuildPanicIncrementsCounter. - reach_rank tests: onDegreeSnapshotLoad written while a previous test's singleflight refresh still reads it (-race flake) #224 and test(server): TestReachRank_ExpiredSnapshotAlwaysRefreshes misses its 3s round deadline under load #237 did not fire.
- Wall time 7m45s, so it did not hit the 10m default. [T]
- All PR-relevant and guard tests pass under
-raceon the merged tree (23 tests: the 7 new ones, both fix(store): move content-hash migration writes to the ingestor and merge duplicates in memory (#215) #222 guards,TestServerSourceHasNoNewWriteSQL,TestServerSourceHasNoCachedRWCalls,TestRequireAPIKey_*,TestOpenAPI*). [T] sh test-all.sh(merged tree): 217/217 files pass. [T]python3 tools/validate-protos.py: master and merged give the same result (see item 2). [T]
Mutants (merged tree, targeted tests)
| # | Mutant | Result |
|---|---|---|
| M1 | Route + handler restored verbatim from master | killed by 405 test (500), SPA test (500, JSON), TestOpenAPISpecHasNoPostPackets, TestServerHasNoPacketTableInserts, TestServerSourceHasNoNewWriteSQL (routes.go 3 > 0) |
| M2 | One INSERT OR IGNORE INTO observers in routes.go |
killed by TestServerHasNoPacketTableInserts and TestServerSourceHasNoNewWriteSQL |
| M3 | INSERT INTO nodes in routes.go (not a packet table) |
killed by TestServerSourceHasNoNewWriteSQL and TestServerSourceHasNoCachedRWCalls |
| M4 | New guard widened (name == "routes.go" skipped) + INSERT INTO transmissions in routes.go |
killed by TestServerSourceHasNoNewWriteSQL; the generic guard backs up the widened one |
| M5 | M4 + knownServerWriteSQL["routes.go"] = 1 (both guards widened) |
survived, as expected: it needs visible edits to two guard files |
| M6 | Swap in exempt ping_score_history.go: a CREATE INDEX literal becomes INSERT INTO nodes (count still 15) |
killed by TestServerSourceHasNoCachedRWCalls |
| M7 | "POST /api/packets" description re-added to routeDescriptions() |
killed by TestOpenAPIDescriptionsHaveRoutes |
| M8 | Like M6, but INSERT INTO observer_neighbors |
survived: pre-existing gap (F5), not caused by this PR |
| M9 | /api/packets GET route widened to Methods("GET","POST") |
killed by 405 test (200 JSON), SPA test, TestOpenAPISpecHasNoPostPackets |
Not verified
- CI on the merged tree: it needs the F1 sync first. My merged-tree results use my own conflict resolution.
- Live UI: no browser check, no staging or prod. The probe ran only against local binaries and a fixture copy with a throwaway local test key.
- External proto consumers: none in this repo, and I could not inspect any outside it.
- The author's own runs: the 30m
-racerun and the proto-validator run, and fix(server): unknown /api/* paths and wrong methods return 200 index.html instead of a JSON 404/405 #233's scope. [K] - The
cmd/ingestortest suite: not run, since this PR does not touch it.
…ove-post-packets Brings in #222 (content-hash migration moved to the ingestor), which also rewrote the knownServerWriteSQL comment. The map merged cleanly to backup.go 1, openapi.go 1, ping_score_history.go 15; only the comment conflicted. Both notes are kept: hash_migrate.go lost its 3 literals in #215 and routes.go lost its 3 with POST /api/packets (#223). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… review F2-F4) F2: TestServerHasNoPacketTableWrites (#215) still described handlePostPacket's INSERTs as a known gap, and #223 had added a second guard, TestServerHasNoPacketTableInserts, with a different table list. They are now one guard: INSERT joins UPDATE/DELETE/REPLACE in the everywhere list, with one table list (transmissions, observations, observers, dropped_packets, ping_triggers, route_mask_changes), no exceptions, and every hit reported with its line. The INSERT cases and the negative cases from the removed test move into TestServerHasNoPacketTableWritesIsSensitive. F3: PayloadJSON in cmd/server/decoder.go lost its only caller with the POST handler. The ingestor's copy stays. F4: TestOpenAPIDescriptionsHaveRoutes moves to openapi_test.go and uses setupTestServer. No behaviour change. Relates to #223 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Collaborator
Rapport — CS-Macmini PR#231 runde 2 — head 558e46fStatus: F1–F4 from the round-1 review are fixed; F5 has no change, and a follow-up issue is proposed below. CI is green (Go, Playwright, Docker). The PR stays draft and is ready for re-review by another reviewer. Merged master (commit Review feedback addressed (commit I took over from the original author (the cloud agent is no longer used) and pushed two fast-forward commits on top of Evidence tags: [T] = tested/run by me, [A] = read/analysed in source, [K] = taken from someone else's statement, not verified by me. F1 — merge conflict with master (fixed,
|
| # | Mutant | Result |
|---|---|---|
| R1 | INSERT OR IGNORE INTO observers re-added in routes.go |
killed by TestServerHasNoPacketTableWrites (routes.go:4822: INSERT OR IGNORE INTO observers) and TestServerSourceHasNoNewWriteSQL |
| R2 | Route + handlePostPacket + PacketIngestResponse + PayloadJSON restored from master |
killed: TestPostPacketsRemovedReturns405OnReadOnlyDB (500 with "attempt to write a readonly database"), TestPostPacketsRemovedFallsThroughToSPAInProductionRouter, TestOpenAPISpecHasNoPostPackets, TestServerHasNoPacketTableWrites (all 3 INSERTs, with lines), TestServerSourceHasNoNewWriteSQL |
| R3 | "POST /api/packets" description re-added to routeDescriptions() |
killed by TestOpenAPIDescriptionsHaveRoutes (in openapi_test.go) |
| R4 | UPDATE observers SET … literal in store.go (newly covered table for UPDATE) |
killed by TestServerHasNoPacketTableWrites and TestServerSourceHasNoNewWriteSQL |
| R5 | INSERT INTO dropped_packets in backup.go (a documented exception file) |
killed by TestServerHasNoPacketTableWrites and TestServerSourceHasNoNewWriteSQL (backup.go 2 > 1) |
| R6 | Swap in exempt ping_score_history.go: one CREATE INDEX literal becomes INSERT INTO transmissions (count stays 15) |
killed by TestServerHasNoPacketTableWrites (ping_score_history.go:624) |
CI (run 37277888063, head 558e46f2)
| Job | Result | Duration |
|---|---|---|
| ✅ Go Build & Test | pass | 22m58s |
| 🎭 Playwright E2E Tests | pass | 20m04s |
| 🏗️ Build & Publish Docker Image | pass | 53s |
| 📦 Release Artifacts | skipped (PR) | — |
| 🚀 Deploy Staging | skipped (PR) | — |
| 📝 Publish Badges & Summary | skipped (PR) | — |
[T] (gh run view, gh pr checks)
Proposed follow-up issue for F5 (not filed)
Title: test(server): pin ping_score_history.go's write SQL to its own tables
Body:
TestServerSourceHasNoNewWriteSQLallowsping_score_history.go15 write-SQL literals, because that file writes its own history database, not the shared one. The allowance is a count, so replacing one of those literals with a write to a shared table keeps the count at 15 and passes.
- The node guard catches
nodesandinactive_nodes.TestServerHasNoPacketTableWritescatches the packet tables (POST /api/packets INSERTs on the server's read-only handle and always returns 500 #223).- Nothing catches e.g.
INSERT INTO observer_neighborsorchannel_proposals.This was found as a surviving mutant in the #231 review.
Proposal: for
ping_score_history.go, require that every write-SQL literal names only that file's own tables (_meta,ping_score_history_entriesand its indexes). Alternatively, check every write literal's target table against an allow-list per exception file. Add a sensitivity test that a swapped literal fails.Acceptance:
- A write to any table outside the allow-list in
ping_score_history.gofails a test.- The existing 15 literals pass.
- The mutant from the refactor(server): remove the dead POST /api/packets endpoint (#223) #231 review (
INSERT INTO observer_neighborsswapped in for a CREATE INDEX) is killed.
Leftovers / not done
- fix(server): unknown /api/* paths and wrong methods return 200 index.html instead of a JSON 404/405 #233:
POST /api/packets, like any unmatched/api/*request, gets 200index.htmlfrom the production router. That is out of scope here, and the current behaviour is pinned byTestPostPacketsRemovedFallsThroughToSPAInProductionRouter. [A] cmd/server/decoder.gohas pre-existing gofmt drift (see Tests). I did not touch it. [T]- Not re-checked this round: the round-1 end-to-end binary probe (master 500 vs merged 200
text/html, DB unchanged). Round 2 does not change any handler or route. [A] - Not done: no browser check, no staging or prod access, and the
cmd/ingestorsuite was not run, since round 2 does not touch it. - Next step: re-review by a different reviewer. I did not review my own fixes.
Owner
Author
Review — CS-MacBook PR#231 runde 2 — head 558e46fDom: APPROVE med nits Evidence tags: [T] run or test output, [A] assessment or inference, [K] checked in code, diff, git or CI. Setup: Round-1 findings
New findings
No blocking finding. 1. F1 — merge and
|
| Request | master | merged |
|---|---|---|
POST /api/packets with key |
500 application/json (SQLite read-only error) |
200 text/html (SPA index) |
POST /api/packets without key |
401 | 200 text/html |
GET /api/packets?limit=5 |
200 | 200, body identical |
GET /api/packets?groupByHash=true&limit=5 |
200 | 200, body identical |
GET /api/packets/timestamps (no params) |
400 | 400, body identical |
GET /api/packets/{hash}, GET /api/packets/{hash}/path |
200 | 200, bodies identical |
POST /api/decode |
200 | 200, body identical |
POST /api/packets/observations |
200 | 200, body identical |
GET /api/spec |
has POST /api/packets |
the same, minus that one operation (every other operation, info, components and tags identical) |
The DB file hash is the same before and after both runs. The 200 text/html for the removed route is the existing SPA fallback, covered by TestPostPacketsRemovedFallsThroughToSPAInProductionRouter and tracked as #233. [T]
The diff removes exactly one route registration and its handler; the 405 behaviour on the plain API router is pinned by TestPostPacketsRemovedReturns405OnReadOnlyDB. [K]
4. Rules
- Fork guards:
deploy.yml9,release-fast-path.yml1 (github.repository == 'Kpa-clawbot/CoreScope'); no workflow file changed. [K] - New
map[string]interface{}: 0 added, 2 removed. [K] - No closing keywords in the PR body or any commit message. [K]
- All three commits: author and committer
dborup <kontakt@meshview.dk>. [K] cmd/serverread-only: the PR removes the last INSERTs on the shared DB. [K]gofmt -l: clean on the touched files exceptdecoder.go, which is already listed on master (struct-tag alignment, outside the changed lines).go vetis clean. [T]
Tests
cd cmd/server && go test -race -count=1 -timeout 30m ./...on the merged tree (master3878d7ea+ head):ok, 395.3 s, exit 0, 0DATA RACElines. None of the known flakes fired. [T]sh test-all.shon the merged tree: 217 passed, 0 failed (217 files). [T]- CI on the head: Go Build & Test, Playwright E2E and Docker pass; Release, Deploy and Badges are skipped (PR). It ran on the earlier base, not on current master. [K]
Mutants (copies of the merged tree; targeted tests)
| # | Mutant | Result |
|---|---|---|
| X1 | INSERT INTO transmissions literal in routes.go |
killed by TestServerHasNoPacketTableWrites (routes.go:4822: INSERT INTO transmissions) and TestServerSourceHasNoNewWriteSQL |
| X2 | UPDATE observations SET … in neighbor_api.go (a random server file) |
killed by both (neighbor_api.go:571) |
| X3 | UPDATE observers SET … in backup.go (a documented exception file) |
killed by both |
| X4 | Split literal "UPDATE " + "observers" + " SET …" in neighbor_api.go |
survived: see N1 |
| X5 | INSERT INTO main.transmissions in neighbor_api.go |
killed by TestServerSourceHasNoNewWriteSQL only; the packet guard misses the schema prefix (N1) |
| X6 | POST /api/packets route re-registered with a stub handler |
killed by the 405 test, the SPA-fallback test and TestOpenAPISpecHasNoPostPackets |
Not verified
- CI on the merged tree. My merged tree has no CI run; I relied on my own local runs.
- No browser check and no staging or prod. The probe ran only against local binaries and a fixture copy.
- External consumers of the removed proto messages, outside this repo.
- The
cmd/ingestorsuite: not run, since the PR does not touch it (the diff ofcmd/ingestoris empty). - The author's own mutants R1–R6: I checked the guard on X1–X3 myself and did not repeat R5 or R6.
- Whether the F5 follow-up issue exists: the report says it was not filed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Relates to #223
Summary
POST /api/packetsran threeINSERTs (transmissions,observers,observations) on the server'smode=rohandle (Kpa-clawbot#1283). In production every call failed with a 500 that carried the raw SQLite error. Nothing in the frontend calls it.This PR removes the endpoint (option b in the issue) and everything that only existed for it. Ingest stays MQTT →
cmd/ingestor.The following are unchanged:
GET /api/packets,POST /api/packets/observations,GET /api/packets/timestamps,GET /api/packets/{id},GET /api/packets/{hash}/path,POST /api/decodeandrequireAPIKey.Removed
cmd/server/routes.go: thePOST /api/packetsroute andhandlePostPacket, plus the now-unusedpacketpathimport.cmd/server/types.go:PacketIngestResponse.cmd/server/decoder.go:PayloadJSON, whose only caller was the handler. The ingestor's copy stays.cmd/server/openapi.go: thePOST /api/packetsdescription.cmd/server/routes_test.go:TestPostPacketPersistsV3Schema. It only passed against a writable test DB.proto/packet.proto:PacketIngestRequest/PacketIngestResponse. Theproto/decoded.protocomment no longer lists the endpoint.tools/generate-packets.js: a dev script whose only job was to POST synthetic packets to this endpoint. Nothing references it.docs/api-spec.md: the section and its TOC link are removed.docs/user-guide/faq.md: Q8 now says that ingest is MQTT + ingestor only and that the server is read-only.BUILD_PLAN.md: the "manual injection" lines and the tool are removed from the tree listing.knownServerWriteSQL: theroutes.go: 3entry is removed, soroutes.gois now held to 0 write-SQL literals. Since fix(store): move content-hash migration writes to the ingestor and merge duplicates in memory (#215) #222 merged,hash_migrate.gois also at 0. The map is nowbackup.go: 1,openapi.go: 1andping_score_history.go: 15.apikey_security_test.goused/api/packetsonly as a placeholder URL around a stub handler. It now uses/api/perf/reset; the assertions are unchanged.CHANGELOG.mdis not touched, because the fork's master has not updated[Unreleased]for any merged PR.What
POST /api/packetsreturns nowRegisterRoutes): 405. The path still matches the GET route, so gorilla/mux rejects the method before any handler or DB access.main.go): 200text/html(index.html).main.gomounts a catch-allPathPrefix("/")SPA handler after the API routes, and gorilla/mux lets a later full match win over a method mismatch. This is the existing fallback for every unmatched/api/*request and is not new in this PR. It is tracked in fix(server): unknown /api/* paths and wrong methods return 200 index.html instead of a JSON 404/405 #233. A test pins the current behaviour, and nothing is written.Guard
There is one guard:
TestServerHasNoPacketTableWritesinreadonly_invariant_test.go, which arrived with #222. This PR extends it.INSERT OR …, UPDATE, DELETE and REPLACE.transmissions,observations,observers,dropped_packets,ping_triggersandroute_mask_changes.cmd/serverfile, with no exceptions. The two documented write exceptions do not touch these tables:ping_score_history.gowrites its own database, andbackup.goonly runsVACUUM INTO.Before this PR, INSERT was only checked in
hash_migrate*.go, and the guard's comment namedhandlePostPacketas a known gap. Every violation is now reported with file, line and statement.TestServerHasNoPacketTableWritesIsSensitivecovers the old migration statements and the three removed INSERTs, and adds negative cases (ping_score_history_entries,observations_archive,observer_neighbors, plainSELECT).Tests
New in
cmd/server/post_packets_removed_223_test.go. Each test uses a seeded file DB opened withOpenDB(mode=ro):TestPostPacketsRemovedReturns405OnReadOnlyDB: a valid key and a decodable body get 405. The body contains no SQLite text, and row counts are unchanged.TestPostPacketsRemovedFallsThroughToSPAInProductionRouter: with themain.gocatch-all, the request gets 200index.htmland nothing is written.TestPacketsRoutesSurviveRemoval: the other/api/packets*routes and/api/decodestill match.TestOpenAPISpecHasNoPostPackets:/api/speckeepsgetand has nopostfor/api/packets.New in
cmd/server/openapi_test.go:TestOpenAPIDescriptionsHaveRoutes: every key inrouteDescriptions()must be a registered method + path. The spec is built by walking the router, so before this test an orphaned description failed nothing.Extended:
TestServerHasNoPacketTableWritesandTestServerHasNoPacketTableWritesIsSensitive, as described under Guard.Perf
No hot path is touched; this PR only removes code and changes tests.
🤖 Generated with Claude Code