Repository navigation
refactor: make the container repository-wide and drop the shortcut scripts - #52
Merged
Merged
Conversation
…ripts The dev container lived under .devcontainer/sre-agent-event-lab/, so Codespaces offered it as one choice among several and VS Code ignored it. Move it to .devcontainer/devcontainer.json, the path both read by default from anywhere in the repository, and take the lab's path out of it: a container that knows one lab's directory has to be edited for the next one. The tools it installs, and the commands to install them without a container, now live in one document. Delete the scripts that ran the lab for the operator. A one-command shortcut is the fastest way to finish this lab having learned nothing -- the point is to make the Azure change and watch what the Agent does with it. The guides already carry every step, so lab.sh, run-scenario.sh, capture-scenario.sh, baseline.sh and doctor.sh go, along with deploy.sh and cleanup.sh, which only forwarded to azd. What survives is what nothing else can cover: the four hooks azure.yaml invokes, the environment uv needs, the values exported once per shell, the library those read, and the evidence collection that is eight queries in a row. The baseline gate moves into guides/01-agent-setup.md as the commands it always was, which needed two more azd outputs exported. Refusal messages in lab_state.py and score.py named lab.sh; they now name the guide that walks the step, and a test keeps anything from pointing an operator at a script that is gone. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
![my-reviewer-agent[bot]](https://avatars.githubusercontent.com/in/4343504?s=60&v=4){kind=small}
There was a problem hiding this comment.
부분 범위 리뷰 (Partial scope)
범위 제한 고지: 이 PR의 원본 diff는 총 5733줄(original_diff_lines)이며, 크기 제한 때문에 전체를 검토하지 못했습니다. 따라서 이번 리뷰는 완전한 리뷰가 아니며, 아래 파일들만 실제로 읽고 판단했습니다.
검토한 파일(included_files, 19개)
.devcontainer/devcontainer.jsonmonitor/sre-agent-event-lab/infra/main.bicepmonitor/sre-agent-event-lab/scripts/baseline.sh,capture-scenario.sh,cleanup-external.sh,cleanup.sh,common.sh,deploy.sh,doctor.sh,lab-env.sh,lab.sh,lab_state.py,run-scenario.sh,score.py,setup-venv.shmonitor/sre-agent-event-lab/scripts/tests/doctor_harness.py,test_cleanup_external.py,test_common.py,test_lab_env.py
검토하지 못한 파일(omitted_files, 19개)
.devcontainer/README.md,monitor/sre-agent-event-lab/README.mdmonitor/sre-agent-event-lab/guides/01-agent-setup.md,02-scenario-s1.md,03-scenario-s2.md,04-scenario-s3.md,05-results.mdmonitor/sre-agent-event-lab/scripts/tests/azd_fake.py,cleanup_harness.py,lab_script_harness.py,test_baseline.py,test_doctor.py,test_lab_cli.py,test_lab_guides.py,test_lab_scripts.py,test_lab_state.py,test_query_evidence.py,test_repo_devcontainer.py,test_score.py
특히 가이드 문서 5종과 삭제된 스크립트를 대상으로 하던 테스트 모듈(test_doctor.py, test_baseline.py, test_lab_cli.py, test_lab_scripts.py 등)이 범위 밖이므로, "삭제된 스크립트를 참조하는 테스트/문서가 남아 있는가"라는 가장 중요한 정합성 질문은 이번 리뷰로 답할 수 없습니다. 그 부분은 CI의 pytest 전체 실행 결과로 확인해 주세요.
총평
방향 자체는 일관됩니다. lab.sh/doctor.sh/baseline.sh/run-scenario.sh/capture-scenario.sh/cleanup.sh/deploy.sh를 제거하고 수동 가이드 중심으로 전환하면서, 남은 코드의 안내 문구를 스크립트 이름 대신 가이드 문서 경로로 바꾼 점, scenario_guide()로 매핑을 한 곳에 모은 점, test_common.py에 "남은 셸 스크립트 집합 고정"과 "사라진 스크립트를 가리키는 문구 금지" 검사를 새로 넣은 점은 회귀 방지 장치로 적절합니다. main.bicep의 AZURE_WORKSPACE_CUSTOMER_ID 출력 추가와 lab-env.sh의 WORKSPACE_CUSTOMER_ID/TELEMETRY_SERVICE_NAME 바인딩 추가도, 수동 실행에서 Log Analytics 질의를 직접 하려면 반드시 필요한 값이라 삭제된 자동화의 공백을 정확히 메웁니다. 차단성 결함은 검토한 범위에서 발견하지 못했고, 아래 3건은 모두 경고/제안 수준입니다.
Contributor
There was a problem hiding this comment.
Pull request overview
Refactors the SRE Agent Event Lab to use a single repository-wide Dev Container configuration and removes shortcut/dispatch shell scripts in favor of running the lab steps explicitly via guides and direct tool invocations, while updating tests and operator-facing messages to avoid pointing at deleted scripts.
Changes:
- Moves/standardizes Dev Container config to
.devcontainer/devcontainer.jsonand centralizes toolchain docs in.devcontainer/README.md. - Deletes shortcut entrypoint scripts (e.g.,
lab.sh,run-scenario.sh,baseline.sh,doctor.sh, etc.) and rewrites guides/tests to reflect the manual workflow. - Updates refusal/error messaging (notably
lab_state.pyandscore.py) to point operators to the correct guide documents; adds/adjusts contract/execution tests accordingly.
Reviewed changes
Copilot reviewed 38 out of 38 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| monitor/sre-agent-event-lab/scripts/tests/test_score.py | Updates stderr expectation to reference scenario guide instead of removed lab.sh. |
| monitor/sre-agent-event-lab/scripts/tests/test_repo_devcontainer.py | Adds contract tests enforcing a single repo-wide devcontainer and shared toolchain doc. |
| monitor/sre-agent-event-lab/scripts/tests/test_query_evidence.py | Adds execution tests for query-evidence.sh using the harness. |
| monitor/sre-agent-event-lab/scripts/tests/test_lab_state.py | Updates tests and docstrings to remove references to deleted scripts and point to guides. |
| monitor/sre-agent-event-lab/scripts/tests/test_lab_scripts.py | Removes execution tests for deleted shell entrypoints (run-scenario.sh, capture-scenario.sh, etc.). |
| monitor/sre-agent-event-lab/scripts/tests/test_lab_guides.py | Updates doc contract tests to enforce no shortcut sections/scripts and validate new command structure. |
| monitor/sre-agent-event-lab/scripts/tests/test_lab_env.py | Removes lab-specific devcontainer assertions and expands env bindings (workspace/customer + telemetry name). |
| monitor/sre-agent-event-lab/scripts/tests/test_lab_cli.py | Removes behavioral tests for deleted lab.sh dispatcher. |
| monitor/sre-agent-event-lab/scripts/tests/test_doctor.py | Removes behavioral tests for deleted doctor.sh. |
| monitor/sre-agent-event-lab/scripts/tests/test_common.py | Updates shared test expectations for the reduced shell-script set and adds guardrails against stale .sh references. |
| monitor/sre-agent-event-lab/scripts/tests/test_cleanup_external.py | Updates README contract expectation to new acknowledge command naming. |
| monitor/sre-agent-event-lab/scripts/tests/test_baseline.py | Removes behavioral tests for deleted baseline.sh. |
| monitor/sre-agent-event-lab/scripts/tests/lab_script_harness.py | Adjusts harness documentation/comments to reflect manual scenario execution. |
| monitor/sre-agent-event-lab/scripts/tests/doctor_harness.py | Removes fake-CLI harness dedicated to deleted doctor.sh/baseline.sh/lab.sh. |
| monitor/sre-agent-event-lab/scripts/tests/cleanup_harness.py | Updates harness docstring to the new lab_state.py acknowledge-agent flow. |
| monitor/sre-agent-event-lab/scripts/tests/azd_fake.py | Updates module docs to reflect current callers (common.sh and its callers). |
| monitor/sre-agent-event-lab/scripts/setup-venv.sh | Updates comments to reflect manual capture/notification steps rather than removed script names. |
| monitor/sre-agent-event-lab/scripts/score.py | Updates operator guidance text to reference scenario guides instead of removed lab.sh commands. |
| monitor/sre-agent-event-lab/scripts/run-scenario.sh | Deletes scenario automation script (manual workflow replaces it). |
| monitor/sre-agent-event-lab/scripts/lab.sh | Deletes the dispatcher script and its usage surface. |
| monitor/sre-agent-event-lab/scripts/lab-env.sh | Adds exports for workspace customer ID and telemetry service name. |
| monitor/sre-agent-event-lab/scripts/lab_state.py | Updates remedies/refusals to reference guide docs and new baseline/acknowledge flow; adds scenario-guide mapping. |
| monitor/sre-agent-event-lab/scripts/doctor.sh | Deletes the diagnostic script. |
| monitor/sre-agent-event-lab/scripts/deploy.sh | Deletes compatibility wrapper that forwarded to azd up. |
| monitor/sre-agent-event-lab/scripts/common.sh | Updates comments/docs to remove references to deleted scripts while keeping shared helpers. |
| monitor/sre-agent-event-lab/scripts/cleanup.sh | Deletes compatibility wrapper cleanup script (manual recovery instructions remain in docs). |
| monitor/sre-agent-event-lab/scripts/cleanup-external.sh | Updates error guidance to point to lab_state.py acknowledge-agent. |
| monitor/sre-agent-event-lab/scripts/capture-scenario.sh | Deletes capture automation script (manual workflow replaces it). |
| monitor/sre-agent-event-lab/scripts/baseline.sh | Deletes baseline automation script (manual workflow replaces it). |
| monitor/sre-agent-event-lab/README.md | Rewrites operator instructions to use repo-wide container and manual baseline/scenario flow; removes shortcut usage. |
| monitor/sre-agent-event-lab/infra/main.bicep | Adds AZURE_WORKSPACE_CUSTOMER_ID output for scripts/guides that need it. |
| monitor/sre-agent-event-lab/guides/05-results.md | Updates scoring command to call scripts/score.py directly. |
| monitor/sre-agent-event-lab/guides/04-scenario-s3.md | Removes shortcut section and updates rerun/remedy wording to manual begin-run flow. |
| monitor/sre-agent-event-lab/guides/03-scenario-s2.md | Removes shortcut section and updates rerun/remedy wording to manual begin-run flow. |
| monitor/sre-agent-event-lab/guides/02-scenario-s1.md | Removes shortcut section; strengthens guidance around running/failed state handling and manual gating. |
| monitor/sre-agent-event-lab/guides/01-agent-setup.md | Moves baseline “gate” into the guide as manual steps and adds required exports/queries. |
| .devcontainer/README.md | Adds centralized toolchain list + local install instructions for all labs. |
| .devcontainer/devcontainer.json | Makes repo-wide container the default, renames it, and removes lab-coupled lifecycle hooks. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…te import The review was right that nothing told an operator what creates app/.venv now that the container no longer does. Checking it turned up a worse problem: the failure row I had written blamed loadgen.py, which imports only the standard library and never needed the venv. Pillow does, in the capture step, so the note now sits where that step begins and says what creates it. scenario_guide() fell back to 'the scenario guide', a direction rather than an answer; it now names the directory, and a test pins the mapping to SCENARIOS and to files that exist. config() asserts before parsing so a missing devcontainer fails with the intended message. Duplicate import removed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 38 out of 38 changed files in this pull request and generated no new comments.
Suppressed comments (2)
monitor/sre-agent-event-lab/README.md:121
- This snippet uses
${EVIDENCE_DIR}without defining it in this section. Copy/pasting will fail (or record an unrelated directory) unless the reader already knows to reuse the baseline step’s evidence directory from guides/01-agent-setup.md. Add an inline note so${EVIDENCE_DIR}is clearly sourced from the baseline step.
python3 scripts/lab_state.py mark baseline_passed --evidence-dir "${EVIDENCE_DIR}"
python3 scripts/lab_state.py acknowledge-agent
monitor/sre-agent-event-lab/scripts/score.py:28
- The docstring says the TSV output is "the same machine-readable shape the evidence files carry", but the evidence artifacts here are JSON (e.g., scorecard.json / conclusion-review.json). This wording is misleading; it would be clearer to state that scorecard.json is the machine-readable output and the TSV table is for human review/log scraping.
Output is `evidence/scorecard.json` plus a tab-separated table
(`SCENARIO<TAB>CRITERION<TAB>STATUS<TAB>POINTS<TAB>DETAIL`), the same
machine-readable shape the evidence files carry.
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.
The container
It lived at
.devcontainer/sre-agent-event-lab/devcontainer.json, so Codespaces offered it as one choice among several and VS Code's "Reopen in Container" ignored it. It now sits at.devcontainer/devcontainer.json, the path both read by default from anywhere in the repository.Its
postCreateCommandran one lab'ssetup-venv.sh. That coupling is gone: a container that knows a lab's directory has to be edited for the next lab, and one lab's setup failure would break container creation for everyone. Nothing is lost —setup-venv.shalready runs inazd up's postprovision hook..devcontainer/README.mdnow owns the tool list and the local install commands, so labs link to it instead of each carrying a copy.The shortcuts
A one-command shortcut is the fastest way to finish this lab having learned nothing. Deleted:
lab.sh,run-scenario.sh,capture-scenario.sh,baseline.sh,doctor.sh, plusdeploy.shandcleanup.sh, which only forwarded toazd.What survives is what nothing else can cover: the four hooks
azure.yamlinvokes, the environmentuvneeds, the values exported once per shell, the library those read, andquery-evidence.sh— eight queries in a row, not a step being skipped.The baseline gate that
baseline.shenforced moves intoguides/01-agent-setup.mdas the commands it always was. That needed two azd outputs (AZURE_WORKSPACE_CUSTOMER_ID,AZURE_TELEMETRY_SERVICE_NAME) exported.A bug this found
lab_state.pyandscore.pytold operators toRun: lab.sh run s1— a script that no longer exists, printed exactly when someone is already stuck. They now name the guide that walks the step, andtest_nothing_points_an_operator_at_a_script_that_is_gonecovers scripts, tools and documents. It caught three more stale references while I wrote it.Verification
pytest scripts infra -q— 492 passed (was 616; the deleted scripts took their tests with them, and the suite went from ~8 min to under 2)pytest app -q— 10 passedbaseline_passed, and every refusal now names a document that exists-4,289 lines, +627.