fix(handlers): extract nested code from handleLocalConnect — resolves build error (#1600) - #1608
Closed
molecule-ai[bot] wants to merge 1 commit into
Closed
molecule-ai[bot] wants to merge 1 commit into
molecule-ai[bot] wants to merge 1 commit into
Conversation
… build error handleLocalConnect at 66ea0b6 accidentally contains: 1. A package-level 'var canCommunicateCheck' declaration nested inside the function body — invalid Go syntax 2. A duplicate '(h *TerminalHandler) HandleConnect' method definition nested inside handleLocalConnect — Go does not allow nested methods Fix: move canCommunicateCheck to package scope (as in the original PR intent), integrate the KI-005 hierarchy check into the routing HandleConnect, and keep handleLocalConnect as a clean single-purpose function. Fixes: #1600 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
Author
|
[Molecule-Platform-Evolvement-Manager] Duplicate terminal fix — #1632 is the latest attempt. |
HongmingWang-Rabbit
pushed a commit
that referenced
this pull request
Jun 12, 2026
#335, supersedes #1608) Empirical finding (a6e3ff018, 2026-05-20): molecule-core's runtime_image_pins table (mig 047) has never had a writer in any repo. The reader at handlers/runtime_image_pin.go has been hitting sql.ErrNoRows on every workspace provision since mig 047 landed, silently falling through to the :latest path. CP's parallel table (CP mig 027) is the de-facto and only SSOT — it has the writer (POST /cp/admin/runtime-image/promote), the reader, the hard-gate (RFC internal#541 Step 2), seeded post-suspension digests (CP mig 028), and the admin endpoints. This PR ratifies that reality. Note: this is a fresh rebase against current main (tip f17375a). PR #1608 was cut from a base before #1585 (RFC#596 Phase 2 dual-push) landed, so merging it would silently revert the publish-runtime.yml Gitea-PyPI-primary path. Sub-agent a5521785 flagged this on PR #1608 comment 41389. The substantive Go logic is identical to PR #1608; the only difference is the base. What - Add 20260520120000_drop_runtime_image_pins.up.sql / .down.sql to drop the unused table. Care zone PRESERVED: workspaces.runtime_image_digest column + its partial index untouched (earmarked for a future stale-workspace panel per RFC internal#617 §3). - Delete handlers/runtime_image_pin.go (the dead reader) + handlers/runtime_image_pin_test.go. - handlers/workspace_provision.go: replace resolveRuntimeImage(ctx, payload.Runtime) with Image: "" (the dead reader was already returning "" on every call). Rewire the surviving db.DB.QueryRow on this call site to QueryRowContext so the provision-timeout ctx stays load-bearing. - Doc comments in provisioner/provisioner.go + provisioner/registry.go updated to point at CP as the SSOT instead of the dead local table. - Add db/migration_20260520_drop_runtime_image_pins_test.go — static- file pin that up.sql DROPs runtime_image_pins, does NOT touch the care-zone column / index, and that the dead reader files cannot be re-added without failing the test. - Hygiene: prune the now-stranded mock.ExpectQuery("SELECT digest FROM runtime_image_pins") rows in handlers/handlers_test.go and handlers/workspace_provision_test.go (the dead reader is gone, so the mock expectation can never fire). Provisioner test comment updated to reflect CP-as-SSOT. Why Two parallel-named tables with structurally incompatible schemas, only one ever written — that is exactly the kind of internal drift feedback_no_single_source_of_truth was written about for non-vendor surfaces. The deletion is reversible (down.sql recreates the table) and the only behavior change is "ctx is now propagated into the workspace_dir DB lookup", which is a small correctness nudge. Verification - [x] go vet ./internal/handlers/... ./internal/db/... ./internal/provisioner/... — clean - [x] go build ./... — clean - [x] go test ./internal/handlers/ ./internal/db/ ./internal/provisioner/ — all pass (16.5s + 0.2s + 0.3s) - [x] New regression tests assert the care-zone column is not touched + the dead reader cannot return - [x] Empirical grep cross-check: no writer for runtime_image_pins in molecule-core; no reader for workspaces.runtime_image_digest anywhere (both confirmed in RFC internal#617 §1 + §3) - [x] Verified clean rebase: branch parent is current main tip (f17375a), NOT pre-#1585 stale base. Diff vs main contains ONLY the migration-drop work — no .gitea/workflows/publish-runtime.yml regression. Tier tier:medium + area:schema — schema/migration change. Reversible by re-running the down-migration. Two-eye review reviewers: core-be (read path / Go) + core-qa (migration correctness). Cascade plan to ~6 live tenant DBs per RFC internal#617 §7 + feedback_image_promote_is_not_user_live (verify on at least 2 tenants post-deploy). Memory consulted: feedback_no_single_source_of_truth, feedback_image_promote_is_not_user_live, feedback_verify_actual_endstate_not_ack_follow_sop, reference_package_distribution_open_ecosystem_dual_push. RFC: https://git.moleculesai.app/molecule-ai/internal/issues/617 Supersedes: https://git.moleculesai.app/molecule-ai/molecule-core/pulls/1608 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
HongmingWang-Rabbit
pushed a commit
that referenced
this pull request
Jun 12, 2026
… na-gate fallback PRs with thousands of bot-relay comments (e.g. mc#1242-class history) pushed `get_issue_comments` past the runner's cgroup memory cap and 137'd the gate job. Sub-agent aa8fa266 verified via SHA64 1:1 mapping that the leak source is `.gitea/scripts/sop-checklist.py:463-484` — a `while True` pagination that accumulates the FULL Gitea-API comment dict (~2 KiB median, ~3 KiB p95) into one list, then iterates it twice (compute_ack_state + compute_na_state). Fix shape (task #369): 1. `get_issue_comments` now projects to a minimal `{user.login, body}` shape during pagination — drops html_url, pull_request_url, assets, created_at/updated_at, user.avatar_url, etc. Synthetic benchmark on a 5000-comment fixture: 8.2 MiB → 5.5 MiB peak heap (~33% smaller). 2. New `iter_issue_comments` generator yields one minimal-dict per comment, allowing future streaming consumers. 3. Per-comment body cap (8 KiB, env-tunable) — the directive parser only walks for ^/sop-* markers in the first few KiB; a single pasted-CI-log comment can no longer OOM the runner. 4. Script-entry `RLIMIT_AS = 2 GiB` (skipped under SOP_CHECKLIST_NO_RLIMIT=1 for the test suite) so any future regression surfaces as a catchable MemoryError instead of SIGKILL. 5. `--max-comments` cap (default 5000, env-tunable) — at the cap we post a SOFT pending status with a `[volume-skipped]` note so neither BP nor the author gets stuck. No-block. 6. Fold in `probe()` n/a-gate fallback (was issue-355-class KeyError 'security-review'): when called from `compute_na_state` with a gate name that isn't also an item slug, fall back to the gate's own `required_teams` from config instead of KeyError'ing on `items_by_slug[slug]`. Tests: 88/88 pass (87 existing + 8 new for streaming + body-cap + na-gate fallback). Live dry-run against open PR #1608 succeeds end- to-end with state=failure (expected, no acks yet) — minimal-dict shape flows cleanly through compute_ack_state + render_status. Closes #369 follow-up to #364 OOM source. RCA: sub-agent aa8fa266 (SHA64 1:1 mapping, no overlap-correlation). Supersedes mis-targeted attempts #365 (qa-review/security-review bash) and #366 (workflow yaml). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
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.
Summary
Fixes CRITICAL build error in terminal.go (issue #1600).
At commit 66ea0b6 (PR #1574), the handleLocalConnect function body was accidentally nested with:
var canCommunicateCheckdeclaration — invalid Go inside a function(h *TerminalHandler) HandleConnectmethod definition — Go does not allow nested methodsThis prevented
go build ./cmd/serverfrom succeeding.Fix:
canCommunicateCheckmoved to package scope (correct placement)HandleConnectwith KI-005 hierarchy check integratedhandleLocalConnectis now a clean, single-purpose functionFixes: #1600
Co-Authored-By: Claude Sonnet 4.6 noreply@anthropic.com