Repository navigation
[refactor] Effective Go 관례에 맞춰 이름·에러·context·문서 주석을 정리한다 - #576
Merged
Merged
Conversation
…er names Lore: - Intent: Make Go names read the way Effective Go asks: the package clause equals the directory name and exported names do not repeat the package name. - Why: A measurement with revive and staticcheck ST found three packages whose clause differs from the directory and 208 exported names that repeat the package name; importers mixed three aliases for one package. Part of #572 (Effective Go conventions). - Changes: - Set the package clause of internal/domain/{issueopsremote,guardpattern,traceclassification} to the directory name and drop the importer aliases. - Strip the package-name prefix from 208 exported names (contract/issueops 105, contract/selfaugment 26, and 26 other packages) with a type-aware rename across the darwin, linux and windows builds; no alias is left behind. - Resolve the collisions first: MutationAck()/BranchPrepareAck() become NewMutationAck()/NewBranchPrepareAck(), policy.PolicyTier* merge into Tier*, IssueOpsRemoteArtifact becomes ArtifactDraft. - Update the string dependents: signals.go search terms, DDD role keys, one judge prompt word, a flag help text, current docs. - Add TestCollectSignalsHoldOnThisRepository, which pins that every repository signal is true, and the issue-folder checks (renames.txt, check_renames.py, check_import_aliases.py). - Verify: GOOS=darwin,linux,windows go build ./... and go vet ./...; go test ./... -count=1; python3 -B .issueops/issues/572/check_renames.py (renames-clean); check_import_aliases.py (aliases-clean). - Risk: Low for behavior (names only; JSON tags and explicit fields are untouched); Go source names under internal/ change.
…nterface checks Lore: - Intent: Fix the convention violations that do not change behavior. - Why: Revive reported 22 confusing-naming pairs, builtin shadowing, redundant returns and an unchecked set of interface implementations; part of #572. - Changes: - Rename or merge every pair of identifiers that differ only by capitalization (installcli Apply/Rollback/Finalize and RunInstall merge; the others get a role name). - Stop shadowing copy, any and max; drop if-return, else-after-return and duplicate imports; use sync.WaitGroup.Go. - Compare errors with sentinels and errors.Is (hostprobe, judgement); add var _ checks for the installers, runners and mcpservice.Service. - Assign to a local instead of the value receiver in the readiness service; keep four proper-noun error strings with a reasoned //nolint. - Verify: GOOS=darwin,linux,windows go build ./... and go vet ./...; go test ./... -count=1; ./scripts/lint-local.sh (0 issues, darwin and linux). - Risk: Low; error texts and results are unchanged.
…s and stdio MCP shutdown Lore: - Intent: Let a caller's cancellation reach the commands it started, and end the stdio MCP process on SIGINT/SIGTERM. - Why: IssueProvider had a context and a context-free API with fallbacks, gh/glab/lsof and the doctor probe ignored cancellation, project lint-diagnose called os.Exit, provider start failures lost their cause, and stdio MCP died with a signal status instead of answering in-flight requests; part of #572. - Changes: - port.IssueProvider CreateIssue/CreatePullRequest/CreateChild/CloseChild and ReconcilePullRequest take ctx first; the optional context interfaces and fallbacks are gone; both providers carry var _ checks for every optional capability. - providerutil gains RunContext and an unwrappable start error; provider errors are matched with errors.Is(exec.ErrNotFound). - LockedChildCreator passes the lease context to CreateChild; cleanup, the issue graph poster, artifact observation, the doctor probe and lsof take the caller's ctx. - project lint-diagnose returns jsonout.Reported(ExitCodeError) and the root maps it to the exit status. - stdio issueops mcp drains in-flight requests for 10 s after SIGINT/SIGTERM, cancels them, and exits 0; a second signal ends it at once; EOF waiting follows the same context. - Tests that failed before the change: lint-diagnose exit code, start-error unwrap, RunContext cancellation, VerifyChildHierarchy cancellation, stdio shutdown (two), plus a real-process SIGTERM smoke. - Verify: GOOS=darwin,linux,windows go build ./... and go vet ./...; go test ./... -count=1; ./scripts/lint-local.sh; python3 -B .issueops/issues/572/stdio_signal_smoke.py (stdio-signal-ok). - Risk: Medium; stdio mcp now waits up to 10 s after a signal and exits 0 instead of dying with the signal status. Provider error texts and the lint-diagnose bytes are unchanged.
…names Lore: - Intent: Give every exported identifier and package a doc comment in the form Effective Go and revive expect. - Why: 3,053 exported identifiers and 271 packages had no comment and 286 comments did not start with the name; part of #572. - Changes: - Unexport 401 names that no other package uses (interface method names, embedded types, names that signals.go or the DDD lists point at, and types returned by exported calls stay). - Add doc comments and package comments in the language of each package; rewrite wrong-form comments to start with the name. - A token-level comparison (comments and whitespace ignored) shows the comment pass changed no code. - Verify: GOOS=darwin,linux,windows go build ./... and go vet ./...; go test ./... -count=1; ./scripts/lint-local.sh; golangci-lint -c .issueops/issues/572/effective-go-lint.yml reports 0 findings on all three GOOS. - Risk: Low; names under internal/ only and comments.
Lore: - Intent: Close #572 with the review fixes, the architecture decision and the verification record. - Why: Independent diff review (three rounds) found a test-only delegation left in production code and stale pre-unexport spellings in current docs and Go comments; the change also needs its decision and evidence on record. - Changes: - Move serveMCPStreamWithDependencies into a test file; deadcode ./cmd/... reports nothing. - Rewrite 25 doc comments, 32 comments and test messages, and 10 doc lines that still used the exported spelling of unexported names; extend check_renames.py coverage by 304 names and make G9/G16 skip files deleted in the worktree. - Add the ADR for Effective Go naming, context-first provider ports and stdio MCP shutdown, link it in ADR.md, and update runtime.md and go-and-packages.md. - Wrap the G6/G15 CHECKs so the policy runner keeps a one-line result, and record the gate ledger (18/18 met) and the verified-execution report. - Verify: issueops gates check (ledger reset, 18/18 met); ./bin/issueops self-verify --base-ref 7714640 --seed=100 --target-score=95 --llm-eval=false (ok, 27/27 steps, minimum_goal_score 100); deadcode ./cmd/... (no output); effective-go-lint 0 findings on darwin, linux and windows (base 4724/4722/4721). - Risk: Low; comments, docs and one test helper move. Two gates attempts failed on the load-sensitive cmux deadline test, which also fails on the base source under load.
Lore: - Intent: Record how the draft PR was published after the provider outage. - Why: remote create-pr ended ambiguous on a GitHub HTTP 502 and its retry budget was spent; the report must say how the PR was created and adopted. - Changes: - Replace the pending remote-write note in the report with the PR link and the outage recovery path. - Verify: Not-tested: documentation-only edit to the verified-execution report; code and gates were verified before the PR head. - Risk: Low; report text only.
…itted
Lore:
- Intent: Make G9 give the same result before and after the new ADR is committed.
- Why: PR review R-1: git diff --name-only BASE does not list untracked files, so G9 passed before the ADR commit and failed after it (".issueops/adr/2026-11-04-...: dated record changed since base"), and the recorded 18/18 did not reproduce at the final head. The rule only forbids editing existing dated records (cautions/2026-10-06-removal-commits-leave-stale-help-text-orphan-fixtures-and-do.md:13).
- Changes:
- check_renames.py lists dated-record changes with --diff-filter=MDR, so added files pass and modified, deleted or renamed ones still fail.
- Verify: python3 -B .issueops/issues/572/check_renames.py -> renames-clean after the ADR commit; appending a line to the 2026-10-06 ADR makes it exit 1 (reverted).
- Risk: Low; issue-folder check script only.
Lore: - Intent: Make the recorded 18/18 reproducible at the head that gets reviewed. - Why: PR review R-1 showed the earlier ledger ran before the ADR was committed. - Changes: - Ledger reset to unchecked and re-run after the G9 fix commit; all 18 gates met with fresh evidence. - Verify: issueops gates check --env HOME,PATH,LANG,GOPATH,GOMODCACHE,GOCACHE --timeout-seconds 900 (18/18 met, one run, about 8 minutes). - Risk: Low; ledger text only.
Lore: - Intent: Keep the execution report in step with the final head. - Why: PR review R-1 changed the G9 check and re-ran the ledger. - Changes: - Add the R-1 section: cause, fix, evidence and review result. - Verify: Not-tested: report text only; the gates and G9 evidence are in the preceding commits. - Risk: Low; report text only.
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.
요약
Effective Go 관례 점검에서 임시 revive·staticcheck 설정이 4,700여 건을 지적했습니다. 이 PR은 그 지적을 모두 고쳐 같은 설정의 재측정이 darwin·linux·windows 세 빌드에서 0건이 되게 합니다. 패키지 이름을 반복하는 이름, 대소문자만 다른 이름 쌍, 오류 비교, 빠진 문서 주석을 정리했고, 호출자의 취소가 외부 명령까지 닿지 않던 경로와 stdio MCP가 신호를 받으면 요청에 답하지 않고 죽던 문제도 고쳤습니다. CLI 명령, MCP tool 이름, JSON 필드, 저장 형식은 바뀌지 않았습니다.
Closes #572
변경 내용
커밋은 다섯 묶음으로 나눴습니다. 앞의 네 묶음은 단계별로 빌드와 테스트를 통과한 상태입니다.
domain/issueopsremote,domain/guardpattern,domain/traceclassification의 package clause를 디렉터리 이름과 같게 맞췄습니다. 패키지 이름을 접두어로 반복하는 exported 이름 208개에서 접두어를 걷어냈습니다. 호환용 별칭은 남기지 않았습니다.copy·any·max가림, 불필요한 if-return과 else, 오류 문자열 비교를 고쳤습니다. 인터페이스 구현에는var _확인을 추가했습니다.port.IssueProvider의 생성·닫기·조정 메서드가 첫 인자로ctx를 받습니다. context 유무로 나뉘어 있던 이중 API와 대체 경로를 없앴습니다.gh,glab,lsof, doctor 점검이 호출자의 context 취소를 따릅니다. provider 시작 실패는 원인을errors.Is로 확인할 수 있게 감쌌습니다.project lint-diagnose는os.Exit대신 종료 코드 오류를 돌려주고, 최상위가 그 값을 종료 상태로 바꿉니다.issueops mcp는 SIGINT·SIGTERM을 받으면 처리 중인 요청을 최대 10초 기다린 뒤 취소하고 종료 코드 0으로 끝납니다. 두 번째 신호는 즉시 종료합니다.signals.go와 DDD 목록이 가리키는 이름은 제외했습니다. 모든 exported 식별자와 패키지에 문서 주석을 달았습니다. 토큰 단위로 비교해 이 묶음이 주석 외의 코드를 바꾸지 않았음을 확인했습니다.바뀌지 않은 것: CLI 명령 목록, MCP tool 목록, 응답 JSON 필드, 저장 레코드의 schema, 고유 명사로 시작하는 오류 문자열 4건, 의도적인 값 수신자 copy-on-write,
port.InstallPlan크기입니다.확인한 것
golangci-lint를 돌렸을 때 base는 darwin 4724건, linux 4722건, windows 4721건이고 이 브랜치는 세 빌드 모두 0건입니다../bin/issueops self-verify --seed=100 --target-score=95 --llm-eval=false가 27개 단계를 모두 통과했고 최소 목표 점수는 100이었습니다. 이 머신의 기본python3에는pydantic이 없어서 CI용 venv를 PATH 앞에 두고 실행했습니다.go build·go vet,golangci-lint,go test ./... -count=1,go test -race ./..., 이름 변경 잔재 검사가 들어 있습니다.contractgolden과TestResponseContractsGolden이 golden 갱신 없이 통과했습니다.RunContext취소, 두 provider의VerifyChildHierarchy취소,LockedChildCreator가 잠금을 잡은 context, 외부 관측자·doctor 취소, stdio 종료 두 경우, 저장소 신호 테스트입니다.issueops mcp를 띄우고 SIGTERM을 보내 종료 코드 0과stdio-signal-ok를 확인했습니다. base에서는 같은 스크립트가mcp exit status -15로 실패합니다.deadcode ./cmd/...가 출력하지 않습니다.deadcode -test ./...는 base와 같은 한 줄(LinkIssueOpsPlanWithActor)만 남습니다.CommandRunner를 소문자로 잘못 바꾼 것입니다. 모두 고쳤고 세 번째 라운드가 통과를 판정했습니다.확인하지 못한 것: 실제 GitHub·GitLab 원격에 대한 provider 통합 호출은 돌리지 않았고 fake 명령 테스트로 확인했습니다.
internal/adapter/cmux의TestExecRunnerReportsFailureKindsWithInvocationState는 시간에 의존해서 부하가 큰 상태에서 가끔 실패합니다. 검증 항목을 세 번째 시도에서 통과시켰고, 같은 테스트가 base 소스에서도 부하 시 비슷한 비율로 실패하는 것을 확인했습니다. 이 PR은 그 테스트 대상 코드의 이름과 주석만 바꿨습니다.리뷰 포인트
cmd/issueops/mcpcli/mcp_stdio_drain.go와mcp_transport.go의 종료 순서를 봐 주세요. 신호 뒤 10초 대기, 2초 안에 반환하지 않는 reader 처리, 두 번째 신호의 즉시 종료가 의도한 대로인지 판단이 필요합니다.port.IssueProvider의 시그니처 변경(internal/port와 두 provider)이 다른 호스트 쪽 호출자와 맞는지 봐 주세요. 저장소 안의 호출자는 모두 옮겼습니다..issueops/issues/572/renames.txt에서 확인할 수 있습니다.대안과 선택 이유
internal/아래 이름이라 저장소 밖 소비자가 없습니다.위험과 되돌리기
internal/아래 Go 이름과port.IssueProvider시그니처가 바뀌었지만 저장소 밖에서 import하는 코드는 없습니다. CLI·MCP 사용자가 보는 계약은 같습니다.issueops mcp가 신호를 받은 뒤 최대 10초 동안 종료를 미룹니다. 종료 상태는 신호 상태 대신 0이 됩니다. 신호로 종료 상태를 확인하던 호스트 설정이 있으면 영향을 받습니다.server run cancelled로그를 stderr에 남깁니다.gh api자식 프로세스는 create 잠금을 상속하며 이는 기존 동작입니다.git revert로 되돌립니다. 저장 형식과 JSON이 그대로라서 되돌린 뒤 남는 상태는 없습니다.호환성과 마이그레이션
CLI·MCP 사용자와 저장된 레코드는 옮길 일이 없습니다. 저장소 안에서
internal/이름을 가리키던 현행 문서와 검색 문자열(signals.go, DDD 역할 목록)은 이 PR에서 함께 갱신했습니다.남은 일
exported규칙을 상시 lint 게이트로 켤지는 이 PR의 재측정 결과를 보고 따로 결정합니다. 기존.golangci.yml이 스타일 linter를 끈 결정과 충돌하므로 ADR이 필요합니다.