jihoson/The-Ouroboros
1
0
Fork 0
Code Issues 11 Pull Requests Actions Packages Projects Releases Wiki Activity

governance: require approval evidence for READ-ONLY file changes (#356) #361

Merged
jihoson merged 2 commits from feature/issue-356-readonly-approval into feature/v3-session-policy-stream 2026-03-01 22:46:53 +09:00
Conversation 4 Commits 2 Files Changed 3 +137 -2
agentson commented 2026-03-01 22:09:57 +09:00
Collaborator
Copy Link

Summary

READ-ONLY 파일(src/core/risk_manager.py) 변경 시, PR 본문에 인간 승인 증적과 테스트 증적 2건을 의무화하는 거버넌스 검증을 추가했습니다.

Changes

  • .gitea/PULL_REQUEST_TEMPLATE.md
    • READ-ONLY Approval 섹션 추가
    • 필수 항목:
      • Touched READ-ONLY files
      • Human approval
      • Test suite 1
      • Test suite 2
  • scripts/validate_governance_assets.py
    • READ_ONLY_FILES 도입 (src/core/risk_manager.py)
    • READ-ONLY 파일 변경 시 PR body의 승인/테스트 증적 검증 추가
    • PR body 미제공(non-PR context)은 warning 처리(체크 skip)
    • PR 템플릿 필수 토큰 검증 목록에 READ-ONLY 섹션 토큰 추가
  • tests/test_validate_governance_assets.py
    • 증적 누락 시 실패 테스트
    • 증적 완비 시 통과 테스트
    • PR body 미제공 시 warning 테스트

Validation

  • ruff check scripts/validate_governance_assets.py tests/test_validate_governance_assets.py
  • pytest -q tests/test_validate_governance_assets.py (11 passed)
  • python3 scripts/validate_governance_assets.py origin/feature/v3-session-policy-stream...HEAD

Scope

  • REQ: REQ-OPS-002, REQ-OPS-003
  • TASK: TASK-OPS-002, TASK-OPS-003
  • TEST: TEST-ACC-008, TEST-ACC-009

Closes #356

## Summary READ-ONLY 파일(`src/core/risk_manager.py`) 변경 시, PR 본문에 인간 승인 증적과 테스트 증적 2건을 의무화하는 거버넌스 검증을 추가했습니다. ## Changes - `.gitea/PULL_REQUEST_TEMPLATE.md` - `READ-ONLY Approval` 섹션 추가 - 필수 항목: - `Touched READ-ONLY files` - `Human approval` - `Test suite 1` - `Test suite 2` - `scripts/validate_governance_assets.py` - `READ_ONLY_FILES` 도입 (`src/core/risk_manager.py`) - READ-ONLY 파일 변경 시 PR body의 승인/테스트 증적 검증 추가 - PR body 미제공(non-PR context)은 warning 처리(체크 skip) - PR 템플릿 필수 토큰 검증 목록에 READ-ONLY 섹션 토큰 추가 - `tests/test_validate_governance_assets.py` - 증적 누락 시 실패 테스트 - 증적 완비 시 통과 테스트 - PR body 미제공 시 warning 테스트 ## Validation - `ruff check scripts/validate_governance_assets.py tests/test_validate_governance_assets.py` - `pytest -q tests/test_validate_governance_assets.py` (`11 passed`) - `python3 scripts/validate_governance_assets.py origin/feature/v3-session-policy-stream...HEAD` ## Scope - REQ: `REQ-OPS-002`, `REQ-OPS-003` - TASK: `TASK-OPS-002`, `TASK-OPS-003` - TEST: `TEST-ACC-008`, `TEST-ACC-009` Closes #356
agentson added 1 commit 2026-03-01 22:09:57 +09:00
governance: enforce READ-ONLY approval evidence for protected file changes (#356)
All checks were successful
Gitea CI / test (push) Successful in 32s
Details
Gitea CI / test (pull_request) Successful in 33s
Details
6be78d73ff
agentson commented 2026-03-01 22:12:51 +09:00
Author
Collaborator
Copy Link

PR #361 코드 리뷰

변경 요약: READ-ONLY 파일 변경 시 PR body에 승인 증적 2건을 강제하는 거버넌스 검증 추가

✅ 잘 된 점

GOVERNANCE_PR_BODY 주입이 CI에 이미 반영돼 있다.

# .gitea/workflows/ci.yml
GOVERNANCE_PR_BODY: ${{ github.event.pull_request.body }}

PR 맥락에서는 실제 PR body가 주입되고, push 이벤트에서는 빈 문자열이 돼 warning으로 처리되는 흐름이 의도에 맞다.

_is_placeholder 대소문자 처리가 정확하다.

PLACEHOLDER_VALUES는 소문자 집합이고, normalized.strip().lower() 후 비교하므로 TBD, tbd, N/A, <link> 등을 모두 잡는다.

_parse_pr_evidence_line 엣지케이스 처리가 올바르다.

  • 필드 자체 없음 → None → _is_placeholder(None) = True → 에러
  • 필드는 있지만 값 없음 (- Human approval: ) → '' → _is_placeholder('') = True → 에러

must_contain 연동이 일관적이다.

PR 템플릿에 READ-ONLY Approval 섹션을 추가하고, must_contain 목록에도 동일 토큰을 등록해 템플릿 완전성까지 CI로 보장한다.

11 passed 직접 확인.

🟡 READ_ONLY_FILES에 src/core/order_policy.py 누락

PR #354에서 src/core/order_policy.py도 함께 변경됐다(OrderPolicyRejected → OrderPolicyRejectedError). 이슈 #356이 해당 PR 리뷰에서 발생했음에도 현재 READ_ONLY_FILES는 risk_manager.py만 포함한다.

READ_ONLY_FILES = {"src/core/risk_manager.py"}  # order_policy.py 없음

공식 정책(CLAUDE.md, docs/agents.md)에 명시된 READ-ONLY 파일은 src/core/risk_manager.py뿐이므로 현재 구현이 정책과 일치하는 건 맞다. 다만 order_policy.py도 세션 매매 금지 판단에 직접 관여하므로 정책에 포함시킬지 의사결정이 필요하다.

포함 원할 시:

READ_ONLY_FILES = {
    "src/core/risk_manager.py",
    "src/core/order_policy.py",
}

이번 PR 범위를 벗어나는 정책 결정이므로 블로커는 아니다. CLAUDE.md와 docs/agents.md를 먼저 업데이트하고 별도 PR로 추가하는 편이 적절하다.

🔵 push 이벤트에서 READ-ONLY 파일 변경이 warning만 발생

push: branches: [main]으로 CI가 트리거될 때 GOVERNANCE_PR_BODY가 빈 문자열이 되어 warning만 내고 체크를 skip한다. main에 직접 push하는 경우 승인 검증이 우회된다.

실질적 리스크는 낮다 — main 직접 push는 이미 브랜치 보호로 막혀야 하고, PR flow를 거치면 검증된다. 단, 브랜치 보호가 설정돼 있지 않다면 잠재적 우회 경로가 된다.

🔵 테스트 커버리지 갭 1개

changed_files에 READ-ONLY 파일이 없는 경우(early return) 테스트가 없다. 함수가 silently 통과해야 하는 정상 경로인데 검증이 빠져 있다.

def test_validate_read_only_approval_skips_when_no_readonly_file_changed():
    errors, warnings = [], []
    module.validate_read_only_approval(["src/main.py"], errors, warnings)
    assert errors == []
    assert warnings == []

✅ 최종 판정

항목 상태
승인 증적 필수화 (에러) ✅
placeholder 판별 ✅
CI 환경변수 주입 ✅
must_contain 템플릿 연동 ✅
11 passed ✅
🟡 order_policy.py 정책 결정 필요 정책 질문, 블로커 아님
🔵 push 이벤트 우회 가능성 브랜치 보호로 완화
🔵 non-READ-ONLY 파일 early return 테스트 없음 Minor

기능 목적 달성. Merge 가능 상태.

## PR #361 코드 리뷰 **변경 요약**: READ-ONLY 파일 변경 시 PR body에 승인 증적 2건을 강제하는 거버넌스 검증 추가 --- ### ✅ 잘 된 점 **GOVERNANCE_PR_BODY 주입이 CI에 이미 반영돼 있다.** ```yaml # .gitea/workflows/ci.yml GOVERNANCE_PR_BODY: ${{ github.event.pull_request.body }} ``` PR 맥락에서는 실제 PR body가 주입되고, push 이벤트에서는 빈 문자열이 돼 warning으로 처리되는 흐름이 의도에 맞다. **`_is_placeholder` 대소문자 처리가 정확하다.** `PLACEHOLDER_VALUES`는 소문자 집합이고, `normalized.strip().lower()` 후 비교하므로 `TBD`, `tbd`, `N/A`, `<link>` 등을 모두 잡는다. **`_parse_pr_evidence_line` 엣지케이스 처리가 올바르다.** - 필드 자체 없음 → `None` → `_is_placeholder(None)` = `True` → 에러 - 필드는 있지만 값 없음 (`- Human approval: `) → `''` → `_is_placeholder('')` = `True` → 에러 **must_contain 연동이 일관적이다.** PR 템플릿에 READ-ONLY Approval 섹션을 추가하고, `must_contain` 목록에도 동일 토큰을 등록해 템플릿 완전성까지 CI로 보장한다. **11 passed 직접 확인.** --- ### 🟡 `READ_ONLY_FILES`에 `src/core/order_policy.py` 누락 PR #354에서 `src/core/order_policy.py`도 함께 변경됐다(`OrderPolicyRejected` → `OrderPolicyRejectedError`). 이슈 #356이 해당 PR 리뷰에서 발생했음에도 현재 `READ_ONLY_FILES`는 `risk_manager.py`만 포함한다. ```python READ_ONLY_FILES = {"src/core/risk_manager.py"} # order_policy.py 없음 ``` 공식 정책(`CLAUDE.md`, `docs/agents.md`)에 명시된 READ-ONLY 파일은 `src/core/risk_manager.py`뿐이므로 현재 구현이 정책과 일치하는 건 맞다. 다만 `order_policy.py`도 세션 매매 금지 판단에 직접 관여하므로 정책에 포함시킬지 의사결정이 필요하다. 포함 원할 시: ```python READ_ONLY_FILES = { "src/core/risk_manager.py", "src/core/order_policy.py", } ``` 이번 PR 범위를 벗어나는 정책 결정이므로 블로커는 아니다. `CLAUDE.md`와 `docs/agents.md`를 먼저 업데이트하고 별도 PR로 추가하는 편이 적절하다. --- ### 🔵 push 이벤트에서 READ-ONLY 파일 변경이 warning만 발생 `push: branches: [main]`으로 CI가 트리거될 때 `GOVERNANCE_PR_BODY`가 빈 문자열이 되어 warning만 내고 체크를 skip한다. main에 직접 push하는 경우 승인 검증이 우회된다. 실질적 리스크는 낮다 — main 직접 push는 이미 브랜치 보호로 막혀야 하고, PR flow를 거치면 검증된다. 단, 브랜치 보호가 설정돼 있지 않다면 잠재적 우회 경로가 된다. --- ### 🔵 테스트 커버리지 갭 1개 `changed_files`에 READ-ONLY 파일이 **없는** 경우(early return) 테스트가 없다. 함수가 silently 통과해야 하는 정상 경로인데 검증이 빠져 있다. ```python def test_validate_read_only_approval_skips_when_no_readonly_file_changed(): errors, warnings = [], [] module.validate_read_only_approval(["src/main.py"], errors, warnings) assert errors == [] assert warnings == [] ``` --- ### ✅ 최종 판정 | 항목 | 상태 | |------|------| | 승인 증적 필수화 (에러) | ✅ | | placeholder 판별 | ✅ | | CI 환경변수 주입 | ✅ | | must_contain 템플릿 연동 | ✅ | | 11 passed | ✅ | | 🟡 `order_policy.py` 정책 결정 필요 | 정책 질문, 블로커 아님 | | 🔵 push 이벤트 우회 가능성 | 브랜치 보호로 완화 | | 🔵 non-READ-ONLY 파일 early return 테스트 없음 | Minor | 기능 목적 달성. **Merge 가능** 상태.
jihoson commented 2026-03-01 22:41:46 +09:00
Owner
Copy Link

order_policy.py는 정책에 포함시키지 않겠다.
main 브랜치는 보호 설정되어 있다.

order_policy.py는 정책에 포함시키지 않겠다. main 브랜치는 보호 설정되어 있다.
agentson added 1 commit 2026-03-01 22:44:03 +09:00
test: cover no-readonly-change early return in governance validator
All checks were successful
Gitea CI / test (push) Successful in 33s
Details
Gitea CI / test (pull_request) Successful in 33s
Details
c431d82c0d
agentson commented 2026-03-01 22:44:11 +09:00
Author
Collaborator
Copy Link

리뷰 minor 항목 반영 완료했습니다.

  • 추가 테스트: test_validate_read_only_approval_skips_when_no_readonly_file_changed
  • 목적: changed_files에 READ-ONLY 파일이 없을 때 early return 경로가 errors/warnings 없이 통과함을 명시 검증

검증:

  • ruff check tests/test_validate_governance_assets.py scripts/validate_governance_assets.py
  • pytest -q tests/test_validate_governance_assets.py (12 passed)
  • python3 scripts/validate_governance_assets.py origin/feature/v3-session-policy-stream...HEAD

반영 커밋: c431d82

리뷰 minor 항목 반영 완료했습니다. - 추가 테스트: test_validate_read_only_approval_skips_when_no_readonly_file_changed - 목적: changed_files에 READ-ONLY 파일이 없을 때 early return 경로가 errors/warnings 없이 통과함을 명시 검증 검증: - ruff check tests/test_validate_governance_assets.py scripts/validate_governance_assets.py - pytest -q tests/test_validate_governance_assets.py (12 passed) - python3 scripts/validate_governance_assets.py origin/feature/v3-session-policy-stream...HEAD 반영 커밋: c431d82
agentson commented 2026-03-01 22:45:15 +09:00
Author
Collaborator
Copy Link

팔로업 리뷰 — commit c431d82 반영 확인

✅ 지적사항 반영 확인

🔵 non-READ-ONLY 파일 early return 테스트 → 추가됨

def test_validate_read_only_approval_skips_when_no_readonly_file_changed() -> None:
    changed_files = ["src/main.py"]
    ...
    assert errors == []
    assert warnings == []

12 passed 직접 확인.

잔여 항목 현황

항목 상태
🔵 early return 테스트 없음 ✅ 완료
🟡 order_policy.py 정책 결정 이번 PR 범위 외 — 별도 처리
🔵 push 이벤트 우회 가능성 브랜치 보호로 완화, 이번 PR 범위 외

✅ 최종 판정

이전 리뷰 지적사항 중 이번 PR 범위 내 항목은 모두 처리됐다. Merge 가능 상태.

## 팔로업 리뷰 — commit c431d82 반영 확인 ### ✅ 지적사항 반영 확인 **🔵 non-READ-ONLY 파일 early return 테스트 → 추가됨** ```python def test_validate_read_only_approval_skips_when_no_readonly_file_changed() -> None: changed_files = ["src/main.py"] ... assert errors == [] assert warnings == [] ``` 12 passed 직접 확인. --- ### 잔여 항목 현황 | 항목 | 상태 | |------|------| | 🔵 early return 테스트 없음 | ✅ 완료 | | 🟡 `order_policy.py` 정책 결정 | 이번 PR 범위 외 — 별도 처리 | | 🔵 push 이벤트 우회 가능성 | 브랜치 보호로 완화, 이번 PR 범위 외 | ### ✅ 최종 판정 이전 리뷰 지적사항 중 이번 PR 범위 내 항목은 모두 처리됐다. **Merge 가능** 상태.
jihoson merged commit 18931c8b58 into feature/v3-session-policy-stream 2026-03-01 22:46:53 +09:00
jihoson deleted branch feature/issue-356-readonly-approval 2026-03-01 22:46:53 +09:00
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
Clear labels
bug
Something is not working
 duplicate
This issue or pull request already exists
 enhancement
New feature
 help wanted
Need some help
 invalid
Something is wrong
 question
More information is needed
 wontfix
This won't be fixed
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
agentson jihoson
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: jihoson/The-Ouroboros#361
Delete Branch "feature/issue-356-readonly-approval"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?