Skip to content

docs: Document deployment validation check [doc-updater] - #663

Merged
jwbron merged 3 commits into
mainfrom
egg/doc-update-deployment-validation
Feb 14, 2026
Merged

docs: Document deployment validation check [doc-updater]#663
jwbron merged 3 commits into
mainfrom
egg/doc-update-deployment-validation

Conversation

@james-in-a-box

Copy link
Copy Markdown
Contributor

docs: Document deployment validation check system

Update documentation to reflect the deployment validation feature added in #653.

Changes:

  • Add deployment_check.py, deployment.py, and devserver.py to file listings in docs/development/STRUCTURE.md
  • Document deployment validation check in docs/architecture/README.md and docs/guides/sdlc-pipeline.md
  • Add comprehensive "Devserver Management (Deployment Validation)" section to docs/architecture/orchestrator.md covering:
    • Architecture and responsibilities split between orchestrator and sandbox
    • Security properties (network isolation, resource limits, credential safety)
    • Configuration format (.egg/deployment.yml)
    • API flow for start/poll/teardown lifecycle
  • Update phase default checks to include deployment validation (optional, with 1 retry)

The deployment validation check enables opt-in testing of agent-modified code against locally running devserver stacks (Docker-in-Docker) during the implement phase. Target repositories configure this via .egg/deployment.yml, which specifies their docker-compose setup, service-to-source mappings, health endpoints, and validation tests.

Triggered by: #653

Authored-by: egg

Update documentation to reflect deployment validation feature added in #653:
- Add deployment_check.py, deployment.py, and devserver.py to STRUCTURE.md
- Document deployment validation in architecture README and SDLC guide
- Add comprehensive devserver management section to orchestrator architecture
- Update phase default checks to include deployment validation (optional)

Authored-by: egg

@egg-reviewer egg-reviewer Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No agent-mode design concerns.

This is a docs-only PR documenting the deployment validation feature from #653. The underlying design aligns well with agent-mode principles:

  • Infrastructure, not pre-fetching: The orchestrator manages DinD lifecycle (network isolation, resource limits, container orchestration) — this is legitimate infrastructure work that belongs outside the agent sandbox, not context pre-fetching.
  • Declarative config: .egg/deployment.yml specifies what to validate (services, health endpoints, test expectations), not how to validate it.
  • Security is sandbox-enforced: Network isolation, resource limits, PID caps, credential scanning, and hard timeouts are all technically enforced — no prompt-level security instructions.

— Authored by egg

@james-in-a-box

This comment has been minimized.

@egg-reviewer egg-reviewer Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Documentation Review — Deployment Validation Check

This is a docs-only PR, so the bar is accuracy against the implementation it documents. I found several factual discrepancies between this documentation and the actual source code.

Issues

1. API endpoint paths are wrong (orchestrator.md lines 212-214)

The docs list:

POST /pipelines/{id}/checks/devserver/start
GET /pipelines/{id}/checks/devserver/status
POST /pipelines/{id}/checks/devserver/teardown

The actual routes in orchestrator/routes/checks.py are:

POST /pipelines/{id}/deployment-check/start
GET /pipelines/{id}/deployment-check/status
POST /pipelines/{id}/deployment-check/teardown

The path segment is deployment-check, not checks/devserver. This same error appears in the "API Flow" section (lines 289-301) where the full /api/v1/ paths are shown — those are also wrong.

2. YAML config example does not match the actual schema (orchestrator.md lines 270-283, sdlc-pipeline.md lines 838-848)

The example YAML uses fields that don't exist in DeploymentConfig and omits fields that do:

Docs example Actual schema (deployment.py)
version: "1" No version field on DeploymentConfig
services[].health_endpoint (full URL) No such field on ServiceMapping
tests (top-level) validation_tests (top-level)
tests[].name validation_tests[].description
tests[].url (full URL) validation_tests[].path (path only, e.g. /users)
(missing) validation_tests[].service (required field)
(missing) health_endpoints (top-level dict mapping service name → path)

The documented YAML would fail Pydantic validation if a user tried to use it. A corrected example:

compose_file: "docker-compose.yml"
services:
  - source_dir: "services/api"
    service_name: "api"
    container_mount_path: "/app"
health_endpoints:
  api: "/health"
validation_tests:
  - service: "api"
    path: "/users"
    method: "GET"
    expected_status: 200
    description: "API smoke test"

3. Credential safety docs say *_TOKEN is scanned — regex is more specific (orchestrator.md line 263)

The docs state: Environment variables scanned for suspicious patterns (AWS_*, *_SECRET_KEY, *_TOKEN, etc.)

The actual regex in deployment.py line 18-22 matches .*_TOKEN$, which is correct, but the docs also imply *_SECRET_KEY is matched while the regex actually matches .*_SECRET_KEY$ AND .*_API_KEY$ AND .*_ACCESS_KEY$ AND .*_PASSWORD$ AND .*_CREDENTIALS$. The docs omit *_API_KEY, *_ACCESS_KEY, *_PASSWORD, and *_CREDENTIALS from the "etc." This is minor but the list could be more complete since users rely on it to understand what's blocked.

4. STRUCTURE.md lists sandbox/scripts/checks/deployment_check.py

The file tree in STRUCTURE.md (line 306) places deployment_check.py under sandbox/scripts/checks/. The actual file is at .github/scripts/checks/deployment_check.py. The STRUCTURE.md file tree already shows the checks directory under .github/scripts/checks/ (line 295+), so the new entry was added in the wrong location in the tree.

Non-blocking notes

  • The sdlc-pipeline.md check table (line 800) lists the check command as check-deployment, which aligns with phase_defaults.py. Good.
  • Resource limits (CPU 1.0, Memory 512m, PIDs 256, 5-min timeout) match constants.py. Good.
  • The implement phase defaults update (optional, 1 retry) matches phase_defaults.py. Good.

Verdict

The API paths and the YAML config example are factually wrong and would mislead anyone trying to use this documentation. Request changes to fix the two substantive issues (API paths and YAML schema mismatch). The STRUCTURE.md file tree placement should also be corrected.

— Authored by egg

@james-in-a-box

This comment has been minimized.

@james-in-a-box

This comment has been minimized.

- Fix API endpoint paths: checks/devserver → deployment-check
  to match actual routes in orchestrator/routes/checks.py
- Fix YAML config examples to match DeploymentConfig schema:
  remove version field, rename tests→validation_tests,
  rename name→description, url→path, add service field,
  add health_endpoints top-level dict, remove health_endpoint
  from services
- Expand credential safety pattern list to include all patterns
  from the actual regex: *_API_KEY, *_ACCESS_KEY, *_PASSWORD,
  *_CREDENTIALS, plus GCP_*, AZURE_*, GOOGLE_CLOUD_* prefixes

@james-in-a-box james-in-a-box Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the thorough review. All substantive issues have been addressed in 67644c1:

1. API endpoint paths — Fixed. Changed checks/devserverdeployment-check in both the endpoint listing (lines 212-214) and the API Flow section (lines 289-301) of orchestrator.md.

2. YAML config examples — Fixed in both orchestrator.md and sdlc-pipeline.md. Corrected to match the actual DeploymentConfig schema: removed version field, renamed testsvalidation_tests, namedescription, urlpath, added service field, added health_endpoints top-level dict, removed health_endpoint from service entries.

3. Credential safety patterns — Fixed. Expanded the pattern list in both docs to include all patterns from the actual regex: prefix matches (AWS_*, GCP_*, AZURE_*, GOOGLE_CLOUD_*) and suffix matches (*_SECRET_KEY, *_API_KEY, *_ACCESS_KEY, *_TOKEN, *_PASSWORD, *_CREDENTIALS).

4. STRUCTURE.md file tree placement — No change needed. The entry at line 306 is already correctly placed under .github/scripts/checks/ (see lines 299-306 in the file tree). There is no sandbox/scripts/checks/ section in STRUCTURE.md.

— Authored by egg

@james-in-a-box

This comment has been minimized.

@egg-reviewer egg-reviewer Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No agent-mode design concerns in the delta.

The changes since 4c25751 are documentation accuracy fixes (API paths, YAML schema, credential patterns) — no changes to agent interaction design.

— Authored by egg

@james-in-a-box

This comment has been minimized.

@egg-reviewer egg-reviewer Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Re-Review — Deployment Validation Documentation (67644c1)

Verified the delta from 4c2575167644c1 against the source code.

Previous Issues — Verification

All four issues from the prior review have been addressed:

1. API endpoint paths — Fixed correctly. Paths now read deployment-check/start, deployment-check/status, deployment-check/teardown in both the endpoint listing and API Flow section. Verified against orchestrator/routes/checks.py lines 94, 230, 256.

2. YAML config examples — Fixed correctly in both orchestrator.md and sdlc-pipeline.md. The examples now match the DeploymentConfig Pydantic schema: version field removed, testsvalidation_tests, namedescription, urlpath, health_endpoint replaced with top-level health_endpoints dict, service field added to test entries. All required fields are present; omitted fields (startup_timeout_seconds, image_registry, expected_body_contains) all have defaults — acceptable.

3. Credential safety patterns — Fixed correctly. Both docs now list the complete set of prefix and suffix patterns matching the regex in deployment.py lines 18-23: prefixes AWS_*, GCP_*, AZURE_*, GOOGLE_CLOUD_* and suffixes *_SECRET_KEY, *_API_KEY, *_ACCESS_KEY, *_TOKEN, *_PASSWORD, *_CREDENTIALS.

4. STRUCTURE.md placement — The author's rebuttal was correct: deployment_check.py was already listed under .github/scripts/checks/ at line 306, not under sandbox/scripts/checks/. My prior review was wrong on this point.

New Changes — Assessment

No new issues found. The delta is clean — it's a focused fix addressing exactly the four items raised, with no unrelated changes or regressions introduced.

Non-blocking observation

The documented check ordering in sdlc-pipeline.md lists deployment validation before auto-fixer, while phase_defaults.py has check-fixer (line 67) before check-deployment (line 75) in the _IMPLEMENT_CHECKS list. Since checks are dispatched individually rather than iterated as a batch, this ordering difference has no functional impact — but if the list order ever becomes meaningful (e.g., for prompt rendering), the docs and code would disagree.

— Authored by egg

@james-in-a-box

This comment has been minimized.

@james-in-a-box

This comment has been minimized.

The implement phase check list in sdlc-pipeline.md and README.md listed
deployment validation before auto-fixer, but phase_defaults.py defines
check-fixer before check-deployment. Reorder docs to match the code.

Authored-by: egg
@james-in-a-box

Copy link
Copy Markdown
Contributor Author

Addressed the non-blocking observation from the re-review in 1712bfa:

Check ordering — Fixed. The implement phase check lists in sdlc-pipeline.md and README.md now match the order in phase_defaults.py: auto-fixer before deployment validation.

— Authored by egg

@james-in-a-box

This comment has been minimized.

@egg-reviewer egg-reviewer Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No agent-mode design concerns in the delta.

The change since 67644c1 (1712bfa) reorders the implement-phase check listings in README.md and sdlc-pipeline.md to match phase_defaults.py — purely a docs accuracy fix with no agent interaction implications.

— Authored by egg

@james-in-a-box

This comment has been minimized.

@egg-reviewer egg-reviewer Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Re-Review — Deployment Validation Documentation (1712bfa)

The branch was rebased since the last review (67644c11712bfa), squashing all changes into a single commit on top of origin/main. Reviewed the full PR diff against source code.

Previous Issues — All Resolved

All four issues from the initial review and the non-blocking ordering suggestion from the re-review have been addressed:

  1. API endpoint paths — Correct. deployment-check/start, deployment-check/status, deployment-check/teardown in both the endpoint listing and API Flow section. Verified against orchestrator/routes/checks.py lines 94, 230, 256.

  2. YAML config examples — Correct in both orchestrator.md and sdlc-pipeline.md. Schema matches DeploymentConfig in deployment.py: compose_file, services (with source_dir/service_name/container_mount_path), health_endpoints (top-level dict), validation_tests (with service/path/method/expected_status/description). All required fields present; omitted fields have defaults.

  3. Credential safety patterns — Correct. Complete set of prefix matches (AWS_*, GCP_*, AZURE_*, GOOGLE_CLOUD_*) and suffix matches (*_SECRET_KEY, *_API_KEY, *_ACCESS_KEY, *_TOKEN, *_PASSWORD, *_CREDENTIALS). Verified against regex at deployment.py lines 18-23.

  4. STRUCTURE.md placement — Correct. All new file entries (devserver.py, routes/checks.py, deployment.py, deployment_check.py, test files) are in the right locations in the file tree, verified against actual file system.

  5. Check ordering (non-blocking from re-review) — Fixed. The implement phase default list in sdlc-pipeline.md (lines 817-818) and README.md (line 154) both list auto-fixer before deployment validation, matching phase_defaults.py lines 67-81.

New Changes — Assessment

No issues found. The delta since the last review is the check ordering fix, which is correct. The check reference table in sdlc-pipeline.md (lines 800-801) lists deployment before auto-fixer, but this table is an inventory of available checks, not an execution order — acceptable.

Verification Summary

  • API routes: match source ✓
  • YAML schema: match Pydantic models ✓
  • Credential patterns: match regex ✓
  • Resource limits (CPU 1.0, Memory 512m, PIDs 256, 5-min timeout): match constants.py ✓
  • Network naming (egg-check-{pipeline_id}): match devserver.py ✓
  • Phase defaults ordering: match phase_defaults.py ✓
  • STRUCTURE.md file entries: all files exist on disk ✓

No blocking issues. Documentation is accurate against the implementation.

— Authored by egg

@james-in-a-box

This comment has been minimized.

@james-in-a-box

Copy link
Copy Markdown
Contributor Author
egg is addressing review feedback...

@james-in-a-box

Copy link
Copy Markdown
Contributor Author

All review feedback has been addressed — confirmed by the final re-review at 1712bfa which verified all items against source code. No remaining action items.

— Authored by egg

@james-in-a-box

Copy link
Copy Markdown
Contributor Author

egg feedback addressed. View run logs

10 previous review(s) hidden.

@jwbron
jwbron merged commit 5b2300e into main Feb 14, 2026
29 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.

1 participant