Repository navigation
[bug] Linux supervisor HOME 불일치와 pr-review grep 대체 경로 결함을 고친다 - #554
Merged
Merged
Conversation
Lore: - Intent: Stop a Linux HTTP install whose HOME differs from the running user systemd manager before it writes anything. - Why: The installer writes the unit under its own HOME, but the running user manager resolves the unit search path from its own HOME, so `systemctl --user enable` failed after the credential and unit were already written (#550 diagnosis, #552). - Changes: - Add `checkUnitPath` to the supervisor interface; launchd passes, systemd reads `systemctl --user show-environment` (HOME, XDG_CONFIG_HOME) and confirms a mismatch against `systemctl --user show -p UnitPath --value`. - Call it first in `Service.Prepare`, before EnsureBearer, the log file, and the unit. - Stay silent when the manager cannot be queried, has no HOME, or prints `$'...'`-escaped values, so it adds no readiness gate. - Wrap the refusal in `errUnitPathNotSearched`; `Start` reports it as `supervisor_failed`, as before this change. - Isolate the Linux wiring test from the runner's real user manager with a systemctl stand-in on PATH. - Record the two new methods in the DDD responsibility inventory. - Verify: go test -race -count=1 -v ./internal/adapter/mcpservice/... (8 Prepare subtests + Start test, RED first); Linux arm64 container run of TestSupervisorUnitRunsAbsoluteBinaryWithExplicitRootAndState with a runner-like systemctl stand-in (RED then GREEN); go test -race ./... -count=1; golangci-lint v2.12.2 darwin and linux 0 issues. - Risk: A HOME that reaches the manager's directory only through a symlink is refused; the error names the fix (same HOME or --mcp-transport=stdio).
Lore: - Intent: Exercise the new unit search path check against a real user systemd manager on the GitHub runner. - Why: Fake-runner unit tests cannot prove that the runner's manager environment carries HOME; this step does, and it pins the #550 failure mode (#552). - Changes: - Add a CI step that runs install-native.sh --mcp-transport=http in a temporary HOME and requires a failure, the "is not searched by the running systemd user manager" text, and no bearer or unit under that HOME. - Pin the step in scripts/ci_workflow_test.py: refusal passes; success, another error, a written unit, or a written credential fail; the temporary HOME is removed and the ambient HOME untouched. - Verify: python3 -m unittest -v scripts/ci_workflow_test.py; disabling the grep check or the written-file check in the step makes the matching subtests fail. - Risk: If the runner's manager environment lacks HOME, the check stays silent and the step fails on the missing text; that would show the assumption is wrong rather than pass silently.
Lore: - Intent: Make the defs.md source line say which search actually ran. - Why: render_defs printed "rg" whenever codegraph was off, even when rg was missing and grep found the rows, and printed only "codegraph explore" when codegraph came back empty and rg filled in (#552). - Changes: - _rg_symbol returns its rows together with the tool that produced them (rg or grep); the output trimming moves to _hits. - collect_defs records a source per symbol; render_defs lists the tools in first-use order without repeats ("none" when no symbol was searched) and drops its unused codegraph argument. - Tests: grep fallback asserts "source: grep", a codegraph-plus-rg case asserts both tools, and a direct source-line test without rg. - Verify: python3 -m unittest discover -s skills/pr-review/tests (RED: the new assertions saw "source: rg"); no consumer parses the source line. - Risk: Low; the source line wording in defs.md changes.
Lore: - Intent: Keep ignored build output out of definition search when rg is unavailable. - Why: The grep fallback walks every file with a fixed exclude list, so .gitignore'd artifacts could appear in defs.md on runners without ripgrep (#552). - Changes: - When rg is missing and the checkout is a git work tree, search with `git grep --untracked -n -w -F -I --max-count 3`, which skips ignored files. - Express rg's exclusions as `:(exclude,glob)` pathspecs; a magic-less `*` also matches `/` and would drop directories such as blocks/. - Fall back to grep when git is missing, the checkout is not a work tree, or git grep exits above 1 (for example git older than 2.38 without --max-count). - Tests: ignored files absent from a temp git repo, a definition under a lock-named directory found while lock/min/node_modules files stay out, a failing git grep falls back to grep. - Verify: python3 -m unittest discover -s skills/pr-review/tests (RED: the ignored-file test saw build/ hits and "source: rg"); 5-run median in this repo, git grep 0.349s vs grep 0.669s. - Risk: Low; only runs when rg is absent.
Lore: - Intent: Keep the operations and caution docs in line with the new Linux HTTP install behaviour. - Why: install.md still said a temporary-HOME HTTP install fails with supervisor_failed after writing; it now refuses before writing, and the cycle hit pitfalls worth not repeating (#552). - Changes: - install.md: describe the pre-write unit search path check, when it stays silent, and the new CI step. - New caution 2026-10-08: systemd-analyze does not ask the manager, UnitPath omits missing directories, show-environment escapes values, a HEAD-CI gate passes falsely on the base commit, a stderr "skipped" grep misfires, and darwin-only gates miss Linux-only test paths. - CAUTIONS.md: index row for the new caution. - Verify: go test ./cmd/issueops/issueopsapp -run TestResponseContractsGolden -update -count=1 (no golden change); project-docs-optimize check reports only the pre-existing adr/roadmap.md line budget. - Risk: Low; documentation only.
…eport Lore: - Intent: Commit the cycle materials that back the #552 changes. - Why: The plan, intent, plan-review summary, gate ledger, and verified-execution report are the inspectable evidence for this cycle (#552). - Changes: - .issueops/issues/552/: plan, intent, plan-review summary, and the G1-G10 gate ledger (G8/G9 abandoned until the PR CI run). - .issueops/verified-execution/552-supervisor-home-and-pr-review-grep.md: acceptance results, ai-slop-clean, performance, docs, three implementation review rounds, and the verification log. - Verify: issueops gates check re-ran G1-G7 and G10 fresh after the last code change (all met). - Risk: Low; documentation 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
1 participant
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.
요약
#548·#550에서 미뤘던 결함 세 가지를 고칩니다.
systemctl --user enable에서 실패했습니다.defs.md의source:줄에 실제로 쓴 검색 도구가 표시됩니다..gitignore로 무시되는 파일을 검색하지 않습니다.Closes #552
변경 내용
internal/adapter/mcpservice)Prepare의 첫 단계에서 systemd 어댑터가 실행 중인 user manager의HOME·XDG_CONFIG_HOME(systemctl --user show-environment)을 읽습니다. 쓰려는 unit 디렉터리가 manager의 기대 디렉터리와 다르면systemctl --user show -p UnitPath --value목록에서 한 번 더 찾습니다.--mcp-transport=stdio)이 들어갑니다.issueops mcp service start는 이 거부를 이전과 같은supervisor_failed로 보고합니다.scripts/ci_workflow_test.py가 이 단계의 실패 전파를 고정합니다.skills/pr-review/scripts/mr_context.py)source:줄에 쓰인 순서대로 나열합니다.git grep --untracked로 찾습니다. 제외 패턴은 glob magic으로 써서blocks/처럼 이름에 lock이 든 디렉터리를 지우지 않습니다. git grep이 실패하면 기존 grep을 씁니다.install.md의 Linux HOME 설명을 고쳤고, 이번에 만난 함정을 새 주의사항으로 남겼습니다.바뀌지 않는 것: Linux 기본 transport,
mcpservice.Status와 error_code 집합, CLI·MCP JSON 출력, rg가 있을 때의 pr-review 검색, macOS 설치.확인한 것
XDG_CONFIG_HOME, UnitPath 일치, 조회 실패, HOME 부재, 이스케이프 값, launchd에서는 기존 동작을 유지합니다. 수정 전 코드에서는 거부 케이스가 실패했습니다.mcp service start의 error_code: Start 경로 테스트로supervisor_failed와 credential 미생성을 확인했습니다. 이 결함(credential_failed로 보고)은 구현 리뷰 1라운드에서 나왔습니다.systemctl로 실패를 재현했고, 테스트를 실제 manager와 분리해 통과를 확인했습니다.HOME=/home/runner를 돌려주었고, 설치가 쓰기 전에 위 오류로 멈춘 것을 확인했습니다. 계획 단계에서 남긴 가정(manager 환경에 HOME이 있다)이 실제 runner에서 맞았습니다.리뷰 포인트
$'...'이스케이프)이 적절한지 봐 주세요. 막지 않으면 기존처럼 enable 단계에서 명령과 출력을 담아 실패합니다.남은 일
.issueops/adr/roadmap.md줄 수 초과는 이번 변경 전부터 있던 문제입니다.