Skip to content

Commit 495767d

Browse files
committed
Merge origin/main into claude/issue-4405-filter-conformance-mongo
2 parents a4aa9e5 + ff17642 commit 495767d

16 files changed

Lines changed: 753 additions & 13 deletions
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
---
2+
"@objectstack/runtime": patch
3+
"@objectstack/observability": patch
4+
---
5+
6+
fix(runtime): declarative `defineJob` cron jobs are actually scheduled (#4567)
7+
8+
Every background job authored as `defineJob({ schedule: { type: 'cron', … } })`
9+
was **silently never scheduled**. `JobSchema.parse` rewrites the cron
10+
`expression` into the canonical expression envelope
11+
(`{ dialect: 'cron', source: '0 1 * * *' }` — the authoring/persistence tier),
12+
but `AppPlugin` handed `job.schedule` verbatim to `IJobService.schedule`, whose
13+
boundary contract documents `expression` as a **bare cron string** because
14+
`CronJobAdapter` passes it straight to croner. croner rejected the object
15+
(`CronPattern: Pattern has to be of type string.`), the throw was swallowed by a
16+
per-job `try/catch` that only `warn`ed, and the author saw a green build and a
17+
green boot with the job never running. `interval` / `once` schedules and
18+
flow `schedule` triggers were unaffected.
19+
20+
**Fix (contract-first).** The authoring→boundary downgrade now happens at the one
21+
place the two tiers meet — `AppPlugin`'s declarative-job registration, alongside
22+
the existing `retryPolicy` / `timeout` threading — via
23+
`toBoundaryJobSchedule()`. The adapters stay strict: no `typeof === 'object'`
24+
tolerance was added downstream, so the boundary keeps exactly one shape.
25+
A schedule that cannot be reduced to it (unknown type, AST-only or non-`cron`
26+
expression envelope, missing `intervalMs` / `at`) is rejected by name.
27+
28+
**The failure path is no longer silent.** A job that cannot be scheduled now logs
29+
at **error** level with its own message (`Background job FAILED TO SCHEDULE — it
30+
will never run`), plus a boot summary line when any job failed, and increments
31+
the new `job_schedule_failures_total` counter
32+
(`SEMCONV.jobScheduleFailuresTotal`, labels `app` / `job`) on the observability
33+
metrics registry. "Failed to schedule" no longer shares the quiet `warn` used by
34+
"handler not found in bundle.functions" — the first is an outage of declared
35+
work, the second is a job that was never going to run.
36+
37+
No authoring change is required: existing `defineJob` cron declarations start
38+
working on upgrade.

.changeset/duplicate-fix-guard.md

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
---
2+
---
3+
4+
ci: fail a PR at open time when an earlier open PR already declares a fix for
5+
the same issue (#4588)
6+
7+
Release-nothing: adds `.github/workflows/duplicate-fix-guard.yml` and updates
8+
agent process docs (AGENTS.md, CLAUDE.md, pm-dispatch claim template) — no
9+
package code.
10+
11+
GitHub lets any number of open PRs declare `Fixes #N` for the same issue.
12+
On 2026-08-02, #4555 and #4559 both declared `Fixes #4551` and both were
13+
implemented in full — 834 duplicate lines through the whole gate suite — with
14+
the duplication machine-detectable from the second PR's open (03:08) yet
15+
unnoticed by any human until 08:52. The shared GitHub identity made the
16+
issue's assignee useless as a warning: "assigned to os-zhuang" reads the same
17+
whether the claimant is you or another session.
18+
19+
Three changes, one per hole:
20+
21+
- **Duplicate Fix Guard workflow**: on PR opened/edited/reopened/synchronize,
22+
parse same-repo closing keywords and fail the PR if an EARLIER open PR
23+
(lower number) declares the same issue, naming it. First come, first
24+
served — matching the pm-dispatch "first claim comment wins" convention.
25+
The check is body-driven and re-runs on `edited`, so a red PR goes green
26+
the moment the conflict is resolved either way.
27+
- **Claim comments must carry a session ID** (pm-dispatch template, AGENTS.md,
28+
CLAUDE.md): under a shared identity, the comment's session line is the only
29+
thing that makes "is this claim mine?" answerable.
30+
- **Branch naming `claude/issue-<n>-<slug>`** (AGENTS.md): puts the issue
31+
number where `git ls-remote | grep issue-<n>` can find it; the workflow
32+
warns (never fails) when a fix PR's branch names no declared issue.
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
---
3+
4+
Internal agent tooling only (`.claude/` pm-dispatch escalation bar + API-body generics trap) — releases nothing.

.claude/agents/os-dev.md

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,23 @@ rules that most often get missed:
3333
consumer (`??` alias, tolerant parse), the bug is at the producer or in the
3434
spec — fix it there, or return `needs_decision`.
3535

36+
**Resource discipline — parallel agents share ONE container; unbounded
37+
build/test runs OOM it.** Binding rules:
38+
39+
1. **Serialize the heavy phase.** Wrap every build and test run in the shared
40+
verification lock, so editing parallelizes but memory peaks never stack:
41+
`flock -w 7200 /tmp/os-heavy-verify.lock -c '<build/test command>'`
42+
(one lock file per container; waiting on it is normal, not a hang).
43+
2. **Cap the heap.** Prefix heavy commands with
44+
`NODE_OPTIONS=--max-old-space-size=4096` (raise only with a reason).
45+
3. **Scope, don't sweep.** Build and test the affected packages
46+
(`pnpm --filter <pkg> build/test`), not the whole repo, unless the task
47+
explicitly requires a full pass. Cap test parallelism:
48+
vitest `--maxWorkers=2`, turbo `--concurrency=2`.
49+
4. **Clean up when done**: after the PR is up, remove your worktree
50+
(`git worktree remove <path> --force`) — leftover `node_modules` trees
51+
exhaust the container's disk, which fails as confusingly as OOM.
52+
3653
Definition of done, in order:
3754

3855
- Implementation matches the issue's acceptance criteria.
@@ -89,3 +106,10 @@ Final message — exactly this JSON, no prose around it:
89106

90107
Use `status: "rework"` for a partial result you know is incomplete (say why in
91108
`summary`); the PM will review and re-dispatch with feedback.
109+
110+
Practical trap when filing issues/PRs through the GitHub API: the body
111+
sanitizer strips `<` followed by a letter as an HTML tag **at rest**, which
112+
destroys TypeScript generics (`Assert<Equal<1, 2>>` is stored as `Assert>`).
113+
Write generics with a space after each `<``Assert< Equal< 1, 2 > >` is
114+
still valid TypeScript — and read the stored body back to verify when a
115+
snippet is load-bearing.

.claude/skills/pm-dispatch/SKILL.md

Lines changed: 79 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,23 @@ known case: accepting a `repo:objectui` PR ⇒ file a `pm:queue` issue in
102102
`objectstack` — "run `pnpm objectui:refresh` and land the console bump",
103103
referencing the merged PR, blocked-by it until it actually merges.
104104

105-
**4. One board, no second tracker.** The pm labels above are the state
105+
**4. Multiple PM sessions shard by repo — never share one queue.** The
106+
claim protocol makes concurrent PMs *safe*, not *useful*: batch
107+
independence (file-disjointness) is only checked within one PM's view, so
108+
two PMs on the same queue can claim different issues that collide on
109+
shared files, and the merge queue is one lane regardless. Scaling order:
110+
111+
1. One PM, bigger batch (`batch:5` is the maintainer's chosen operating
112+
point, riding on the resource discipline above), heavy tasks via
113+
`mode:cloud` — adds compute without adding schedulers.
114+
2. When one PM genuinely can't keep up: a second session takes a **whole
115+
repo** as its shard (`/pm-dispatch repo:objectstack-ai/objectui`) —
116+
file universes are disjoint by construction. A sharded PM states its
117+
shard in every claim comment and **never claims outside it**; cross-repo
118+
parent/sub-issue chains stay with the main-backlog PM.
119+
3. Multiple PMs on the SAME queue: prohibited — all cost, no throughput.
120+
121+
**5. One board, no second tracker.** The pm labels above are the state
106122
machine; an org-level GitHub Project pulling issues/PRs from all three repos
107123
gives the maintainer a single view (filter by `repo:*` and `pm:*`). The PM
108124
maintains no tracking state outside GitHub — that invariant is what keeps
@@ -189,18 +205,34 @@ execute atomically, in order:
189205
1. **Assign** to yourself (`@me`) and add `pm:dispatched`. Skip — and drop
190206
from the batch — any issue that acquired an assignee since step 1.
191207
2. **Claim comment** (Chinese), fixed shape — the branch name is the key,
192-
every later artifact (worktree, push, PR) hangs off it:
208+
every later artifact (worktree, push, PR) hangs off it. The session ID is
209+
NOT optional: under the shared identity it is the only line that lets a
210+
later reader — including your own future self after a context reset —
211+
answer "is this claim mine?". A claim without it caused the #4555/#4559
212+
duplicate (#4588): the second session saw its own shared name as assignee
213+
and could not tell the claim was someone else's.
193214
> 认领:PM 循环第 N 轮
215+
> 会话:`session_<id>`
194216
> 分支:`claude/issue-<n>-<slug>`
195217
> Worktree:`<repo>-issue-<n>`
196218
3. **Race check**: assignment is idempotent, so two agents can both
197219
"succeed". Re-read the comments; if an earlier claim comment with a
198-
*different* branch name exists, you lost — touch nothing of theirs,
199-
reply 「已有认领,让行」, and pick another issue. First comment wins.
220+
*different* session ID or branch name exists, you lost — touch nothing of
221+
theirs, reply 「已有认领,让行」, and pick another issue. First comment wins.
200222

201223
Dev agents push their branch early — a remote branch is the hardest evidence
202224
of work in flight, closing the gap between "claimed" and "PR exists".
203225

226+
**Multiple GitHub accounts (colleagues' Claude Code sessions) simplify
227+
this, not complicate it.** Across accounts the assignee alone already says
228+
*who*: `assignee isn't you → taken, never touch` is the entire cross-account
229+
protocol, and it's already the rule. The claim-comment ritual (branch name,
230+
round, race check) matters *within* one account's sessions. When several
231+
accounts work the backlog, partition it the same way as multi-PM sharding —
232+
by repo or by an agreed label per account — and record the assignment table
233+
once in a pinned issue or the round report so nobody triages another
234+
account's shard.
235+
204236
**Stale-claim reclaim**: a claim older than ~24 h whose promised branch does
205237
not exist on the remote and has no PR is presumed dead — comment asking, and
206238
after another window of silence, remove the assignee (note why) and return
@@ -242,6 +274,19 @@ Follow your operating procedure (you are the os-dev agent). Non-negotiables:
242274
Return ONLY the JSON report defined in your agent definition.
243275
```
244276

277+
#### Resource limits — parallel agents share ONE container
278+
279+
Memory peaks come from **build + test**, not editing, so the fix is not less
280+
parallelism but serialized heavy phases: the os-dev definition requires every
281+
build/test run to hold the container-wide verification lock
282+
(`flock /tmp/os-heavy-verify.lock`), a `NODE_OPTIONS=--max-old-space-size`
283+
heap cap, scoped `--filter` builds/tests, capped vitest/turbo workers, and
284+
worktree cleanup after the PR is up. PM-side: treat `batch:3` as assuming
285+
normal-sized tasks — for build-heavy ones (dependency-family upgrades, full
286+
regression passes) drop to `batch:2`, or dispatch that issue via
287+
`mode:cloud` so it gets its own container. If an agent dies with a
288+
heap/OOM signature, redispatch it alone rather than into a full batch.
289+
245290
#### Dispatch backends
246291

247292
**`mode:subagent` (default).** The `Agent` tool, as described above. The devs
@@ -313,8 +358,36 @@ Verdict per issue:
313358

314359
### 8. Escalate uncertainties to the maintainer
315360

316-
Whenever a dev returns `needs_decision`, an issue is too vague to dispatch, or
317-
rework has failed twice:
361+
**First, apply the escalation bar — most things that FEEL like decisions are
362+
not.** The maintainer's words: 「明显的问题直接修,不是事事都需要我确认」.
363+
Escalate ONLY when at least one of these holds:
364+
365+
- the options genuinely diverge on **product semantics or public contract
366+
shape** and neither the issue, AGENTS.md, ADRs, nor existing code norms
367+
determines the answer;
368+
- the fix requires a **destructive or hard-to-reverse action** (stored-data
369+
migration shape, deleting a shipped capability, force operations).
370+
371+
Everything else is the PM's call — decide, dispatch, and give the maintainer
372+
a **veto window instead of a permission gate**: state what you decided and
373+
why in the issue comment and the round report; they can stop it, but you do
374+
not wait for them. Named non-escalation classes (act immediately):
375+
376+
- **Restore-invariant fixes.** When the repo already states the invariant —
377+
one contract version across the family, declared = enforced, a gate must
378+
actually compile/run what it claims to check — a finding that the
379+
invariant is broken carries its own decision. A dual-version dependency
380+
graph, an inert tripwire, an unwired gate: queue it, dispatch it, report
381+
it. Asking "may I restore the invariant?" is the anti-pattern.
382+
- **Sequencing and dependency ordering** between technical tasks.
383+
- **Verification strategy** (what regression pass a risky-but-decided change
384+
needs) — that is scoping the work, not deciding it.
385+
- A dev's `needs_decision` that, on PM review, falls into the classes above:
386+
answer the dev yourself with the decision and rationale; do not relay it
387+
upward.
388+
389+
Whenever a dev returns `needs_decision` that passes the bar above, an issue
390+
is too vague to dispatch, or rework has failed twice:
318391

319392
1. **Default: the decision lives ON the issue it belongs to — never a new
320393
issue.** Post the analysis as a comment on that issue, add the
Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,140 @@
1+
# GitHub happily lets any number of open PRs declare `Fixes #N` for the same
2+
# issue. On 2026-08-02 that cost a full duplicate implementation: #4555 and
3+
# #4559 both declared `Fixes #4551`, both ran the whole gate suite green, and
4+
# the duplication sat machine-detectable from the moment the second PR opened
5+
# (03:08) until a human noticed it (08:52). All agents here share one GitHub
6+
# identity, so the issue's assignee could not warn the second author either —
7+
# "assigned to os-zhuang" reads the same whether it is you or another session.
8+
# Full post-mortem: #4588.
9+
#
10+
# This gate makes the second PR red at open time. First come, first served —
11+
# the EARLIER open PR (lower number) keeps its claim and stays green; the
12+
# later one fails with a pointer to it. That matches the pm-dispatch claim
13+
# convention ("first claim comment wins") already in .claude/skills/.
14+
#
15+
# Scope: same-repo references only (bare `#N` and qualified `<this repo>#N`).
16+
# Cross-repo references are the cross-repo-issue-closer's territory, and a
17+
# duplicate across repos cannot be resolved by failing one side's CI anyway.
18+
name: Duplicate Fix Guard
19+
20+
# `edited` matters as much as `opened`: a PR that adds `Fixes #N` to its body
21+
# after the fact must re-run this check, and one that drops the line must be
22+
# able to go green again.
23+
on:
24+
pull_request:
25+
types: [opened, edited, reopened, synchronize]
26+
27+
permissions:
28+
pull-requests: read
29+
30+
jobs:
31+
duplicate-fix-guard:
32+
name: No other open PR may claim the same issue
33+
runs-on: ubuntu-latest
34+
steps:
35+
- name: Check declared issues against other open PRs
36+
uses: actions/github-script@v9
37+
with:
38+
script: |
39+
const pr = context.payload.pull_request;
40+
const thisRepo = `${context.repo.owner}/${context.repo.repo}`;
41+
42+
// GitHub's own closing-keyword set. The optional colon is part of
43+
// GitHub's accepted syntax (`Fixes: #123`). Two same-repo forms:
44+
// bare `#N` and qualified `owner/repo#N` naming THIS repo.
45+
const KEYWORDS = 'close|closes|closed|fix|fixes|fixed|resolve|resolves|resolved';
46+
const pattern = new RegExp(
47+
`\\b(?:${KEYWORDS}):?\\s+(?:([\\w.-]+)\\/([\\w.-]+))?#(\\d+)\\b`,
48+
'gi',
49+
);
50+
51+
// Strip fenced blocks and inline code spans before matching.
52+
// GitHub's own closing-keyword parser ignores code formatting, and
53+
// this guard must not be stricter than the linker it protects:
54+
// this very PR's first run counted its own DISCUSSION of
55+
// `Fixes #4551` (in backticks, describing the incident) as a
56+
// declaration. Benign there — #4551 is closed — but a prose
57+
// mention of an issue some other open PR really fixes would have
58+
// been a spurious red.
59+
const declaredIssues = (body) => {
60+
const prose = (body || '')
61+
.replace(/```[\s\S]*?```/g, ' ')
62+
.replace(/`[^`\n]*`/g, ' ');
63+
const found = new Set();
64+
for (const [, owner, repo, number] of prose.matchAll(pattern)) {
65+
// A qualified reference to ANOTHER repo is not ours to judge.
66+
if (owner && `${owner}/${repo}`.toLowerCase() !== thisRepo.toLowerCase()) continue;
67+
found.add(Number(number));
68+
}
69+
return found;
70+
};
71+
72+
const mine = declaredIssues(pr.body);
73+
if (mine.size === 0) {
74+
core.info('This PR declares no same-repo closing keywords — nothing to guard.');
75+
return;
76+
}
77+
core.info(`This PR declares: ${[...mine].map((n) => `#${n}`).join(', ')}`);
78+
79+
// Branch-name convention (advisory, never red): a fix branch named
80+
// `claude/issue-<n>-<slug>` is discoverable by the next session
81+
// with one `git ls-remote | grep issue-<n>`. #4555 vs #4559
82+
// happened partly because the branches shared no token to grep.
83+
// Warning only — existing branches must not go red retroactively.
84+
const branch = pr.head.ref;
85+
if (![...mine].some((n) => branch.includes(`issue-${n}`))) {
86+
core.warning(
87+
`Branch \`${branch}\` does not name any declared issue. ` +
88+
`Convention: claude/issue-<n>-<slug> (e.g. claude/issue-${[...mine][0]}-short-slug) ` +
89+
`so parallel sessions can discover in-flight work with git ls-remote.`,
90+
);
91+
}
92+
93+
// Drafts count: a draft PR is work in flight, which is exactly
94+
// what the second session needs to see.
95+
const openPrs = await github.paginate(github.rest.pulls.list, {
96+
owner: context.repo.owner,
97+
repo: context.repo.repo,
98+
state: 'open',
99+
per_page: 100,
100+
});
101+
102+
const conflicts = [];
103+
for (const other of openPrs) {
104+
if (other.number === pr.number) continue;
105+
const theirs = declaredIssues(other.body);
106+
const shared = [...mine].filter((n) => theirs.has(n));
107+
if (shared.length > 0) conflicts.push({ other, shared });
108+
}
109+
110+
if (conflicts.length === 0) {
111+
core.info('No other open PR declares these issues.');
112+
return;
113+
}
114+
115+
// First come, first served: only the LATER PR goes red. Failing
116+
// both would leave the original author red through no action of
117+
// their own; failing the earlier one would reward racing.
118+
const older = conflicts.filter((c) => c.other.number < pr.number);
119+
const newer = conflicts.filter((c) => c.other.number > pr.number);
120+
121+
for (const { other, shared } of newer) {
122+
core.info(
123+
`#${other.number} (newer) also declares ${shared.map((n) => `#${n}`).join(', ')} — ` +
124+
`it will fail its own run of this guard; this PR keeps its claim.`,
125+
);
126+
}
127+
128+
if (older.length > 0) {
129+
const lines = older.map(({ other, shared }) =>
130+
` - ${shared.map((n) => `#${n}`).join(', ')} is already claimed by #${other.number} ` +
131+
`(${other.html_url}, branch \`${other.head.ref}\`${other.draft ? ', draft' : ''})`,
132+
);
133+
core.setFailed(
134+
`Another open PR already declares a fix for the same issue(s):\n${lines.join('\n')}\n` +
135+
`If this PR is the duplicate, close it and add anything it uniquely covers to the ` +
136+
`earlier PR or a follow-up issue (that is what saved #4560 when #4559 was closed). ` +
137+
`If the EARLIER one is abandoned, close it first — this check re-runs on 'edited' ` +
138+
`and 'synchronize', and goes green once the conflict is gone.`,
139+
);
140+
}

0 commit comments

Comments
 (0)