Skip to content

fix(db,reconcile,docker): plugin-name guard, plugin-dir drift, docker stream funnel, StartMarks (D-0602, D-0644, D-0711, D-0589) - #958

Open
acamarata wants to merge 4 commits into
mainfrom
dw2-cli-4
Open

acamarata wants to merge 4 commits into
mainfrom
dw2-cli-4

Conversation

@acamarata

Copy link
Copy Markdown
Collaborator

Four tech-debt rows, one commit each. D-0164 is not touched (see below).

D-0602: verify-checksums --plugin name grammar

  • runDBVerifyChecksums now calls validatePluginFlag, like db migrate, before it loads the project.
  • The name grammar (slug, length, reserved bin/common/status, no dots) moved to the new lower-layer package internal/pluginname. plugin.ValidateName delegates to it. database.validatePluginArg now uses it too, so the database layer refuses notify.prev, bin and common itself. This covers MigrateUp, PendingMigrations and VerifyChecksums.
  • internal/pluginname is an L1 package by default, so layers.yaml needs no change.
  • Tests: TestDBVerifyChecksumsRefusesNonPluginNames (cmd), the extended TestPluginArgRefusedBeforeAnyScan (database), and internal/pluginname tests.

D-0644: @plugins/ keys outside a build

  • GeneratedState.HandEditedFn resolves the plugin dir from the project env cascade (cascadeValue(root, nil, NSELF_PLUGIN_DIR), else the default). The standalone doctor drift check therefore sees a fragment hand-edited under a dir set only in .env.
  • pruneLegacyPluginKeys now also drops @abs/ keys that lie under the build's plugin dir. Since D-0318 the build no longer writes shared plugin fragments, so these old keys can only go stale. This differs from the row's "re-key to @plugins/": re-keying would keep a hash for a file the build never writes again. PlannedRecord stays pure on the record.
  • Tests: TestDoctorDriftPluginDirOnlyInProjectEnv, TestHandEditedFnPluginDirOnlyInProjectEnv (shell unset and shell different), TestPlannedRecordDropsLegacyAbsPluginKeys. All fail without the fix.

D-0711: logs -f outside the docker funnel

  • New docker.Stream and docker.StreamLines in internal/docker/stream.go. They always forward stdout and stderr, so deploy --follow | tee keeps its logs. The child runs in its own process group. SIGINT, SIGTERM or a context stop sends SIGTERM to the group (kill after 5s) and is not an error, so Ctrl-C leaves no orphan.
  • Moved onto them: deploy_follow.go (the row's deploy_run.go site had already moved there), functions_logs.go, logs_stream.go, and the two logs sites in plugin_logs.go (grep stays a separate grep -E process).
  • Allowlist lines removed or lowered, dockerBasisTotal 107 to 102.
  • logs_stream.go no longer attaches stdin to a follow stream. A group outside the foreground would otherwise get SIGTTIN, and compose logs does not read stdin.
  • logs.go has no logs -f site; its compose ps -q stays allowlisted.
  • Tests (fake docker): non-TTY forwarding, non-zero exit, context stop leaves no orphan, SIGINT stop, line callback error stops docker, over-long line is an error.

D-0589: StartMarks vs a removed container

  • StartMarks skips errContainerGone, like FreshStates. Table test TestStartMarksDropsVanishedContainer (no such object, no such container, other failure) fails before the fix.

D-0164: not changed

Blocked, see the report: the lineage part needs P7-LIVE-15, and the row does not name the other dead items.

Commands run

  • go build ./..., go vet and gofmt -l on the touched packages: clean.
  • GOOS=windows go vet ./internal/docker: clean.
  • go test ./cmd/commands ./internal/docker ./internal/repoqa ./internal/reconcile ./internal/database ./internal/plugin ./internal/pluginname -count=1: 5834 passed.
  • Tests that need Docker were not run (none on this worker).

@acamarata

Copy link
Copy Markdown
Collaborator Author

Rebased onto current origin/main (944046c). After the rebase go build, go vet and go test ./cmd/... plus the changed packages (docker, reconcile, plugin, pluginname, database, repoqa incl. the docker funnel allowlist, 300-line budget and new panic ratchet) all pass locally. No ratchet loosened; the PR's own funnel allowlist edit stands. The earlier go:test failure was main moving under the ratchet tests.

@acamarata
acamarata force-pushed the dw2-cli-4 branch 2 times, most recently from 944046c to 4e658fa Compare October 26, 2026 10:50
@acamarata

Copy link
Copy Markdown
Collaborator Author

Rebased onto origin/main (39b7e8a) and force-pushed with lease (head 4e658fa). The rebase had no conflicts. The full suite passes there with CGO_ENABLED=0 go test -mod=vendor -count=1 ./..., including the docker funnel allowlist, nginx call-site list, 300-line budget and panic baseline ratchets. I changed no ratchet and no source file. The earlier go:test failure came from main moving under the old base; no code change was needed. Please re-run nself-ci on this head.

@acamarata

Copy link
Copy Markdown
Collaborator Author

Rebased onto current origin/main (head eb00254). The gate failure was a stale-base ratchet count: after the rebase go test -mod=vendor -count=1 ./... is green, along with go vet and the gofmt check. No ratchet, allowlist or baseline was loosened beyond this PR's own docker-funnel allowlist entries. Not reproduced against the old head. I could not rerun it in a scratch worktree here, so the stale-base cause is inferred. Separately, NSELF_V15=1 mode has failures in cmd/commands (e.g. TestLegacyPathArchRefusal, TestEnvTargetTrustHostKey) that also fail on origin/main, so they are not from this PR.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant