Repository navigation
Chore: Move the demos to rossoctl/cortex-demos - #1759
Conversation
The demos are now in https://github.com/rossoctl/cortex-demos, with their history. This change removes demos/ and the demo skill from this repo. - CI: remove the go-ci-demos job (it is now in cortex-demos). Remove the demos exclusions from dependabot-tidy and the Trivy config scan. - The cpex hr-demo tests read two hr-cpex manifests. Copies of those files are now in core/plugins/cpex/testdata/hr-cpex/. - Links and comments that point at demos/ now point at cortex-demos. - CLAUDE.md, .github/CLAUDE.md and CONTRIBUTING.md: 7 Go modules, not 10. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
|
Important Review skippedToo many files! This PR contains 231 files, which is 131 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry.
 |
clawgenti
left a comment
clawgenti
left a comment
There was a problem hiding this comment.
Summary
This is a clean move. After the change no tracked file outside docs/superpowers/ refers to demos/. go.work and the Makefile never listed the demo modules. dependabot.yml, CODEOWNERS and the spellcheck config have no demo entries. The removed go-ci-demos job is now in rossoctl/cortex-demos/.github/workflows/ci.yaml, with the same matrix and steps. I checked every new cortex-demos link and path against the new repo's main tree. All of them exist, and so do the #configmaps-and-secrets-expected-at-runtime, #shared-volume-contract and #building-everything-locally anchors in its CLAUDE.md. The one exception is mtls/ (see inline). The two testdata/hr-cpex/ copies match cortex-demos/hr-cpex/k8s/ byte for byte. The demo skill now lives in cortex-demos under .claude/skills/demo/. The demo modules never imported core, so dropping the job loses no coverage of core's API.
Two nits, inline. There is also a pre-existing one outside the diff: the allow-ghsas exception in security-scans.yaml (around lines 48-52) points at demo/agents/github_agent/pyproject.toml. That path does not exist, and crewai no longer appears anywhere in this repo. You could drop the exception, or move it to cortex-demos if it is needed there.
Author: huang195 (MEMBER — maintainer)
Areas reviewed: CI/GitHub Actions, Go (test paths and comments), Docs, cross-repo link targets
Agent/IDE config (.claude/.vscode): one deletion only (.claude/skills/demo/SKILL.md, now in cortex-demos). Nothing added, modified or renamed.
Commits: 1 commit, signed off: yes
CI status: passing
| @@ -806,7 +806,7 @@ flows through to a per-agent rendered envoy-config with the matching TLS | |||
| blocks. | |||
|
|
|||
| > **This envoy-sidecar design has no end-to-end verification in-tree.** The | |||
| > `demos/mtls/` demo that used to exercise it was retired as superseded. | |||
| > `cortex-demos/mtls/` demo that used to exercise it was retired as superseded. | |||
There was a problem hiding this comment.
nit: mtls/ was retired before the split, so it was never in cortex-demos and cortex-demos/mtls/ does not exist. Rewriting the path here makes it read as if it had been moved. Suggest keeping the historical name, for example: "The former demos/mtls/ demo that used to exercise it was retired as superseded."
| func hrManifestConfig(t *testing.T) hrCPEXConfig { | ||
| t.Helper() | ||
| raw, err := os.ReadFile(filepath.Join("..", "..", "..", "demos", "hr-cpex", "k8s", "authbridge.yaml")) | ||
| raw, err := os.ReadFile(filepath.Join("testdata", "hr-cpex", "authbridge.yaml")) |
There was a problem hiding this comment.
nit: The comments around this function still say k8s/authbridge.yaml (lines 296, 312, 321, 352, 357, 362), which was the path in the demo tree. The test now reads testdata/hr-cpex/authbridge.yaml. A short note here and in hrPolicy would help, something like "copied from cortex-demos/hr-cpex/k8s/; resync by hand". It would make the drift the PR mentions under "Not done" visible to the next person who edits these tests.
The mTLS demo was retired before the move. It is not in cortex-demos, so the note names its old path, demos/mtls/. The hr-cpex test reads copies in testdata/hr-cpex/, not the demo's k8s/ files. The header now says the files are a copy that does not follow cortex-demos. The comments and t.Fatal messages name the testdata path. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
clawgenti
left a comment
clawgenti
left a comment
There was a problem hiding this comment.
Summary
I re-reviewed at 262e881. The second commit fixes both nits from my review of 160e7ef. docs/framework-architecture.md is now unchanged from main and keeps the historical demos/mtls/ name. The hr-cpex test header says the two manifests are copies that do not follow cortex-demos, and the comments and t.Fatal messages name testdata/hr-cpex/.
What I checked at head:
- No dangling references. Outside
docs/superpowers/, no tracked file names ademos/path. The only match is the intentionaldemos/mtls/note.go.work, the rootMakefile,.dockerignore,dependabot.yml,.pre-commit-config.yamland every remaininggo.modreplacehave no demo entries. Nothing outsidedemos/imported the demo modules, and none of the three demo modules importedcore. No otherrossoctlrepo links torossoctl/cortex/.../demos(org code search; theauthbridge/demoshits inrossoctl/rossoctlpoint at a path that was retired earlier, not at this one). - New repo. All 205 files that were under
demos/onmainexist at the same paths inrossoctl/cortex-demos@main(its tree is public and not truncated). Every newcortex-demoslink and path in this diff resolves, and so do the#shared-volume-contract,#configmaps-and-secrets-expected-at-runtimeand#building-everything-locallyanchors. Itsci.yamlhas the sameecho/finance-sparc/ibacvet+build matrix as the removedgo-ci-demosjob. No branch rule requiresgo-ci-demos, so removing it cannot block merges. - cpex tests. The two
testdata/hr-cpex/files are byte-identical todemos/hr-cpex/k8s/onmainand tocortex-demos/hr-cpex/k8s/. The PR touchescore/, sogo-ci-cpexrango test -tags cpex ./plugins/cpex/...at 262e881 and passed. The 18TestHRDemo*functions have no skips, so CI did exercise the relocated testdata on this PR. (The PR body's "CI does not run these tests" holds only for PRs that do not touch the cpex paths.) I could not reproduce locally because the Go module proxy timed out.
Pre-existing and outside the diff (repeating it in case it got lost): the allow-ghsas: GHSA-f4j7-r4q5-qw2c exception in security-scans.yaml (lines 48-52) cites demo/agents/github_agent/pyproject.toml, which does not exist, and nothing in this repo uses crewai. It could be dropped in a follow-up.
Author: huang195 (MEMBER — maintainer)
Areas reviewed: CI/GitHub Actions, Go (test paths and comments), Docs, cross-repo link targets
Agent/IDE config (.claude/.vscode): one deletion only (.claude/skills/demo/SKILL.md, present in cortex-demos). Nothing added, modified or renamed.
Commits: 2 commits, all signed off: yes
CI status: passing
github-project-automation
Bot
moved this from New/ToDo
to Done
in Rossoctl Issue Prioritization
Summary
The demos moved to rossoctl/cortex-demos, with their history. This PR removes
demos/and thedemoskill from this repo.Changes
demos/(13 demos) and.claude/skills/demo/.go-ci-demosjob. cortex-demos has the same job now. Removed thedemosexclusions fromdependabot-tidy.ymland from the Trivy config scan.hrdemo_cpex_test.goread twohr-cpexmanifests fromdemos/. Copies of those two files are now incore/plugins/cpex/testdata/hr-cpex/.demos/now point at cortex-demos.How the new repo was made
git filter-repokept every commit that touchedAuthBridge/demos/,authbridge/demos/ordemos/, and moved those paths to the repo root (505 commits).github.com/rossoctl/cortex-demos/<demo>.agentopfrom source readCORTEX_DIR. The default is a cortex checkout beside cortex-demos.Verification
go vetandgo buildpass forecho,finance-sparcandibac.go vet ./...passes incoreandcmd/agentop. The 28 hr cpex tests pass with-tags cpexagainst the v0.3.1 FFI on darwin-arm64. CI does not run these tests.Not done
Assisted-By: Claude (Anthropic AI) noreply@anthropic.com