Conversation
Addresses state transition issues, testing infrastructure bugs, and consistency problems discovered through repeated end-to-end cycling of the software factory flow (factory:go → bootstrap → implement → validate). Key changes: - Orchestrator owns issue status transitions (backlog → in_progress → done) rather than letting the agent set status directly, which caused premature exits when validation was still failing - Card update tools (update_issue, update_project, create_knowledge) use read-patch-write instead of full document replacement, preventing attribute clobbering on partial updates - Fix buildCardDocument to use correct darkfactoryModuleUrl from software-factory realm instead of target realm, which broke issue refresh searches by type - Fix Playwright waitForFunction timeout argument position (was being treated as arg instead of options, causing 30s default instead of configured 5min) - Remove run_tests from agent tool set — validation pipeline handles test execution automatically after each agent turn - Add ensureJsonExtension helper for card tool paths (card+source requires full file path with extension) - Auto-mark project as completed when all issues are done - Set issue to in_progress when loop picks it up - Status badges on Issue and Project cards (fitted + isolated templates) - TestRun 0/0 passed shows gray "empty" badge instead of green - Fix uneven red border on failing test module groups - Issue linked-card in isolated view uses embedded format to prevent occlusion - Drop "MVP" from project naming convention - Update prompts, skills, and phase-2-plan.md with validated design decisions - 352 unit tests passing, all covering new behaviors Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The test was passing status: 'in_progress' to update_issue, but the tool now only allows 'blocked' and 'backlog' (orchestrator owns other transitions). Updated test to use 'blocked'. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Gate project completion on all realm issues being done (not just processed ones) - Read-patch-write: only create fresh doc on confirmed 404, error on other failures - Extract shared readPatchDocument helper for update_project/update_issue/create_knowledge - Copy attributes before mutating to avoid side effects on caller's args - Fix test comment about removed buildRunTestsTool - Add note that we expect one Project per realm Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add `status` field to readFile() return type so callers can distinguish 404 (not found) from auth errors, network failures, and server errors. Update callers: createSeedIssue() now throws on non-404 failures instead of silently proceeding to write. readPatchWrite() similarly guards against non-404 errors. Logging in RealmIssueRelationshipLoader and RealmIssueStore now includes HTTP status for better debugging. Fixes CS-10722 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 888e8bc0b7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Contributor
There was a problem hiding this comment.
Pull request overview
This PR enhances the software-factory realm I/O and orchestration flow by surfacing HTTP status codes from readFile() and using them to make safer decisions (e.g., distinguishing “not found” from other failures), while also refining the agentic loop’s status-transition responsibilities and related tooling/docs.
Changes:
- Add
status?: numbertoreadFile()results and thread status-aware handling into seed creation, patching, and logging. - Update orchestration semantics: loop sets
in_progresson pickup, promotes todoneonly aftersignal_done+ passing validation, and optionally marks the Projectcompletedwhen all realm issues are done. - Switch card update tools to read–patch–write (preserve existing attrs), normalize
.jsonpaths, adjust tests, prompts, and realm UI templates accordingly.
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/software-factory/src/realm-operations.ts | Adds status?: number to readFile() responses and introduces ensureJsonExtension(). |
| packages/software-factory/src/factory-seed.ts | Uses readFile().status to decide when to create the seed issue vs surface failures. |
| packages/software-factory/src/factory-tool-builder.ts | Adds read–patch–write helper, normalizes .json paths, restricts agent-set issue statuses, removes run_tests tool. |
| packages/software-factory/src/issue-loop.ts | Orchestrator owns status transitions (in_progress on pickup; done on signal_done + passing validation) and project completion update. |
| packages/software-factory/src/issue-scheduler.ts | Uses ensureJsonExtension() for issue reads/writes; adds optional updateProjectStatus() to IssueStore and implements it in RealmIssueStore. |
| packages/software-factory/src/realm-issue-relationship-loader.ts | Improves warn logs to include HTTP status codes from readFile(). |
| packages/software-factory/src/darkfactory-schemas.ts | Makes buildCardDocument() take an explicit darkfactoryModuleUrl (software-factory realm) rather than deriving from target realm. |
| packages/software-factory/src/factory-issue-loop-wiring.ts | Threads darkfactoryModuleUrl into ToolBuilderConfig. |
| packages/software-factory/src/test-run-execution.ts | Fixes Playwright waitForFunction argument ordering and increases timeout for QUnit completion wait. |
| packages/software-factory/tests/issue-loop.test.ts | Updates mocks/expectations and adds coverage for orchestrator-owned status transitions + project completion behavior. |
| packages/software-factory/tests/factory-tool-executor.spec.ts | Updates tool config to provide darkfactoryModuleUrl; adjusts issue status expectations. |
| packages/software-factory/tests/factory-tool-builder.test.ts | Updates mocks for GET+POST read–patch–write and adds tests for status stripping and merging. |
| packages/software-factory/scripts/smoke-tests/factory-tools-smoke.ts | Supplies darkfactoryModuleUrl in smoke-test tool config. |
| packages/software-factory/realm/test-results.gts | Tweaks TestRun UI to show “empty” status when 0/0 passed, and adjusts styling. |
| packages/software-factory/realm/darkfactory.gts | Adds status badges styling for Issue/Project and embeds linked Project in Issue isolated view. |
| packages/software-factory/prompts/system.md | Updates system prompt to reflect removal of run_tests tool and automatic validation after signal_done. |
| packages/software-factory/prompts/ticket-implement.md | Reorders implementation steps and clarifies orchestrator-managed status transitions. |
| packages/software-factory/prompts/bootstrap-implement.md | Updates bootstrap artifact naming and completion instructions (use signal_done). |
| packages/software-factory/.agents/skills/software-factory-operations/SKILL.md | Removes run_tests tool guidance and updates required flow ordering. |
| packages/software-factory/.agents/skills/software-factory-bootstrap/SKILL.md | Updates project/issue naming conventions and completion instructions (use signal_done). |
| packages/software-factory/docs/phase-2-plan.md | Updates design documentation for orchestrator-owned transitions, read–patch–write tools, and related operational details. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
readFile() returns status=undefined on network errors. The previous guards checked `status !== undefined && status !== 404`, which let network errors fall through to create/overwrite cards. Simplify to `status !== 404` so undefined (network error) is also caught. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
burieberry
approved these changes
Apr 14, 2026
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.
Summary
status?: numberfield toreadFile()return type inrealm-operations.tsso callers can distinguish 404 (not found) from auth errors, network failures, and server errorscreateSeedIssue()now throws on non-404 failures instead of silently proceeding to write a new seed issuereadPatchDocument()now usesstatusfield instead of string-parsingexisting.errorto detect 404sRealmIssueRelationshipLoaderandRealmIssueStoreto include HTTP status codes for debuggingTest plan
tsc --noEmit)vitest run)pnpm lint)okare unaffected (backward compatible)createSeedIssue()are now surfaced as errorsFixes CS-10722
🤖 Generated with Claude Code
Co-Authored-By: Claude Opus 4.6 (1M context) noreply@anthropic.com