Skip to content

Feature/codex fable opus pipeline - #20

Closed
Thibault1818 wants to merge 16 commits into
oxgeneral:mainfrom
Thibault1818:feature/codex-fable-opus-pipeline
Closed

Feature/codex fable opus pipeline#20
Thibault1818 wants to merge 16 commits into
oxgeneral:mainfrom
Thibault1818:feature/codex-fable-opus-pipeline

Conversation

@Thibault1818

Copy link
Copy Markdown

What does this PR do?

Type of change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Refactoring (no functional changes)
  • Documentation update
  • CI/build improvement

Checklist

  • npm test passes (all tests green)
  • npx tsc --noEmit is clean (no type errors)
  • I've added tests covering my changes
  • I've updated relevant documentation (if applicable)
  • My code follows the project conventions (ESM imports with .js, no any, no console.log)
  • I've self-reviewed my own code

Test plan

Related issues

@Thibault1818
Thibault1818 requested a review from oxgeneral as a code owner August 3, 2026 15:31

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎉 Thanks for your first PR to ORCH! We're excited to review your contribution.

Our CI will run automatically:

  • ✅ TypeScript strict mode check (tsc --noEmit)
  • ✅ Full test suite (1493+ tests via Vitest)
  • ✅ ESM build verification

A maintainer will review your changes shortly. In the meantime, check the Contributing Guide to make sure everything is in order.

⭐ If you enjoy working with ORCH, a star on the repo goes a long way!

@oxgeneral

Copy link
Copy Markdown
Owner

Thanks for the contribution. I can’t merge this PR in its current form.

The PR mixes the Codex–Fable–Opus pipeline with extensive unrelated changes: it renames the package, disables npm publishing, rewrites repository and security links to the contributor’s fork, and changes existing adapters, goals, TUI, dependencies, and generated dist files.

It is also behind main, has multiple merge conflicts, and the actual native workflow is not covered by end-to-end tests—the current integration tests use fake Codex, Claude, and Git adapters. The default merge gate runs only git diff --check, and the merge implementation does not verify the target branch before merging.

Please split this into separate PRs:

  1. Security hardening, rebased onto the latest main.
  2. The workflow feature with real subprocess/worktree integration tests.

All upstream package identity, npm publishing, repository links, and security-reporting links must remain unchanged. Please also remove unrelated fork-specific and generated changes.

@Thibault1818

Copy link
Copy Markdown
Author

Thank you for the clear review. You were right that #20 mixed the workflow with unrelated package identity, publishing, repository/security-link, fork-specific, and generated dist changes.

I replaced it with two fresh PRs based directly on the current upstream main:

The replacement PRs preserve upstream package identity, npm publishing, repository/documentation/security-reporting links, and contain no generated dist files. The workflow integration tests launch the real ORCH CLI as an OS subprocess, use real Git repositories/branches/worktrees/commits/merges, and place deterministic local codex/claude executables on an isolated PATH; no paid or network model calls are made.

Closing #20 as superseded by #21 and #22.

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.

3 participants