Skip to content

refactor(orchestrator): warn on dirty workspaces - #915

Merged
moncher-dev merged 8 commits into
mainfrom
symphony/hojinzs-github-symphony-905
Sep 6, 2026
Merged

refactor(orchestrator): warn on dirty workspaces#915
moncher-dev merged 8 commits into
mainfrom
symphony/hojinzs-github-symphony-905

Conversation

@moncher-dev

@moncher-dev moncher-dev commented Sep 5, 2026

Copy link
Copy Markdown
Collaborator

Issues

Summary

  • Replaces dirty-workspace quarantine and attribution with observable warnings that preserve the original workspace.
  • Runs foreign-branch recovery from a deterministic disposable checkout while retaining the original branch and uncommitted files for operators.
  • Removes obsolete attribution and linked-PR adapter hooks, including the declaration accidentally restored while merging main.

Change-point diagram

  • recovery dirty-status inspection → retained-workspace warning → same-path recovery or deterministic -recovery checkout
  • structured recovery event → formatter/status snapshot → operator-visible retained path, branch, and active recovery path
  • tracker adapter contract → removes dead dirty-attribution and linked-PR extension surfaces

Start here

  • packages/orchestrator/src/service.ts — dirty recovery path selection, retained warning state, and repeat recovery population
  • packages/orchestrator/src/service.test.ts — real redispatch, preservation, and repeat-contamination regression coverage
  • packages/core/src/observability/structured-events.ts — warning event contract

User-Visible Behavior / Operational Impact

  • Dirty workspaces are not moved, renamed, deleted, or treated as a failure.
  • A dirty workspace on a foreign issue branch is retained unchanged while the run proceeds from a stable fresh checkout.
  • Legacy .quarantine-* directories remain operator-managed.

Validation

  • pnpm lint — pass
  • pnpm test — pass (all workspace packages)
  • pnpm typecheck — pass
  • pnpm build — pass
  • pnpm exec prettier --check packages/core/src/contracts/tracker-adapter.ts — pass
  • git diff --check — pass
  • Docker E2E — environment exception documented in fix(e2e): restore Docker daemon access for Symphony workers #914: Docker daemon unavailable on two attempts during the behavior-changing cycles; no container started

Changeset

  • .changeset/warm-dogs-warn.md (patch for @gh-symphony/cli)

Risks & rollback

Changed files

  • packages/orchestrator/src/service.ts, git.ts, and tests — warn, preserve, and recover from deterministic active paths
  • packages/core/src/workflow/issue-identity.ts and tests — remove dirty-work attribution
  • packages/core/src/observability/* — warning event and formatting
  • packages/core/src/contracts/tracker-adapter.ts, packages/tracker-linear/src/* — remove dead adapter hooks
  • WORKFLOW.md, AGENT_TEST.md, docs/**, e2e/scenarios/09-linear-sandbox.md — align policy and living documentation
  • .changeset/warm-dogs-warn.md — release note

Post-merge / human validation

Security

  • No real tokens, private keys, .env files, or generated installation tokens are committed

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 5, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-05T16:14:03.651900Z 91bc279 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 40.74% 41825 / 102658
🔵 Statements 40.74% 41825 / 102658
🔵 Functions 72.7% 2054 / 2825
🔵 Branches 79.35% 9667 / 12182
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
packages/core/src/contracts/tracker-adapter.ts 53.84% 100% 0% 53.84% 14-19
packages/core/src/observability/event-formatter.ts 85.71% 68% 100% 85.71% 12-14, 16-18, 22, 36, 38, 42, 46, 48, 52, 64, 112-113
packages/core/src/observability/structured-events.ts 100% 100% 100% 100%
packages/core/src/workflow/issue-identity.ts 100% 100% 100% 100%
packages/orchestrator/src/git.ts 80.71% 85.71% 83.33% 80.71% 54-57, 67-70, 113-114, 154-155, 169-195, 240-241, 254, 263-265
packages/orchestrator/src/service.ts 91.7% 85.58% 95.54% 91.7% 258-259, 304, 306, 318-323, 377, 496-502, 519, 627, 653-654, 659-660, 682, 686, 690, 727-731, 754-772, 801-815, 940-941, 946-947, 951-969, 981-982, 994-995, 1000-1011, 1013-1014, 1046-1063, 1085-1092, 1114-1115, 1127-1128, 1135, 1142-1146, 1162, 1172, 1203, 1229-1230, 1280-1282, 1294-1295, 1302, 1355-1356, 1518, 1520, 1652-1653, 1665-1667, 1809, 1824, 1839, 1864-1868, 1918, 1990-1992, 1997-1998, 2012-2022, 2029, 2054, 2081-2083, 2088-2089, 2116, 2165, 2197-2201, 2305, 2336-2337, 2341-2345, 2349-2350, 2359-2360, 2375, 2402, 2427, 2430-2433, 2556-2572, 2648-2653, 2655-2660, 2724-2725, 2800-2801, 2808-2809, 2839-2842, 2916-2929, 2960-2963, 3047-3050, 3082-3085, 3302-3312, 3327, 3335-3336, 3355-3356, 3375-3379, 3387-3388, 3418, 3430-3435, 3466-3468, 3471-3472, 3515, 3518, 3615, 3916, 3952, 4041, 4043, 4046, 4048-4050, 4053-4054, 4073, 4138-4139, 4144-4148, 4152, 4165-4166, 4215, 4313, 4326-4327, 4338, 4399-4400, 4430-4436, 4496-4500, 4642-4643, 4696-4697, 4723-4724, 4855-4863, 4916, 4972-4979, 4997-5000, 5155, 5169-5175, 5238-5244, 5271, 5281, 5321-5324, 5331-5337, 5487-5490, 5630-5631, 5643-5652, 5718-5719, 5752-5753, 5809-5810, 5987-5988, 6094-6095, 6199, 6292-6293, 6312-6313, 6315-6316, 6318-6319, 6325-6326, 6334-6335, 6434-6465, 6472-6473, 6481-6482, 6501-6502, 6543-6546, 6576, 6693-6694, 6762-6764, 6770-6771, 6784-6785, 6790-6796, 6857-6859, 6993-6994
packages/tracker-linear/src/orchestrator-adapter.ts 92.78% 86.97% 95.55% 92.78% 273, 401-403, 462-463, 490-497, 527-535, 766, 828, 840-846, 849-850, 866-867, 911-912, 918-919, 938-939, 1002-1005, 1020-1029, 1050-1051, 1074-1075
Generated in workflow #1460 for commit adfb1e4 by the Vitest Coverage Report Action

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 91bc279ecc

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread packages/orchestrator/src/service.ts Outdated
Comment thread packages/core/src/observability/event-formatter.ts Outdated
Comment thread docs/architecture.md Outdated
@moncher-dev
moncher-dev requested a review from hojinzs September 5, 2026 16:32
@moncher-dev

Copy link
Copy Markdown
Collaborator Author

Reviewed at 91bc279e, and the decided behaviour is exactly what the code does: warn and proceed — not fail, not move, not delete.

Checked against the branch of the decision that a wrong implementation would have taken: the dirty path no longer calls quarantineIssueWorkspace, does not rm, does not fail the run, and does not touch the tracker state. It writes one stderr warning plus a recovery-dirty-workspace run event, then continues into the same recovery dispatch as before (SYMPHONY_RECOVERY_KIND=incomplete-turn-dirty-workspace still reaches the worker).

The retention test is a real one. warns and continues when a recovery workspace is dirty plants a foreign artifact (.gh-symphony/workpads/2.md, i.e. issue 2's workpad) on a foreign branch fix/2-foreign while dispatching a different issue — the exact case the old code quarantined — and then asserts the file still exists with its original content, git status --porcelain still reports it, HEAD is still fix/2-foreign, and no *.quarantine-* sibling was created.

Mutation-checked — the tests can fail (each mutation applied alone, then reverted):

mutation of the source result
add rm(issueWorkspacePath, {recursive:true}) to the dirty branch red: ENOENT … repository/.gh-symphony/workpads/2.md
drop the recovery-dirty-workspace appendRunEvent red: expected … to contain '"event":"recovery-dirty-workspace"'
change the formatter string to "mutant" red: event-formatter > formats supported orchestrator events

Warning is observable on all three surfaces: stderr (dirty workspace retained for <issue>; continuing recovery without moving or deleting files), events.ndjson ("event":"recovery-dirty-workspace" with "dirtyFiles":[".gh-symphony/"]), and the operator formatter (Dirty workspace retained; recovery is continuing).

inspectIssueWorkspaceDirtyStatus survives in packages/orchestrator/src/git.ts:448 and is still called from three sites in service.ts — only quarantineIssueWorkspace and attributeDirtyWorkToIssue are gone. No stale recovery-quarantined references remain in any package, and docs/AGENT_TEST.md/e2e scenario 09 were updated to match.

pnpm build clean; 706 tests pass across orchestrator + core.

@hojinzs hojinzs left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

리뷰 1/6 — 변경 요청

91bc279에서 검토했습니다. 방향은 맞습니다. 임의로 실패 처리를 고르지 않았고, quarantine·귀속을 한 덩어리로 제거했으며, 608줄을 지우고 61줄을 더하는 삭제 중심 refactor라 오버엔지니어링도 없습니다. 오케스트레이터 레이어만 보면 결정대로 동작합니다.

다만 결정의 핵심이 워커 경계에서 무너집니다. 그래서 변경 요청입니다.

완료 조건 대조

#905 완료 조건 상태
더러운 워크스페이스를 만나면 경고를 남기고 진행. 이동/실패/삭제 없음 ⚠️ 오케스트레이터는 지키지만 런은 워커 startup에서 실패 (P1)
경고가 관측 가능 ✅ stderr + recovery-dirty-workspace 구조화 이벤트 + 포매터
.quarantine-* 미생성 ✅ 테스트가 sibling 부재를 단언
귀속 판정 코드와 이벤트 제거 ⚠️ 어댑터 훅 5곳 잔존 (P2)
WORKFLOW.md 문구 정리 ✅ 문구만 고치고 다른 규칙 의미는 안 건드림
docs/architecture.md 정리
changeset .changeset/warm-dogs-warn.md (patch)

P1 — 경고 후 진행이 실제로는 실패로 끝납니다

워크스페이스를 그 자리에 두면 브랜치도 그대로 남습니다. 워커는 에이전트를 띄우기 전에 identity preflight를 돌리고(worker/src/index.ts:535), 브랜치가 다른 이슈 번호를 가리키면 fail-closed로 거부합니다(worker/src/identity-preflight.ts:100-108).

이 PR의 회귀 테스트가 만드는 바로 그 상황을 실제 preflight에 넣어 확인했습니다:

{"ok":false,"reason":"workspace branch 'fix/2-foreign' belongs to issue #2 instead of 'acme/platform#1'"}

exitWorkerStartupFailure로 이어져 런이 시작조차 못 합니다. 이제 워크스페이스를 옮기지도 지우지도 않으니 재시도해도 브랜치가 그대로라 재시도 예산이 소진될 때까지 같은 벽에 부딪힙니다. 예전 quarantine 경로는 깨끗한 클론을 새로 만들어 이 문제를 우연히 피하고 있었습니다.

결과적으로 "사람이 판단하도록 보여준다" 는 이 이슈의 목적 대신 조용한 반복 실패가 됩니다. 완료 조건이 명시적으로 금지한 "실패"입니다.

service.test.ts의 회귀 테스트는 spawn을 목킹하므로 이 경로를 한 번도 타지 않습니다. dispatch 도달만 검증하고 에이전트 기동은 검증하지 못합니다.

그 외

  • P2 — 경고에서 currentBranch가 빠졌습니다. 이벤트에도 복구 프롬프트에도 브랜치가 없어, 운영자가 남은 변경의 소속을 판단할 근거가 부족합니다.
  • P2resolveAttributableBranches 훅이 계약·Linear 구현·테스트·문서 2곳에 남아 있습니다. 소비자는 없습니다.
  • nit — 살아남는 함수들의 근거 주석까지 축약되었고, event-formatter.test.ts에 무관한 재포매팅이 있습니다.

Codex 지적 중 정정

Codex의 P2 "Classify the new recovery event as a warning"현재 코드베이스에 해당하지 않습니다. logs 명령은 이미 제거되었고(cli/src/index.ts:544), packages/cli/src/commands/logs.ts는 어디에서도 import되지 않는 고아 모듈입니다. --level warn 필터는 도달 불가능하므로 이 지적에 라운드를 쓰지 마세요. #905은 "구조화 이벤트 또는 상태 스냅샷 경고" 를 요구하고 구조화 이벤트로 이미 충족됩니다.

검증

  • pnpm build — 통과
  • core / orchestrator / worker / cli 테스트 — 1,518개 전부 통과 (338 + 368 + 241 + 571)
  • CI(Test, Container Smoke) — green
  • P1은 실제 evaluateWorkerIdentityPreflight를 PR 테스트와 동일한 입력으로 실행해 재현했습니다

P1만 해결되면 나머지는 가벼운 정리입니다. 방향 선택은 결정 사항이라 제가 고르지 않았고, 인라인에 선택지만 적어두었습니다.


Generated by Claude Code

Comment thread packages/orchestrator/src/service.ts Outdated
Comment thread packages/core/src/observability/structured-events.ts
Comment thread docs/architecture.md Outdated
Comment thread packages/core/src/workflow/issue-identity.ts Outdated
@moncher-dev
moncher-dev requested a review from hojinzs September 6, 2026 00:26

@hojinzs hojinzs left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

리뷰 2/6 — 변경 요청

4fefdf8에서 검토했습니다. 지난 라운드 지적 4건은 모두 처리되었습니다. 확인했습니다.

지난 라운드 상태
P1 워커 identity preflight 실패 ✅ preflight를 약화시키지 않고 우회 — 방향 선택은 존중합니다
P2 경고에 currentBranch 누락 workspacePath / currentBranch / recoveryWorkspacePath 추가
P2 resolveAttributableBranches 잔존 ✅ 계약·Linear 구현·테스트·문서 2곳 모두 제거
nit 범위 밖 JSDoc 축약 / 무관한 재포매팅 ✅ 근거 주석 복원, 포매터 테스트 diff 축소
Codex P2 (고아 CLI 분류기) ✅ 근거 있는 미반영 의사를 스레드에 남김 — 적절합니다

그런데 P1 해결책이 한 dispatch 뒤에 삭제를 만들어냅니다. 그래서 다시 변경 요청입니다.

#905 완료 조건 대조

완료 조건 상태
더러운 워크스페이스를 만나면 경고를 남기고 진행. 이동/실패/삭제 없음 ⚠️ same-issue 케이스는 ✅ / foreign 케이스는 다음 dispatch에서 삭제 (P1)
경고가 관측 가능 ⚠️ 구조화 이벤트 + 포매터 ✅ / 단 필드 출처가 섞임 (P2)
.quarantine-* 미생성 rename import까지 제거, 테스트가 sibling 부재 단언
귀속 판정 코드와 이벤트 제거 ✅ 잔존 참조는 과거 CHANGELOG뿐
WORKFLOW.md 문구 정리 ⚠️ 요청대로 문구만 고침 ✅ / 단 전제가 깨짐 (P2)
docs/architecture.md 정리 ⚠️ "delete 하지 않는다"가 현재 코드보다 강함 (P3)
changeset .changeset/warm-dogs-warn.md (patch)

P1 — 보존한 더러운 워크스페이스가 다음 dispatch에서 삭제됩니다

useFreshRecoveryWorkspace가 워크스페이스 레코드<key>-recovery로 옮기는데, workspaceKey<key> 그대로입니다. 다음 일반 dispatch에서 issueWorkspacePath는 다시 <key>로 파생되고 existingWorkspaceAtConfiguredRoot가 거짓이 되어 needsPopulationrm(repositoryDirectory)가 돕니다. 보존해 둔 바로 그 디렉터리입니다.

실제 runOnce()로 재현했습니다:

PROBE SUMMARY {"dispatched":1, ...}
PROBE RETAINED BRANCH: symphony/tenant-1/acme-platform-1   ← fix/2-foreign 이었음
PROBE RETAINED DIRTY FILE: null                            ← 2.md 소실
PROBE RECORD PATHS: [".../acme_platform_1-0e0ed77a1c2f4ef5"]  ← <key>로 되돌아감

훅 신뢰 승인이 없어 dispatch가 실패하는 경우에도 삭제는 먼저 일어나고, 이어서 catchpopulationWasFresh 경로가 워크스페이스 전체를 지웁니다. 즉 보존은 정확히 한 dispatch 동안만 유지됩니다. #905이 명시적으로 금지한 "삭제"입니다.

근본 원인은 더러움 탐지(세 지점 모두 workspaceKey에서 파생한 설정상 경로)와 실제 런 위치(레코드의 workspacePath)가 서로 다른 소스로 파생된다는 점입니다. 둘이 갈라지는 한 어느 한쪽은 항상 잘못된 디렉터리를 봅니다. P2 두 건도 같은 뿌리입니다.

그 외

  • P2 — 경고 이벤트가 두 디렉터리의 필드를 섞습니다. reuseStableRecoveryWorkspace 경로에서 workspacePath/currentBranch-recovery, dirtyFiles<key>에서 옵니다. 덤으로 -recovery 안에서 중단된 실제 작업은 영영 dirty로 탐지되지 않습니다.
  • P2 — 복구 프롬프트가 에이전트 체크아웃 밖 경로를 가리키며 "검증하고 커밋해서 푸시하라"고 지시합니다. foreign 케이스에서 #905의 목적(사람이 판단하도록 보여주기)이 달성되지 않습니다.
  • P2 — 회귀 테스트가 첫 dispatch에서 끝나 위 삭제를 못 잡습니다.
  • P3docs/architecture.md의 "delete 하지 않는다"가 현재 코드보다 강한 보장입니다. 그리고 -recovery 사이드 디렉터리는 터미널 정리에서 지워지지 않아, #905이 한 번만 인정한 종류의 잔여물 부채를 반복 생성합니다.

설계에 대한 의견 (차단 아님)

-recovery 사이드 워크스페이스는 제가 지난 라운드에 만든 P1에 대한 응답이므로 임의로 넣은 오버엔지니어링은 아닙니다. 삭제 중심 refactor라는 성격도 유지되고 있습니다(+153 / −636). 다만 결과적으로 운영자 눈에는 여전히 디렉터리 두 개가 남고 그중 하나는 아무도 치우지 않습니다 — quarantine이 워크스페이스를 옮겼다면 이번엔 런을 옮겼을 뿐, 같은 모양의 잔여물이 생깁니다. P1을 고치실 때 이 점도 같이 저울에 올려 주세요. 경로 파생을 한 곳으로 모으는 쪽이 세 지적을 한 번에 닫습니다.

검증

  • pnpm lint / pnpm typecheck / pnpm build — 통과
  • pnpm test1,965개 전부 통과 (orchestrator 345, worker 242, core 331, cli 568, 그 외)
  • CI(Test, Container Smoke) — green
  • Smoke: 실제 OrchestratorService.runOnce()로 recovery dispatch와 그 다음 dispatch를 연속 실행해 P1을 재현. same-issue dirty 경고 경로는 정상 동작 확인
  • Preview 환경 없음. Docker E2E는 #914(데몬 미가용)로 이 세션에서도 수행 불가

P1의 삭제만 닫히면 나머지는 파생 정리와 문구입니다. 방향 선택은 결정 사항이라 이번에도 제가 고르지 않았고, 제약만 인라인에 적어두었습니다.


Generated by Claude Code

Comment thread packages/orchestrator/src/service.ts Outdated
Comment thread packages/orchestrator/src/service.ts
Comment thread WORKFLOW.md Outdated
Comment thread docs/architecture.md Outdated
Comment thread packages/orchestrator/src/service.test.ts
@moncher-dev moncher-dev changed the title refactor(orchestrator): warn on dirty workspaces refactor(orchestrator): replace dirty-workspace quarantine and attribution with a warning Sep 6, 2026
@moncher-dev
moncher-dev requested a review from hojinzs September 6, 2026 01:01

@hojinzs hojinzs left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

리뷰 3/6 — 변경 요청

5f7f86e에서 검토했습니다. 지난 라운드 지적 5건은 모두 처리되었습니다.

지난 라운드 상태
P1 보존 워크스페이스가 다음 dispatch에서 삭제 실제 dispatch로 검증 — 아래 참조
P2 경고 이벤트 필드 출처 혼합 ✅ 세 탐지 지점이 resolveRunIssueWorkspacePath로 통일
P2 복구 프롬프트가 체크아웃 밖을 가리킴 ✅ 조건 분기 추가, WORKFLOW.md도 문구만 대응
P2 회귀 테스트가 첫 dispatch에서 멈춤 ⚠️ 구간은 늘렸으나 그 안에서 dispatch가 0건 (P2)
P3 docs/architecture.md의 delete 보장 ✅ 보장이 참이 되었고 -recovery 정리 책임도 명시

P1은 정말로 닫혔습니다 — 제가 확인한 방법

reuseStableRecoveryWorkspace에서 recovery?.kind 조건을 빼신 것이 핵심이고, 이게 의도대로 동작합니다. 별도 하네스로 진짜 일반 dispatch를 강제해 확인했습니다:

PROBE[record-at-recovery] SUMMARY {"dispatched":1,...}
PROBE[record-at-recovery] 2.md EXISTS:     true
PROBE[record-at-recovery] RETAINED BRANCH: fix/2-foreign
PROBE[record-at-recovery] SPAWN CWD:       .../-recovery/repository

그리고 대조군으로 그 조건 한 줄만 되돌리면 삭제가 재현됩니다(2.md EXISTS: false, 브랜치가 배정 브랜치로 덮임). 수정이 원인이고 결과라는 것까지 확인했습니다.

완료 조건 대조

#905 완료 조건 상태
더러운 워크스페이스를 만나면 경고를 남기고 진행. 이동/실패/삭제 없음 ⚠️ 1차 오염은 ✅ / 2차 오염에서 "실패" 재발 (P2)
경고가 관측 가능 ⚠️ 이벤트·포매터·stderr ✅ / 단 1회성 (P3)
.quarantine-* 미생성
귀속 판정 코드와 이벤트 제거 ✅ 리포 전체에서 잔존 참조 0건 (CHANGELOG 제외)
WORKFLOW.md 문구 정리 ✅ 문구만 고침, 다른 규칙 의미 유지
docs/architecture.md 정리
changeset .changeset/warm-dogs-warn.md (patch)

이번 라운드 지적

  • P2 — 탈출구가 이슈당 한 번만 열립니다. useFreshRecoveryWorkspace = retainedWorkspaceHasForeignBranch && !reuseStableRecoveryWorkspace 이므로, 레코드가 -recovery로 옮겨간 뒤에는 -recovery 자신이 foreign 브랜치로 오염되어도 탈출이 불가능합니다. 실제 runOnce()로 재현했고(ASSIGNED BRANCH: fix/3-foreign), 그 입력을 실제 evaluateWorkerIdentityPreflight에 넣으면 1라운드와 같은 fail-closed 거부가 나옵니다. #905이 금지한 "실패" 입니다. -recovery는 운영자 증거물이 아니라 오케스트레이터 소유 스크래치이므로, 그 자리에서 repopulate하는 최소 수정이 가능해 보입니다 — 무한 체이닝을 요구하는 게 아닙니다.
  • P2 — 새로 추가된 세 번째 runOnce()가 dispatch를 0건 합니다 (activeRuns: 1로 스킵). 아래 단언들이 "보존되었다"가 아니라 "아무 일도 없었다"를 통과시킵니다. 수정 자체는 유효하니 급한 건 아니지만, 이 회귀를 지키는 테스트가 지금은 없습니다.
  • P3 — 보존된 <key>가 첫 dispatch 이후 관측에서 사라집니다. 경고가 사실상 1회성이라 운영자가 다시 마주칠 표면이 없습니다.
  • nit — 프롬프트 분기 문구의 at ${activeWorkspacePath} 오독 여지와, "손대지 마라" 직후에 붙는 Suggested operator command.

오버엔지니어링 / 임의 결정 점검

없습니다. -recovery 사이드 워크스페이스는 제가 1라운드에 만든 P1에 대한 응답이고, 이번 변경도 제가 2라운드에 지적한 제약(경로 파생을 한 곳으로)을 그대로 따랐습니다. 요구사항에 없는 기능 추가는 보이지 않고, 여전히 삭제 중심 refactor입니다(+223 / −651).

검증

  • pnpm lint / pnpm typecheck / pnpm build — 통과
  • pnpm test1,965개 전부 통과
  • pnpm format — 65개 파일이 실패하나 이 PR이 건드린 파일은 하나도 포함되지 않습니다(리포 전반의 기존 드리프트). 이 PR 사유 아님
  • CI — Test / Container Smoke 모두 green (5f7f86e)
  • Smoke: 실제 OrchestratorService.runOnce()로 (a) recovery dispatch 후 일반 dispatch 보존, (b) 대조군 회귀 재현, (c) -recovery 2차 오염을 각각 실행. 실제 evaluateWorkerIdentityPreflight로 (c)의 거부 확인
  • Preview 환경 없음. Docker E2E는 이 세션에서도 데몬 미가용(#914)이라 수행 불가 — 오케스트레이션 계층 변경이라 위 runOnce() 스모크로 대체했습니다

다음 라운드에 대해

P2 두 건만 닫히면 승인하겠습니다. P3와 nit은 차단 사유가 아니고, 판단하신 대로 반영하지 않으셔도 근거만 스레드에 남겨 주시면 됩니다. 방향 선택은 이번에도 제가 고르지 않았습니다.


Generated by Claude Code

Comment thread packages/orchestrator/src/service.ts Outdated
Comment thread packages/orchestrator/src/service.test.ts Outdated
Comment thread packages/orchestrator/src/service.ts
Comment thread packages/orchestrator/src/service.ts Outdated
@moncher-dev
moncher-dev requested a review from hojinzs September 6, 2026 01:31

@hojinzs hojinzs left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

리뷰 4/6 — 승인

3e76dba에서 검토했습니다. 지난 라운드에 차단 사유로 걸었던 P2 두 건이 모두 닫혔습니다. 3라운드에 "P2 두 건만 닫히면 승인하겠습니다" 라고 적었고, 그 약속대로 승인합니다.

지난 라운드 상태
P2 탈출구가 이슈당 한 번만 열림 (2차 오염에서 실패 재발) 실제 dispatch로 검증 — 아래 참조
P2 세 번째 runOnce()가 dispatch 0건 dispatched === 1 단언 추가, 2차 오염 dispatch까지 커버
P3 보존된 <key>가 관측에서 사라짐 ✅ 프로젝트 상태 경고가 원본이 깨끗해질 때까지 지속
nit 프롬프트 문구 오독 / Suggested operator command ✅ 괄호 처리 + Operator-only inspection command (do not run from this worker)

P2 두 건은 정말로 닫혔습니다 — 확인한 방법

repopulateStableRecoveryWorkspaceneedsPopulation을 켜는 것이 핵심이고, 의도대로 동작합니다. 결정 지점에 계측을 넣고 테스트가 만드는 세 번의 dispatch를 그대로 관찰했습니다:

PROBE-DECIDE {"recoveryKind":"incomplete-turn-dirty-workspace","retainedWorkspaceBranch":"fix/2-foreign",
              "useFreshRecoveryWorkspace":true,  "repopulateStableRecoveryWorkspace":false}
PROBE-DECIDE {"recoveryKind":null,"retainedWorkspaceBranch":"symphony/tenant-1/acme-platform-1",
              "reuseStableRecoveryWorkspace":true,"repopulateStableRecoveryWorkspace":false}
PROBE-DECIDE {"recoveryKind":null,"retainedWorkspaceBranch":"fix/3-foreign",
              "reuseStableRecoveryWorkspace":true,"repopulateStableRecoveryWorkspace":true}

세 번째에서 탈출구가 다시 열립니다. -recovery-recovery는 생기지 않고, 배정 브랜치가 복원되며, 원본 <key>fix/2-foreign / 2.md는 그대로입니다. 1·3라운드에서 fail-closed 거부를 만들던 입력이 더 이상 워커에 도달하지 않습니다.

테스트 쪽도 이번엔 공회전이 아닙니다. laterDispatch.summary.dispatched === 1을 명시적으로 단언하고, spawnImpl이 3회 호출되며, 세 번째 호출의 SYMPHONY_ASSIGNED_BRANCH까지 검증합니다. 3라운드에 제가 요청한 그대로입니다.

#905 완료 조건 대조

완료 조건 상태
더러운 워크스페이스를 만나면 경고를 남기고 진행. 이동/실패/삭제 없음 ✅ 원본에 대해 충족 (-recovery 재채움 경로의 경고 누락은 P2, 차단 아님)
경고가 관측 가능 ✅ 구조화 이벤트 + 포매터 + stderr + 지속되는 프로젝트 상태 경고
.quarantine-* 미생성 ✅ 테스트가 sibling 부재를 단언
귀속 판정 코드와 그 이벤트 제거 ✅ 잔존 참조는 과거 CHANGELOG뿐
WORKFLOW.md 문구 정리 ✅ 문구만 고침, 다른 규칙 의미 유지
docs/architecture.md 정리 ✅ 재채움과 정리 책임까지 서술
changeset .changeset/warm-dogs-warn.md (patch)

병합 전에 하나 — merge conflict

main과 충돌합니다. 승인과 별개로 병합 전에 풀어야 합니다.

CONFLICT (content): Merge conflict in AGENT_TEST.md
CONFLICT (content): Merge conflict in packages/core/src/contracts/tracker-adapter.ts

#919(orchestrator 코멘트 제거)가 tracker-adapter.ts의 같은 구역에서 findActiveLinkedPullRequest와 코멘트 타입들을 지웠고, 이 PR은 그 옆에서 resolveAttributableBranches를 지웁니다. 양쪽 삭제라 해소는 기계적일 겁니다 — 둘 다 제거된 상태로 맞추시면 됩니다. AGENT_TEST.md도 같은 모양으로 보입니다.

3e76dba에는 체크런과 커밋 상태가 아직 하나도 붙지 않았습니다(total_count: 0, pending). 충돌을 풀고 푸시하면 CI가 다시 돌 테니, 그 결과만 green인지 확인해 주세요.

이번 라운드 지적 (전부 비차단)

  • P2-recovery 재채움 경로가 미커밋 파일이 있는 체크아웃을 지우면서 이벤트도, stderr도, 프롬프트 Recovery Context도 남기지 않습니다. 삭제 자체는 제가 지난 라운드에 제안한 방향이라 문제 삼지 않지만, #905의 첫 조건이 "경고를 남기고" 이므로 한 줄 이벤트가 있으면 깔끔합니다. 충돌 푸시하는 김에 넣으실지 판단만 해주세요.
  • P3 — 브랜치 검사가 비대칭입니다. <key>깨끗한 채로 남의 브랜치에 있으면 여전히 그물을 빠져나가 워커 preflight에서 거부됩니다. 이 PR의 회귀는 아니고 #905 범위 밖이라 별도 이슈를 권합니다.
  • nit — 새 상태 경고가 상태 조회마다 워크스페이스당 git 서브프로세스 2개를 띄웁니다.

오버엔지니어링 / 임의 결정 점검

없습니다. -recovery 사이드 워크스페이스와 그 자리 재채움은 각각 제가 1·3라운드에 만든 지적에 대한 응답이고, 요구사항에 없는 기능 추가는 보이지 않습니다. 여전히 삭제 중심 refactor입니다(+370 / −653). 4라운드 동안 커진 것은 회귀 테스트(+106)와 경고 표면이지 제품 코드의 범위가 아닙니다. 임의로 실패 처리를 고르지 않았다는 #905의 완료 조건도 끝까지 지켜졌습니다.

검증

  • pnpm lint / pnpm typecheck / pnpm build — 통과
  • pnpm test1,965개 전부 통과 (orchestrator 345, worker 242, core 331, cli 568, 그 외)
  • Smoke: 워크스페이스 선택 지점에 계측을 넣고 세 번의 실제 dispatch를 관찰해 2차 오염 재채움과 원본 보존을 확인. 대조 계측으로 -recovery 삭제 시 경고 부재도 확인
  • Preview 환경 없음. Docker E2E는 이 세션에서도 데몬 미가용(#914)이라 수행 불가 — 오케스트레이션 계층 변경이라 위 runOnce() 스모크로 대체했습니다
  • 계측은 전부 되돌렸고 작업 트리는 깨끗합니다

merge conflict만 풀어서 CI green을 확인하시면 병합 가능합니다. 위 P2·P3·nit은 반영하시든, 근거를 스레드에 남기고 넘기시든 제 쪽에서 더 돌리지 않겠습니다.


Generated by Claude Code

Comment thread packages/orchestrator/src/service.ts
Comment thread packages/orchestrator/src/service.ts
Comment thread packages/orchestrator/src/service.ts

@hojinzs hojinzs left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

리뷰 5/6 — 승인 유지, 병합 전 5줄만

f26873c에서 검토했습니다. 4라운드에 병합 조건으로 걸었던 merge conflict가 풀렸고 CI도 green입니다. 승인은 그대로 유지합니다. 다만 충돌 해소 중 한 헝크가 어긋나서, 그것만 짚습니다.

4라운드 병합 조건 상태
AGENT_TEST.md 충돌 해소 ✅ 양쪽 보존 — PR의 새 행 + #919의 시나리오 12 문구, 시나리오 09는 PR 쪽 "dirty-workspace recovery"
tracker-adapter.ts 충돌 해소 ⚠️ resolveAttributableBranches 제거는 ✅ / #919이 지운 훅이 되살아남 (P2)
CI green ✅ Test / Container Smoke 모두 success (f26873c, 01:57 UTC)

머지 충실도 — 확인한 방법

머지 커밋을 양쪽 부모와 각각 대조했습니다. PR 쪽 작업은 전부 살아 있습니다:

$ git diff db7f44e f26873c --stat
 17 files changed, 375 insertions(+), 653 deletions(-)
 .changeset/warm-dogs-warn.md, WORKFLOW.md, docs/architecture.md,
 orchestrator/{service.ts,service.test.ts,git.ts},
 core/{observability/*, workflow/issue-identity*}, tracker-linear/* ...

service.ts의 머지 diff(259줄)는 전부 #919의 코멘트 발행 제거였고, 복구 로직에는 한 줄도 닿지 않았습니다. reuseStableRecoveryWorkspace / repopulateStableRecoveryWorkspace / useFreshRecoveryWorkspace / resolveRunIssueWorkspacePath 모두 3e76dba와 동일하게 남아 있습니다. 그래서 4라운드에 실제 dispatch로 검증한 동작이 이 헤드에서도 그대로 유효합니다.

이번 라운드 지적 (1건)

  • P2findActiveLinkedPullRequest가 계약에 되살아났습니다. #919이 선언·구현·소비자·테스트 네 곳을 지웠는데 머지가 선언만 PR 쪽에서 가져왔습니다. 리포 전체에 소비자 0건, 문서 참조 0건. optional 메서드라 타입체크도 테스트도 잡지 못합니다. 1라운드에 지적하고 이 PR이 닫은 resolveAttributableBranches와 같은 모양의 함정이고, #919의 일부를 되돌립니다. 인라인에 대조 근거와 지울 5줄을 적어두었습니다.

#905 완료 조건 대조 (최종)

완료 조건 상태
더러운 워크스페이스를 만나면 경고를 남기고 진행. 이동/실패/삭제 없음 ✅ 원본에 대해 충족
경고가 관측 가능 ✅ 구조화 이벤트 + 포매터 + stderr + 지속되는 프로젝트 상태 경고
.quarantine-* 미생성
귀속 판정 코드와 그 이벤트 제거 ✅ 잔존 참조는 과거 CHANGELOG와 테스트의 부재 단언뿐
WORKFLOW.md 문구 정리
docs/architecture.md 정리
changeset .changeset/warm-dogs-warn.md (patch)

4라운드 비차단 3건

repopulate 경로의 경고 누락(P2), 브랜치 검사 비대칭(P3), 상태 조회 git 서브프로세스(nit) — 세 스레드 모두 아직 열려 있고 응답이 없습니다. 4라운드에 "반영하시든 근거를 스레드에 남기고 넘기시든 제 쪽에서 더 돌리지 않겠습니다" 라고 적었고 그 약속을 지킵니다. 차단하지 않습니다. 위 5줄 푸시하시는 김에 한 줄씩만 정리해 주세요. P3는 이 PR의 회귀가 아니므로 별도 이슈 권장이 그대로 유효합니다.

오버엔지니어링 / 임의 결정 점검

없습니다. 이번 커밋은 순수 머지이고 새 결정이 들어오지 않았습니다. 최종 규모는 +375 / −653으로 다섯 라운드 내내 삭제 중심 refactor를 유지했습니다. "임의로 실패 처리를 고르지 마세요" 라는 #905의 완료 조건도 끝까지 지켜졌습니다.

검증

  • pnpm lint / pnpm typecheck / pnpm build — 통과 (typecheck는 build 선행 필요, 신선한 클론에서의 순서 문제일 뿐 이 PR 사유 아님)
  • pnpm test1,950개 전부 통과 (cli 569, orchestrator 340, core 331, worker 242, tracker-github 123, runtime-claude 107, 그 외)
  • CI(f26873c) — Test / Container Smoke success
  • Smoke: 머지 헤드에서 dirty-recovery 회귀를 직접 실행해 same-issue 경고 경로와 foreign 경로의 fresh 워크스페이스 전환을 로그로 확인
    dirty workspace retained ... branch=feat/1-partial; continuing recovery without moving or deleting files
    dirty workspace retained ... branch=fix/2-foreign; continuing recovery from fresh workspace=...-recovery without moving or deleting files
    
  • 머지 충실도: 양쪽 부모와 파일별 대조, service.ts 머지 diff 259줄이 전부 #919 코멘트 제거임을 확인
  • Preview 환경 없음. Docker E2E는 이 세션에서도 데몬 미가용(#914)이라 수행 불가 — 오케스트레이션 계층 변경이라 위 스모크로 대체했습니다
  • 작업 트리는 깨끗합니다

인라인 5줄만 지우고 CI green 확인하시면 병합하셔도 됩니다. 승인은 유지되며 제 쪽에서 추가 라운드는 돌리지 않겠습니다.


Generated by Claude Code

Comment thread packages/core/src/contracts/tracker-adapter.ts Outdated
@moncher-dev moncher-dev changed the title refactor(orchestrator): replace dirty-workspace quarantine and attribution with a warning refactor(orchestrator): warn on dirty workspaces Sep 6, 2026
@moncher-dev
moncher-dev requested a review from hojinzs September 6, 2026 02:23

@hojinzs hojinzs left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

리뷰 6/6 — 승인, 병합 가능

adfb1e4에서 검토했습니다. 5라운드에 병합 조건으로 남긴 5줄이 제거되었고, CI green에 충돌도 없습니다. 승인하고 이 PR에 대한 제 리뷰를 여기서 종료합니다.

5라운드 조건 상태
findActiveLinkedPullRequest 5줄 제거 ✅ 정확히 그 5줄, 다른 변경 없음
CI green ✅ Test / Container Smoke 모두 success (adfb1e4, 02:26 UTC)
4라운드 비차단 3건 응답 ✅ 3건 모두 스레드에 근거 있는 응답

마지막 커밋 — 확인한 방법

f26873c → adfb1e4 디프는 1파일 5줄 삭제뿐입니다. 그리고 이 파일을 main과 직접 대조하면, 이제 차이가 이 PR이 의도적으로 지운 resolveAttributableBranches 두 줄뿐입니다:

$ git diff db7f44e adfb1e4 -- packages/core/src/contracts/tracker-adapter.ts
-  /** Return tracker-owned branch evidence for dirty-workspace attribution. */
-  resolveAttributableBranches?(issue: TrackedIssue): string[];

즉 머지가 되살렸던 #919의 훅은 완전히 사라졌고, 계약 파일에 그 외 어떤 잔여물도 남지 않았습니다. 리포 전체 검색에서 findActiveLinkedPullRequest / resolveAttributableBranches / DirtyWorkAttribution / quarantineIssueWorkspace 참조 0건입니다(과거 CHANGELOG 제외). WorkflowLifecycleConfig import는 214·266줄에서 계속 쓰이므로 미사용 import도 생기지 않았습니다 — lint/typecheck로 확인했습니다.

4라운드 비차단 3건 — 응답 확인

세 스레드 모두 응답이 달렸고, 근거가 타당합니다. 어느 것도 다시 열지 않겠습니다.

  • P2 (repopulate 경로 경고 누락) — 미반영. "-recovery는 오케스트레이터 소유의 폐기 가능한 스크래치이고 그 자리 재채움은 리뷰어가 지시한 해소책이며, 보호 대상인 원본은 지속되는 경고를 계속 낸다". 제가 지난 라운드에 직접 그 방향을 제안했으므로 일관됩니다. 근거 있는 미반영으로 받습니다.
  • P3 (브랜치 검사 비대칭) — #927로 분리. 이슈를 확인했고 재현 조건·기대 동작·커버리지 제안까지 제가 적은 내용이 그대로 담겼습니다. 제가 권한 그대로입니다.
  • nit (상태 조회 git 서브프로세스) — 미반영. 캐시는 무효화 의미론을 새로 들여오므로 #905 범위 밖이라는 판단. 동의합니다.

#905 완료 조건 — 최종

완료 조건 상태
더러운 워크스페이스를 만나면 경고를 남기고 진행. 이동/실패/삭제 없음
경고가 관측 가능 ✅ 구조화 이벤트 + 포매터 + stderr + 지속되는 프로젝트 상태 경고
.quarantine-* 미생성 ✅ 테스트가 sibling 부재 단언
귀속 판정 코드와 그 이벤트 제거 ✅ 잔존 참조 0건
WORKFLOW.md 문구 정리 ✅ 문구만, 다른 규칙 의미 유지
docs/architecture.md 정리
changeset .changeset/warm-dogs-warn.md (patch)

"임의로 실패 처리를 고르지 마세요" — 여섯 라운드 내내 지켜졌습니다. 이것이 #905이 완료 조건으로 못 박은 문장이었고, 매 라운드 제가 만든 P1/P2도 전부 "실패로 끝난다"는 지적이었는데 그때마다 실패가 아닌 방향으로 닫혔습니다.

오버엔지니어링 / 임의 결정 — 최종

없습니다. 최종 규모 +370 / −653으로 여섯 라운드 내내 삭제 중심 refactor를 유지했습니다. 늘어난 것은 회귀 테스트(+233)와 경고 표면이지 제품 기능이 아닙니다. -recovery 사이드 워크스페이스(1라운드)와 그 자리 재채움(3라운드)은 각각 제 지적에 대한 응답이고, 요구사항에 없는 기능 추가는 여섯 라운드 어디에도 보이지 않습니다.

검증 (adfb1e4)

  • pnpm install / pnpm build / pnpm typecheck / pnpm lint — 통과
  • pnpm test1,950개 전부 통과, exit 0 (cli 569, orchestrator 340, core 331, worker 242, tracker-github 123, runtime-claude 107, runtime-codex 68, tracker-linear 48, control-plane 38, tracker-file 22, dashboard 21, tool-linear-graphql 20, tool-github-graphql 12, extension-github-workflow 9)
  • CI(adfb1e4) — Test / Container Smoke 모두 success
  • mergeable_state: clean — 충돌 없음
  • Smoke: 최종 헤드에서 dirty-recovery 회귀를 직접 실행해 두 경로를 로그로 확인
    dirty workspace retained ... branch=feat/1-partial; continuing recovery without moving or deleting files
    dirty workspace retained ... branch=fix/2-foreign; continuing recovery from fresh workspace=...-recovery without moving or deleting files
    
  • 머지 충실도: 계약 파일을 main과 직접 대조해 이 PR 고유 삭제 외 차이 0건 확인
  • Preview 환경 없음. Docker E2E는 이 세션에서도 데몬 미가용(#914)이라 수행 불가 — 오케스트레이션 계층 변경이라 위 스모크로 대체했습니다
  • 작업 트리는 깨끗합니다 (소스 변경 없음)

병합하셔도 됩니다

열린 지적이 없고, 모든 스레드가 resolved이며, CI green에 충돌도 없습니다. 여섯 라운드 상한에 도달했고 차단 사유가 남아 있지 않으므로 승인으로 종료합니다. PR 본문의 post-merge 체크 두 건(#914 복구 후 Docker E2E 재실행, 레거시 .quarantine-* 및 보존된 원본 워크스페이스 정리)만 병합 후 챙겨 주세요.


Generated by Claude Code

@moncher-dev
moncher-dev merged commit 66eaf9b into main Sep 6, 2026
2 checks passed
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.

refactor(orchestrator): replace dirty-workspace quarantine and attribution with a warning

2 participants