Skip to content

fix: converge Desktop titles natively - #87

Draft
ericlitman wants to merge 1 commit into
mainfrom
open-swe/bear-59-source-native-convergence
Draft

fix: converge Desktop titles natively#87
ericlitman wants to merge 1 commit into
mainfrom
open-swe/bear-59-source-native-convergence

Conversation

@ericlitman

Copy link
Copy Markdown
Owner

Summary

  • replace detached App Server title writes with a deterministic persisted outbox
  • drain exact revision/title-guarded operations through the retained control task's native Codex title capability
  • keep operation payloads inside a fixed raw-V8 tool runtime, with aggregate-only reporting and no additional semantic model call
  • bind the control task's own title to its exact terminal ThreadBear footer
  • preserve bounded guided-install close, unchanged-heartbeat silence, drift/ownership guards, retries, and archive ordering

Verification

  • go test ./...
  • go vet ./...
  • CGO_ENABLED=0 GOOS=darwin GOARCH=arm64 go build ./cmd/threadbear
  • CGO_ENABLED=0 GOOS=darwin GOARCH=amd64 go build ./cmd/threadbear
  • sh -n install.sh
  • cmp install.sh site/install.sh
  • cmp INSTALL.md site/install
  • node scripts/replay-title-batch.mjs

The operator-authenticated native Desktop canaries remain the gate for moving this PR out of draft.

Closes BEAR-59

Stage deterministic title operations for the retained source task so native Codex mutations, rather than detached App Server writes, drive visible catalog convergence.

Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com>
@ericlitman

Copy link
Copy Markdown
Owner Author

I found two release-blocking correctness defects and three user-facing contract gaps in the current candidate. Please keep the PR draft and fix these on the existing branch.

  1. Replacement control-task plans disappear before actuation. titlebatch.Service.plan appends the control task only when the pending plan has ExpectedFooter != "", and watch.Run uses the same condition. TestSourceNativePlanRecomputesWhenPersistedFooterDiffers deliberately creates a replacement control-task plan with ExpectedFooter == "". On the next title-batch --list, the bulk inventory excludes the control task, findTask returns false, and the replacement plan is deleted as missing; a heartbeat likewise drains it in settleOrDrainPendingTitles. Include the targeted control-task Task() result whenever any pending control-task plan exists. Add an end-to-end regression that creates the newer-footer replacement, invokes a subsequent list/heartbeat, proves the replacement remains actionable rather than missing, reports native success, and settles canonically.

  2. A realistic batch can yield before completion, but managed guidance forbids waiting. The fixed functions.exec program has no // @exec yield pragma. The tool defaults to 10 seconds, while the live 206-title native batch took about 56 seconds. A yielded call returns a cell ID and requires functions.wait, yet the next instruction says to make no further tool call. Put a sufficiently bounded first-line pragma on the fixed program (120 seconds is supported), and explicitly require functions.wait until that same cell reaches a terminal result if it still yields. Only after the terminal result should the no-more-tools rule apply. Lock this into the managed-content and fresh-V8 replay tests.

  3. The install still gives no expectation before the native repaint. The exact opening update covers installation and health checks, then the guide is silent while a large ready-title batch runs. Once health is known and before the native batch, send one concise update that says ready titles are being repainted now, that a catalog around 200 tasks took about one minute in the live canary, and that older genuinely ambiguous classification continues in the background. This must not expose task IDs, titles, or per-task evidence, and it must not wait for the five-minute cadence.

  4. The V8 result needlessly emits every operation ID into assistant context. The helper-to-helper report needs IDs, but the final text() does not. Emit only counts for accepted, canonically verified, failed, drifted, and rejected operations, then update the replay assertions. This keeps a 200-title run compact and supports the installation's token-efficiency claim.

  5. The success copy promises the wrong final control-task title. The installation templates say the task remains named 🧵🐻 ThreadBear 🐻🧵, but source-native reconciliation changes it to the title corresponding to the staged terminal footer (for the successful close, ✅ ThreadBear 🐻🧵). Rewrite all success/failure/resource-contract occurrences so they accurately say the retained task is ThreadBear's home and its title reflects current status, without freezing the pre-footer bootstrap title as the final visible result.

Please rerun go test ./..., go vet ./..., both Darwin builds, installer/site mirrors, the fresh-V8 replay, and focused replacement-plan tests after the fixes.

@ericlitman

Copy link
Copy Markdown
Owner Author

One additional release blocker is visible in the guided-install contract:

  1. A fresh install stages a footer and then explicitly omits it. The post-install handoff says to stage the exact terminal footer and run the native batch, so the source plan records ExpectedFooter. Later, lines 969–975 say that a fresh installation task with no preloaded ThreadBear rule must omit the footer even when status guidance was just enabled. The persisted turn can therefore never match the staged footer, leaving the source plan at canonical_verified_awaiting_footer until a later recovery cycle replaces or drains it. For a successful fresh install where status guidance and title maintenance enable the native batch, require the final response to end with the exact footer that was staged (normally 🧵🐻 complete); preserve the omit behavior only when no native source plan was staged and no preloaded rule requires a footer. Add a guide-contract test that forbids the stage-plus-omit combination and an end-to-end source settlement test using the actual successful install footer.

@ericlitman

Copy link
Copy Markdown
Owner Author

There is a more fundamental first-install sequencing blocker that directly reproduces the reported stale-title behavior:

  1. The bounded install handoff invokes title-batch while the first sweep cannot have published plans. The guide kickstarts heartbeat, observes “running” or first progress within ten seconds, explicitly does not wait for semantic classification/mutations, and then tells the retained task to stage/list the native batch. But title-batch calls refuseCycle, so stage/list fails whenever cycle.json exists. Even after deterministic evidence is complete, heartbeat does not create PendingTitlePlans until after all Luna work, prepareOperations, applyOperations, and commitState (the plans are created around commitState lines 1462–1473). On the live legacy corpus that boundary arrived around 4.5 minutes; at the intended ten-second handoff there are no bulk plans and the helper refuses the active cycle. Once the heartbeat eventually stages them, the install turn has already ended, so no retained-task native actuator remains to repaint the visible titles.

The first-install path needs a real deterministic handoff boundary: after latest-turn reads and deterministic footer/runtime resolution, commit the mechanically resolved records and guarded native title plans, leave genuinely ambiguous legacy tasks for the semantic continuation, and make the ready plans drainable without racing an active cycle. The installer should then run the native batch against that committed ready set (with the measured about-one-minute expectation for ~200 native writes) and close without waiting for Luna. Add an exact integration rehearsal with a mixed corpus where deterministic rows become drainable within the bounded foreground window while semantic rows remain pending/background; prove title-batch --stage/--list does not return cycle_in_progress, all ready titles settle natively, no Luna call is required for them, and the ambiguous cohort is preserved for later processing. A test that only starts heartbeat or only exercises a pre-populated pending-plan fixture is not sufficient.

@mobilyze-open-swe-studio2 mobilyze-open-swe-studio2 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.

Open SWE Review found 1 potential issue.

Open in Web


```js
const q=v=>"'"+String(v).replace(/'/g,"'\\''")+"'";
const run=async cmd=>JSON.parse((await tools.exec_command({cmd})).output);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟠 Failed reports masquerade as success

The fixed batch program parses exec_command output but ignores the command exit status, so a failing title-batch --json --report still flows through as a normal JSON value. If the native setter succeeds and the report command then exits non-zero (for example because saving state failed), the program emits a final aggregate instead of failing closed; ThreadBear keeps the operation as NativeTitlePending, and the next list sees the now-changed title as drift and deletes the pending plan instead of committing ownership. Please make the helper throw on non-zero exec_command results (and mirror the same fixed-program change in assets/skill/SKILL.md) before using the parsed result.

(Refers to line 13)


Your feedback helps Open SWE learn. React with 👍 or 👎 to tell us if this review comment was useful.

Suggested change
const run=async cmd=>JSON.parse((await tools.exec_command({cmd})).output);
const run=async cmd=>{const r=await tools.exec_command({cmd});const x=JSON.parse(r.output);if(r.exit_code&&r.exit_code!==0)throw new Error(x.error_code||"title_batch_command_failed");return x;};

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.

1 participant