Conversation
…inline URL adder, 📦 Things rename - Post-type badges are now additive toggles: Text is the always-on base; Photos, Marketplace, and Things each switch their field group on top without deselecting the others (clicking Text switches them all off). The stored crystal type is derived from the live toggles (things > marketplace > photos-with-visual-media > text), so the server vocabulary is unchanged. The media panel only shows when Photos is on. - Edit mode mounts the live upload panel again: PATCH /api/v1/things attachmentIds is upgraded from a pure-permutation reorder to a sync — the full desired display order that must cover every bound id (removals still rejected) and may append newly uploaded ready drafts, which are bound with the same owner/purpose fences create-time binding uses (plus an owner-fenced post-family target check so an edit can never deface another owner's thread). - The linked-image URL adder moves below the upload grid as a single inline input with an Add button: each valid URL becomes a grid tile and the field clears for the next one (Enter works, multi-URL paste splits). - 🌀 Thingtime badge renamed to 📦 Things (POST_TYPE_META, composer eyebrows, feed filter chips follow). Verified: attachments + things unit suites green (planAttachmentSync + sync-kind tests added), Vite client build green, live browser pass on the worktree dev stack at desktop + 375px mobile (toggle combos, URL adder, create, edit-add-linked-image, Things sheet). Upload binding on edit is unit-tested; local stack has no S3, so the upload-complete-then-save flow needs a preview/prod pass per TESTING.md. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…-visible pending; hidden-tolerant PATCH sync Round 2 of the composer overhaul (owner QA feedback on PR #496): - URL media now mints real linked-attachment things (POST /api/v1/attachments/link): attachmentLinked root marker, linked/<id> object key, zero object bytes, url-bearing crystal with a DECLARED render hint, ready at mint, moderation stamped 'skipped'. Accounting treats the shape as a closed variant (partial/forged combinations fail closed); only doc bytes hit quota; the server never fetches the URL. Duplicate URLs are deliberately allowed. Rate-limit row, import-map entry, and apiDocs entry (docs registry = Nitro registration + capability feature) included. - Lifecycle: cleanupClaimedDoc takes a lazy S3 getter and short-circuits linked docs straight to the transactional remove+refund, so delete/cancel/ reap/cascade/session-sweep work without S3; the content endpoint 302s linked ids to their external URL as a renderer fallback; the analyzer skips linked docs. - Composer: the add-by-URL input lives INSIDE the Media & files panel below the grid (Add button; clears per add; probe demotes extensionless URLs to file when they fail to load as images); linked entries share the uploads list so reorder/snapshot/markCommitted/remove behave identically; the panel stays usable before upload approval. Legacy crystal.images seed as local linked tiles on edit and migrate to linked attachments on save; new posts never write crystal.images. LinkedImageGallery removed from the composer. - Renderers (card gallery, lightbox, media page, reorder gallery, layout canvas) use crystal.url directly for linked media; linked file rows and downloads open the original URL in a new tab. - Moderation fixes for the vanished-image + edit-409 report: owners now see their own PENDING attachments (pending: true → "Checking…" badge; blocked stays hidden for all), and planAttachmentSync exempts moderation-hidden bound ids from the cover requirement, re-stamping them after the requested order inside the bind transaction. Verified: full test:unit green (new suites: linked crystal canonicalization, closed-union accounting, ownerView pending, hidden-tolerant sync, extension table client/server pin), build:client green, live E2E on the worktree stack (duplicates, pdf file-row + download, probe path, post render from external URLs, edit-add-URL save via PATCH sync, legacy 4-image migration, linked mint/delete with no S3 configured, desktop + 375px mobile). Detailed note in PRs/496-claude-post-editor-media-badges-7b2acd--*.md. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…atomic S3 failure, name round-trip, mint compensation, seed cap) Confirmed by the 12-agent adversarial review of a6409b8: - The content endpoint now 404s linked attachment ids instead of 302ing to crystal.url — the redirect made the first-party content URL an open redirect to attacker-chosen origins (CWE-601). Renderers always use crystal.url directly, so nothing needed the fallback. - cleanupClaimedDoc resolves S3 BEFORE the destructive deleting claim for non-linked docs (attachmentLinked is immutable, so the pre-claim doc is authoritative): an unconfigured/broken S3 fails atomically again instead of half-deleting a mixed linked+uploaded cascade and stranding uploaded docs in an endless deleting retry loop. Linked docs still need no S3. - linkedAttachmentNameForUrl re-validates after the 255-char slice (trim, control chars, well-formed unicode; falls back hostname → 'linked-media') so exotic basenames can no longer produce a non-canonical crystal that fails the mint. Round-trip pin test added. - A linked mint whose tile was removed mid-flight now fires a compensating delete instead of orphaning the draft until the 24h reap. - Legacy image seeds cap at the remaining attachment slots so a >25-media legacy post cannot 400-loop on every save. Accepted-by-design (documented in the PR note): linked mints skip the beta upload-approval gate and byte moderation — exact parity with the legacy crystal.images flow they replace; pre-hygiene URLs that fail today's canonicalizer drop on edit-save exactly as the old composer's client filter already did. Verified: test:attachments 134/134 green, lint green, build:client green, live check — content?id=<linked> now 404s while the card keeps rendering from the external URL. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…gallery-search # Conflicts: # remix/CHANGELOG.md
…gallery-search # Conflicts: # remix/CHANGELOG.md # remix/app/components/Feed/PostComposer.tsx
…7b2acd Composer: toggle type badges, edit-mode media adding, inline URL adder, 📦 Things rename
lopugit
added
the
no-ai-rebase
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
github-actions
Bot
added
lopu: mergeable
Lopu repository reviewLopu reviewed this PR against main as Thingtime's principal PR and repository manager. Using Claude Opus 5. Lopu found no justified local change to publish from this review pass. Lopu review — PR #516 ·
|
| Command | Result |
|---|---|
node --test scripts/graphify-cas.test.mjs on f31864b2 |
15/15 pass |
git diff main..develop -- . ':(exclude)graphify-out' |
2 files, 75 lines |
git merge-base --is-ancestor main develop |
true — clean fast-forward |
| all 85 PR checks | pass |
| CodeQL open alerts on this head | 0 — no dispositions written; 516.json left [] |
Changes made
None. The promotion is clean and safe to merge.
github-actions
Bot
added
lopu: unknown state
🤖 Lopu live PR updateStatus: Current phase: The detailed Lopu result explains the stopped phase Estimated completion: Done — no further active-work ETA. Time conversion (UTC source)
Los Angeles and Melbourne use their real IANA time zones, so PDT/PST and AEST/AEDT offsets change automatically. Lopu queue and PR pulse
Related PR context
Exact branch pair: Timeline
Technical run details — optional; this comment is the human-facing source of truth. |
|
Residual conflicted files:
|
github-actions
Bot
added
lopu: queued
|
Residual conflicted files:
|
🤖 Lopu detected an out-of-date PR branchStatus: Work detected — Lopu is taking ownership. Current phase: Entering Lopu's serialized PR-resolution queue. Estimated completion: around 11:01 UTC (~20 minutes; this adjusts as the queue moves). Next automatic check-in: within 10 minutes while work remains active. You can stay on this PR; there is no need to find the Actions run. Time conversion (UTC source)
Los Angeles and Melbourne use their real IANA time zones, so PDT/PST and AEST/AEDT offsets change automatically. Lopu queue and PR pulse
Related PR context
Exact branch pair: Timeline
|
|
Lopu review — this promotion carries no product change. The full tree difference between No application code, no docs, no config, no tests. The 2.35M-line diffstat is entirely content-addressed Graphify cache and snapshot artifacts; the only three deletions are So The generated artifacts are correctly marked Checks: One observation at the repository-direction level, not a request: committing ~47 MB of regenerated Graphify output on every branch is what makes each PR in this batch look enormous and makes review diffstats unreadable. It's clearly a deliberate convention — |
🤖 Lopu detected an out-of-date PR branchStatus: Work detected — Lopu is taking ownership. Current phase: Entering Lopu's serialized PR-resolution queue. Estimated completion: around 11:08 UTC (~20 minutes; this adjusts as the queue moves). Next automatic check-in: within 10 minutes while work remains active. You can stay on this PR; there is no need to find the Actions run. Time conversion (UTC source)
Los Angeles and Melbourne use their real IANA time zones, so PDT/PST and AEST/AEDT offsets change automatically. Lopu queue and PR pulse
Related PR context
Exact branch pair: Timeline
|
|
🦄 Lopu review — this promotion carries no product change; it's pure Graphify snapshot Targeting The 2.35M reported additions read alarming, so worth stating plainly: Every product change has already been promoted. What remains is exactly the tracked ℹ️ One
|
|
Lopu review — this promotion would have tripled All checks pass and the merge is clean, so this is not a CI failure. The problem is in what the promotion carries. Although the changelog lists four PRs,
Root causeThe retention logic is fine — Snapshots are content-addressed at Confirmed by history: all three snapshots entered Why no test caught it
Changes on this branch
No production logic changed; the retention implementation is correct and editing it would have been churn. Durable fix (separate PR)The real gap is in the controller, so it is not on this branch. Validation
No correctness, security, crash, or data/API-compatibility issues in the promoted content — the delta contains no source changes at all. 🤖 Posted by Lopu, Thingtime's PR manager.
|
|
📋 Promotion changelog updated — now carrying 5 PRs (33 commits). Added:
The full changelog lives in the PR description. |
🤖 Lopu live PR updateStatus: ✅ Lopu finished — this PR is mergeable Current phase: GitHub verified the published branch result Estimated completion: Done — no further active-work ETA. Time conversion (UTC source)
Los Angeles and Melbourne use their real IANA time zones, so PDT/PST and AEST/AEDT offsets change automatically. Lopu queue and PR pulse
Related PR context
Exact branch pair: Timeline
Technical run details — optional; this comment is the human-facing source of truth. |
|
Lopu review — found the durable root cause of the Graphify snapshot growth: it's in the stager, and it's fixed on No failing checks (16 pass / 23 skipping / 9 queued;
|
| Ref | Snapshots | Tracked bytes under graphify-out/snapshots |
|---|---|---|
main @ 53c461c7 |
1 | 47,470,533 |
develop @ 7ac74ecd (after that manual prune) |
1 | 46,232,636 |
main @ 4adda985 (this PR's base, now) |
26 | 1,129,716,437 |
develop @ 96b352f8 (this PR's head) |
26 | 1,128,478,540 |
4adda985 merges fc5b14d3 (1 snapshot) with b34043d3 (26) — the feature-stack branch promoted by #566. The regression re-entered through the feature-stack promotion, not the develop sync. And sync-main-into-develop.yml has no Graphify step at all, so a fix there would have done nothing.
Root cause
Retention has been correct in the router and dead at staging. graphify-cas.mjs prunes superseded trees from the worktree; .github/scripts/stage-graphify-snapshots.mjs then decides what reaches the commit, and it only ever walked files that still exist:
const snapshots = filesUnder(path.join(output, "snapshots", "v1")).filter(...)
addExisting(root, [...snapshots, ...semantic], { force: true })A pruned tree isn't in that list, so it's never passed to git add and the deletion is never staged. Every controller-authored Graphify commit re-tracked exactly what the router had just pruned, and left the removals as unstaged worktree deletions.
Reproduced on a scratch repo — two tracked snapshot trees, one removed as pruneSnapshots would:
before fix: controller commit still tracks 5/5 files of the pruned tree, 5 unstaged deletions left behind
after fix: pruned tree 0 files, retained tree 5 files, `git status -- graphify-out` clean
That's why manual prunes keep working once and evaporating. The bound could never land in Git.
Fixed in the protected control plane, not here — per the control-plane rule it's staged in the trusted checkout for its own PR against github-actions. Scoped to graphify-out/snapshots/v1 only (the semantic CAS is deliberately never pruned), and fail-closed: if zero portable snapshot files remain, it stages nothing, because the router always activates before pruning — an empty namespace means a failed build, not a legitimate prune. I extended the stager's --self-test to cover it and verified the new test fails when the fix is reverted (actual: '' against the expected D entry).
Changes on this branch
Small, both about the test this PR adds:
-
The new test was coupled to ambient operator config.
snapshotRetentionLimit()defaults its argument toprocess.env.GRAPHIFY_SNAPSHOT_RETENTION, and the test called it bare. Under the documented override it asserted something different:GRAPHIFY_SNAPSHOT_RETENTION=2 node --test scripts/graphify-cas.test.mjs not ok 5 - snapshot retention defaults to one and rejects unsafe values not ok 15 - a merge unions branch snapshots and pruning restores the retention boundThe suite now clears the variable once after its imports. That keeps test 5's intent intact — it exists to assert the default, so pinning an explicit value would have defeated it.
-
The suite never ran in CI.
scripts/graphify-cas.test.mjswasn't reachable from any job, so this PR was adding a test that couldn't gate anything.remix'stest:unitalready pulls in the sibling root suite viatest:vercel-root→node --test ../scripts/vercel-root-flow.test.mjs; addedtest:graphify-casthe same way and wired it in.
remix/package.json | 3 ++-
scripts/graphify-cas.test.mjs | 6 ++++++
No production logic touched — the retention implementation is correct and editing it would be churn.
Why I did not prune this branch
I did this last round when main was healthy and the prune protected it. It's a different call now: main is already at 26, so pruning here means folding a ~1 GB deletion across 25 snapshot trees into a promotion you merge. Until the staging fix is live that's a treadmill — the next feature-stack promotion re-inflates it. Ordering matters: land the controller fix, then one publish restores the bound and it sticks. Widening a routine review into that deletion is your call, not mine, so I've flagged it instead of doing it.
Worth noting this promotion doesn't regress anything: the head is ~928 KB smaller than its base, a 1-for-1 snapshot replacement. All 26 committed snapshots validate as well-formed records.
Validation
node --test scripts/graphify-cas.test.mjs→ 15/15, and 15/15 atGRAPHIFY_SNAPSHOT_RETENTION= unset,2,7,0,all(was 13/15 at2)npm run test:graphify-casfromremix/→ 15/15, confirming the wiring resolves from CI's working directory- Snapshot store integrity via the router's own
listSnapshots/selectSnapshot: 26/26 valid - Control plane: stager
--self-testOK and proven to fail without the fix;graphify-cas20/20;workflow-control-plane-contract.mjs --self-testOK;git diff --checkclean
One more thing
.github/scripts/graphify-cas.mjs and product scripts/graphify-cas.mjs have drifted — 277 differing lines, with the controller copy carrying --exclude scope normalization and 20 tests against the product copy's 15. Not touched here, but worth a deliberate reconciliation before the two diverge further.
🤖 Posted by Lopu, Thingtime's PR manager.
|
Lopu review — Only two real files hereThe
The
So an operator's exported override really would have silently redefined the retention assertions. Good catch by whoever wrote it.
|
🤖 Lopu — review of
|
| File | Change |
|---|---|
scripts/graphify-cas.test.mjs |
new — a 15-test suite for the graphify CAS module |
remix/package.json |
registers test:graphify-cas and threads it into test:unit |
Everything else in the range is main→develop sync merges and feature work main already carries.
Findings
No defects. The wiring is right — node --test ../scripts/graphify-cas.test.mjs resolves correctly from remix/, scripts/graphify-cas.mjs exists at the promoted head, and the entry sits in its alphabetical slot in the test:unit chain.
The suite is substantive rather than decorative: the source fingerprint excludes graphify-out and leaves the caller's staged index untouched, content-addressed variants coexist and dedupe byte-identically, an existing artifact path rejects changed bytes, a >50% node collapse fails closed, retention rejects 0/1.5/all, a live writer serializes the next one, and merges union branch snapshots while pruning restores the bound. It also deletes an ambient GRAPHIFY_SNAPSHOT_RETENTION up front rather than asserting against whatever the caller exported — the right instinct for a policy-sensitive suite.
No API, schema, or data-shape change; nothing to migrate; no new endpoints, permissions, or secrets. All checks green, mergeStateStatus: CLEAN. CodeQL: no open alerts on this head.
Validation: node --test scripts/graphify-cas.test.mjs in the PR worktree → 15/15 pass.
One operational note, not blocking
The promoted tree carries 26 portable graph snapshots against a documented default retention of 1 — the accumulation this PR's own new test describes: "a merge unions branch snapshots and pruning restores the retention bound." Each merge unions both sides and nothing re-applies the bound afterwards.
That has a live cost, and I hit it today. On #485 the same accumulation pushes the diff past GitHub's diff generator, which returns HTTP 422 — and that took the repository-wide merge-conflict detector down for every open PR (root cause was a stdout/stderr leak in the control plane; diagnosed and fixed separately on github-actions). A scripts/graphify prune in the canonical environment would collapse the tree back toward the bound. Worth doing before it grows further, but it does not block this promotion.
— Lopu 🌸
🤖 Lopu — review of
|
| Run | Result |
|---|---|
GRAPHIFY_SNAPSHOT_RETENTION=3, guard present |
15 pass / 0 fail |
GRAPHIFY_SNAPSHOT_RETENTION=3, guard stripped |
13 pass / 2 fail |
Why I did not prune the 26 snapshots here
The promoted tree carries 26 portable snapshots against a default retention of 1 — 1,130,880,398 bytes across 996 files, 91% of a 1,184 MB checkout. Confirmed with the repository's own code, read-only: valid snapshot records: 26, would prune: 25.
It's real, but it is not caused by this PR — main @ 4adda985 carries the same 26 and this promotion is net-neutral (one swapped, 56559a02 → 673aee29). More to the point, pruning now wouldn't hold: until the stager fix lands, the next controller Graphify commit re-tracks everything the prune removed. Sequencing matters — land the control-plane fix, then run scripts/graphify prune once in the canonical environment, and for the first time that removal will survive.
CodeQL: no open alerts on this head. Checks: 36 pass / 49 skipping / 0 failing, mergeStateStatus: CLEAN.
— Lopu 🌸
🤖 Lopu — closing the loop on
|
|
🤖 Lopu review — Verdict: green, no changes needed. 36 checks pass, 0 fail, merge state The 51-file / ±280k diff is almost entirely regenerated Graphify output. The whole hand-written delta is two files: wiring Two things I liked: the new test builds real git branches with distinct source fingerprints instead of mocking, and the suite now 🧹 One repository-health note, for a separate PRNot introduced here — this merge is net-neutral (one snapshot swapped out, one in) — but this PR's new test names the exact mechanism, so it seems like the right moment to raise it:
I deliberately did not fix it here. Pruning means deleting ~1 GB of tracked files from |
|
🤖 Lopu review — Verdict: merge it. 36 checks pass, 0 fail, Two corrections to my earlier rounds on this head1. The unit job did run. My 15:28 comment said the Web-CI scope classifier skipped the full unit job, so nothing in CI exercised the new suite. Wrong — run The 2. The snapshot growth is not "merges don't re-apply the bound". That same comment attributed 26-snapshots-against-a-bound-of-1 to merges unioning the sets. Merges do union them, but a single
That is #574's subject and it is already 🔎 New finding: the snapshot cache key can never hitChasing the retention thread turned up something separate, and I don't think it's been raised yet. 0 of 26 committed snapshots record a So Cause.
Fixed in the control plane, not here. A Product branches carry their own copy at 📋 The changelog table oversells this promotionThe body lists 5 PRs that "will land in That's the documented contract ( What I checked on the PR itselfBoth hand-written files are correct and I verified the parts that could have been cargo-culted:
Merge order that makes the ~1.05 GiB backlog resolve itself: #574 first, then this. The next controller Graphify run then prunes to the bound and, for the first time, commits it. 🤖 Posted by Lopu, Thingtime's PR manager. |
|
🌸 Lopu — batch review note (conflict-batch Reviewed and clean. Underneath 49 files of
One thing worth an explicit decision before this promotes. Measured on the
Retention is 1. This promotion doesn't make it worse — both sides are already Divergence worth knowing: product branches carry no No CodeQL alerts on this head, so no dispositions were written. |
2 participants
Standing promotion PR opened by the Promote develop to main workflow. Its head is
develop, so every new push or merge todevelopshows up here on its own. Merge it whenever main should catch up — the workflow opens the next one after the following push todevelop. The Sync main into develop workflow levels develop with main again after each promotion.📋 What this promotion carries
5 pull requests merged into
develop(37 commits) will land inmainwhen this PR merges — newest first:codex/thingtime-chatgpt-pluginsync/main-into-developsync/main-into-developcodex/feature-stack-run-id-developclaude/post-editor-media-badges-7b2acdDirect commits on `develop` without a merged PR (19)
96b352f8Merge remote-tracking branch 'origin/main' into develop7ac74ecdMerge remote-tracking branch 'origin/main' into developbf9ed9deMerge remote-tracking branch 'origin/main' into developdc052c6bMerge remote-tracking branch 'origin/main' into develop364769a7Merge remote-tracking branch 'origin/main' into developf813fb9fMerge remote-tracking branch 'origin/main' into develop701faeb6Merge remote-tracking branch 'origin/main' into developbdaf7cb5Merge remote-tracking branch 'origin/main' into develope975b8f9Merge remote-tracking branch 'origin/main' into develop7cb88b07Merge remote-tracking branch 'origin/main' into developdcdd7491Merge remote-tracking branch 'origin/main' into developff767ccdMerge remote-tracking branch 'origin/main' into develop88bd5d05Merge remote-tracking branch 'origin/main' into developec5b3e51Merge remote-tracking branch 'origin/main' into develop98c66b07Merge remote-tracking branch 'origin/main' into developd1971e10Merge remote-tracking branch 'origin/main' into developdfafac87Merge remote-tracking branch 'origin/main' into develop0d54ee2bMerge remote-tracking branch 'origin/main' into develop27e4c854Merge pull request fix(graphify): bound portable snapshot retention #522 from lopugit/codex/graphify-bounded-snapshotsAuto-maintained by the Promote develop to main workflow — refreshed from
develop@f31864b2(2026-09-01). Delta comments below track when entries enter or leave the promotion window.