Repository navigation
Conversation
bryantderosier
added this pull request to stack #165
September 16, 2026 13:12
Jacksondr5
requested changes
Sep 18, 2026
Jacksondr5
left a comment
Owner
There was a problem hiding this comment.
Requesting changes: the stack edits upstream-owned files without recording a single FORK.md case.
FORK.md ("Add, don't modify") and docs/j5/process/working-in-the-repo.md require every edit to an upstream-owned file to be an appended integration case that is enumerated in the FORK.md inventory. This stack leaves FORK.md byte-identical while touching the files below. That is the blocker for the whole stack, since each later PR merges through this one.
Upstream-owned files touched, by PR:
| PR | File | Shape of the edit |
|---|---|---|
| #156 | apps/server/src/orchestration-v2/ThreadLaunchService.ts |
Appended optional input field. Needs an inventory case. |
| #157 | apps/server/package.json, pnpm-lock.yaml |
Workspace dependency append. Note in the case. |
| #158 | apps/server/src/orchestration-v2/RestartContinuation.ts + test |
Two early returns inserted into upstream function bodies. This file is already listed under "Temporary in-place patches" to be re-evaluated at the next upstream merge; adding more J5 logic there raises that tax. Needs a case and a stated exit plan, or a different mechanism. |
| #159 | apps/server/scripts/copy-playbook-yaml.mjs |
New J5-only file placed under an upstream directory. Move under a j5/ path. |
| #160 | apps/server/src/persistence/Layers/Sqlite.ts |
Appended migration call in the fork-owned lane. Fine in shape, needs a case. |
| #160 | apps/server/package.json (build:bundle) |
Inserts a manifest check and a copy step into upstream's build script. Needs a case, or move the steps to a J5-owned script that wraps the upstream one. |
| #162 | apps/web/src/components/ChatView.tsx, RightPanelTabs.tsx, Sidebar.tsx, LegacySidebar.tsx, routes/_chat.$environmentId.$threadId.tsx, routes/_chat.pull-requests.tsx |
Appended surface wiring, acceptable shape, needs a case. Two upstream strings were rewritten for no functional reason (the "workflow runs" comment in ChatView and the Agents card description); revert those. |
| #162 | apps/web/src/rightPanelStore.ts |
Beyond an appended case: rewrites the persisted-state migration filter and bumps the storage version to 13, which upstream will also use. Keep the append, drop the rewrite, or justify it in the case. |
| #162 | apps/web/src/components/sidebar/SidebarChrome.tsx |
Beyond an appended case: changes the environment pill breakpoint and removes shrink-0 on the brand for every user. Isolate the new button so existing layout rules stay untouched. |
| #163 | apps/web/src/components/settings/SettingsSidebarNav.tsx, settingsSearch.ts, routes/settings.playbooks.tsx |
Appended entries, needs a case. |
| #163 | apps/web/src/hooks/useHandleNewThread.test.ts |
+105 lines importing J5 code into an upstream test file. Move these tests to the J5-owned CreatePlaybookLauncher.logic.test.ts. |
| #164 | apps/server/src/j5/agents/agentPersonaProviderPolicy.ts, agentPersonaRuntime.ts |
J5-owned, so not a fork violation, but note that #164 changes diagnostic authority globally, not only for playbooks. Call that out in the PR body. |
What would unblock this:
- Add one numbered FORK.md case (or a small group) for the Playbooks integration seams, naming the symbols and tests as the existing cases do.
- Reduce the four edits marked "beyond an appended case" to appends, or record the justification for each in the case.
- Move the copy script and the build-step additions into J5-owned locations.
- Move the J5 tests out of the upstream test file.
The runtime and correctness findings from the review are posted separately and are not part of this block.
Reviewed by Claude Fable 5.1 (Claude Code) with GPT-6-Astra (Codex) on the server slices.
This branch has not been deployed
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.
Playbook steps need stable persona definitions and model routes throughout a run.
Bind the required personas when execution starts, save immutable snapshots, and let thread launch consume the prepared assignments without silently rerouting them.
Stack 1/9. Based on
j5/main. Merge in order, retargeting toj5/mainas predecessors land. Full-stack testing branch:codex/playbooks-09-aurora-loops.Validation at the integrated stack tip: 179 server/shared tests and 104 web tests passed; server and web typechecks passed. Each regenerated runtime manifest was checked. The adapter slice independently passed 92 tests, including the parser regressions.
Review order:
Model: GPT-5 · Harness: Codex