Upgrade Olympus mechanism to v0.4.0 (full post-rename migration: .olympus.json + olympus_ref)#167
Conversation
There was a problem hiding this comment.
Now I have a complete picture. Here is my review.
Summary
PR #167 is two commits: (1) a clean rename/version-bump of the Olympus CI agent framework from v0.2.0/v0.3.1 to v0.4.0, including policy file rename, workflow pin updates, comment scrubbing, and a migration lint script; (2) an auto-commit fallback that accidentally checked a gitlink (_agentops) for the internal Netis/agent-ops repo into the tree. The first commit is solid. The second commit is a hard block — it leaks an internal submodule reference and the working tree contains a persisted GitHub token. REQUEST_CHANGES — revert/drop the second commit and purge the working-tree token.
Blocking
-
_agentops(gitlink, commit ba9cdfe) — The second commit adds a gitlink (mode160000) for the internalNetis/agent-opsrepository. Two concrete problems:- Public clone broken. There is no
.gitmodulesfile, so external contributors (and GitHub Actions on hosted runners) can't resolve the submodule URL.git checkoutof this commit will produce a dangling, unresolvable gitlink at_agentops/. - Credential leakage. The working tree's
_agentops/.git/config(line 12 of that file) contains abasicAuthorization header with a base64-encodedghp_*GitHub token in theextraheader. The committed gitlink itself doesn't include.git/config, but the working tree on the CI/agent runner currently holds this credential on disk and it is trivially recoverable.
Fix: Drop commit
ba9cdfe(the second commit) entirely. Thenrm -rf _agentopsand ensure.gitignorecatches it (or better, add_agentops/to.gitignoreso the agent worktree can't accidentally commit it again). - Public clone broken. There is no
-
_agentopsnot gitignored —.gitignorehas no entry for_agentops/(or_agentops). The directory was created by the agent loop as a transient worktree/clone; without a gitignore entry, future agent runs will re-introduce this same class of dirty-tree auto-commit. Add_agentops/to.gitignore.
Suggestions
-
scripts/lint/check-olympus-migration.sh— The migration lint script is created but never wired intoci.yml. As-is, it will silently bitrot — nobody will run it, and a future workflow that reverts toagent_ops_refwon't be caught. Add a CI step (e.g.,bash scripts/lint/check-olympus-migration.sh) toci.ymlso the migration invariant is enforced at PR time. (Alternative: if this is truly a one-shot validation, move it toscripts/withoutlint/and add a comment that it's historical; but in-tree lints should run in CI.) -
scripts/lint/check-olympus-migration.sh:66— The regexNetis/olympus/.*@v0\.4\.0hardcodes the version. When the next version bump arrives (v0.5.0), this lint will both fail and need manual editing, creating busywork. Consider either: (a) extracting the expected version from.olympus.jsonor aVERSION-like file, or (b) relaxing this check to "all wrappers agree on the same version" rather than "must be v0.4.0 specifically."
Verified
- Workflow consistency: All 5 wrapper workflows (
guard.yml,issue-implement.yml,issue-triage.yml,pr-review.yml,pr-revise.yml) useolympus_ref: v0.4.0andNetis/olympus/...@v0.4.0— consistent with each other and the lint script's expectations. pr-review.ymlversion bump: Correctly bumped from@v0.3.1(it was ahead of the other 4 which were at@v0.2.0) to@v0.4.0— all 5 now aligned.- Policy file rename:
.agent-ops.json→.olympus.jsonis clean;$schemaURL correctly points toNetis/olympus/main/schema/olympus.schema.json. - No stale
agent_ops_refreferences: Grep across all 5 workflows confirms zero remainingagent_ops_refstrings. - Comment scrub: All YAML comments in the 5 workflows reference "olympus" not "agent-ops" — consistent rename.
🤖 Reviewed by vivi • workflow run
- Rename .agent-ops.json to .olympus.json and update $schema URL - Bump all 5 wrapper workflows to @v0.4.0 with olympus_ref input - Update all stale comment references from agent-ops to olympus - Add migration validation lint script and wire it into ci.yml; the expected version is derived from the wrappers (not hardcoded), so it survives the next bump - Gitignore the transient _agentops/ agent-loop clone so a stray gitlink — and the runner credential in its .git/config — can never be committed again
ba9cdfe to
259784a
Compare
|
Addressed the review. Force-pushed; the PR is now a single clean commit. Blocking — fixed
Suggestions — done
The first commit's content is unchanged apart from the three items above. |
There was a problem hiding this comment.
I've completed my review. This is a clean, mechanical migration PR.
Summary
This PR renames the agent-ops mechanism to olympus across all 5 workflow wrappers, bumps them uniformly to @v0.4.0 (previously pr-review.yml was ahead at v0.3.1 while others were at v0.2.0), adds a check-olympus-migration.sh lint that prevents future version drift or partial reversion, and gitignores the _agentops/ transient clone directory. The migration is thorough — no stale Netis/agent-ops references remain anywhere in the tree, and the self-validating lint script is wired into ci.yml so a future edit can't silently regress. APPROVE.
Verified
- No stale
Netis/agent-opsreferences remain in any tracked file; only intentional stale-name checks exist in the migration lint script itself. - All 5 wrapper workflows agree on
uses: Netis/olympus/...@v0.4.0+olympus_ref: v0.4.0. .agent-ops.jsonis gone from the working tree;.olympus.jsonexists with the correct$schemaURL._agentops/transient clone directory is properly gitignored (working tree clean).- No references to
.agent-ops.jsonoragent-opsinCLAUDE.md,docs/,project.yaml, orscripts/(outside the migration lint itself). check-secrets.shdoes not reference the old file name — no missed update needed.
🤖 Reviewed by vivi • workflow run
Olympus's design names its roles after Greek deities (its own dogfood config uses themis/hephaestus/argus). heron carried over off-theme early names through the #167 migration. Align them: - review_bot_login: vivi -> themis (justice/order — the reviewer) - dev_agent_name: wiwi -> hephaestus (the forge — the implementer) - observer label: mara -> argus (the all-seeing watchman) review_bot_login is a body-footer marker (reviews post as github-actions), so auto-merge/revise key off the config value consistently — self-contained rename. dev_agent_name is the commit-author display name. The observer label is dormant (heron wires no observe.yml). Validates against the v0.4.0 olympus schema.
…nt names (#173) * chore(olympus): bump agent model off retired claude-3-5-sonnet-20241022 claude-3-5-sonnet-20241022 was retired 2025-10-28; the LiteLLM gateway would 404 on it. Move wiwi/vivi to claude-sonnet-4-6, the documented drop-in for that retired ID — same Sonnet tier, current generation. Validates against the v0.4.0 olympus schema. * chore(olympus): align agent names to the Greek-deity convention Olympus's design names its roles after Greek deities (its own dogfood config uses themis/hephaestus/argus). heron carried over off-theme early names through the #167 migration. Align them: - review_bot_login: vivi -> themis (justice/order — the reviewer) - dev_agent_name: wiwi -> hephaestus (the forge — the implementer) - observer label: mara -> argus (the all-seeing watchman) review_bot_login is a body-footer marker (reviews post as github-actions), so auto-merge/revise key off the config value consistently — self-contained rename. dev_agent_name is the commit-author display name. The observer label is dormant (heron wires no observe.yml). Validates against the v0.4.0 olympus schema. --------- Co-authored-by: Vader Yang <vader.yang@netis.com>
Summary
Upgrade the Olympus mechanism from the minimal-repoint state (split
@v0.2.0/@v0.3.1tags) tov0.4.0, completing the post-rename migration.Changes
.agent-ops.json→.olympus.jsonand updated$schemato point toNetis/olympus/main/schema/olympus.schema.json. All existing policy values carry over unchanged.guard.yml,issue-implement.yml,issue-triage.yml,pr-revise.yml,pr-review.yml) bumped to@v0.4.0with the input renamedagent_ops_ref→olympus_ref: v0.4.0.agent-ops/.agent-ops.jsonacross all workflow file comments andci.yml.scripts/lint/check-olympus-migration.shthat deterministically verifies the migration invariants.Test plan
cargo build --workspacepassescargo test --workspace— all tests pass (0 failures)scripts/lint/check-olympus-migration.sh— all 18 checks passCloses #166
🤖 Implemented by wiwi • issue author: @vaderyang
Eligible for auto-merge on vivi APPROVE.