Skip to content
Merged
5 changes: 5 additions & 0 deletions docs/cosmetic-backlog.md
Original file line number Diff line number Diff line change
Expand Up @@ -715,3 +715,8 @@
- [R202606b-ARCH-8] Scheduler 字段索引块与 per-field godoc 双份维护,必漂移 — internal/cron/scheduler.go:28-44
- [R202606b-PERF-016] prepareInbound 每条入站消息 slog.With 分配新 logger(owner 路径丢弃) — internal/dispatch/dispatch.go:658-661
- [R202606b-ARCH-7] internal/session/api 包仅 1 消费者(sysession),抽象成本>收益,可并入消费者 — internal/session/api/capabilities.go:1-42
- [R202606c-GO-003] recordBootStep Get→Register 非原子 TOCTOU(init 期单线程实际无并发 trigger) — internal/wireup/boot.go:96
- [R202606c-GO-005] selfupdate Checker.installed 无同步(单 goroutine Run 契约,无实际并发) — internal/selfupdate/checker.go:125
- [R202606c-ARCH-8] wireup requiredBootSteps 缺 schedulers,scheduler 注册漏掉不被 Validate 捕获 — internal/wireup/boot.go:112
- [R202606c-ARCH-5] attachment.Dir 是可变 package var 被 tracker 路径遍历安全守卫消费,应 const+WithDir — internal/attachment/store.go:43
- [R202606c-CR-006] sysession renameOne 截断 title 不加省略号,clip 中途断词 — internal/sysession/auto_titler.go:685
14 changes: 13 additions & 1 deletion internal/attachment/refcount_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -420,8 +420,10 @@ func TestUpdateMetaFile_AppliesMutation(t *testing.T) {
// synthesise upload metadata). Tracker callers must check this and
// log a warning, not crash.
func TestUpdateMetaFile_MissingSidecar(t *testing.T) {
dir := t.TempDir()
metaPath := filepath.Join(dir, "ghost.meta")
changed, err := UpdateMetaFile(
filepath.Join(t.TempDir(), "ghost.meta"),
metaPath,
func(m *Meta) bool { return true },
)
if err == nil {
Expand All @@ -430,4 +432,14 @@ func TestUpdateMetaFile_MissingSidecar(t *testing.T) {
if changed {
t.Errorf("changed=true for missing sidecar")
}
// [R202606c-CR-003] The error string must not echo the workspace /
// absolute meta path — package policy keeps operator-only paths out
// of error strings (the path travels separately to the Observer).
msg := err.Error()
if strings.Contains(msg, metaPath) || strings.Contains(msg, dir) {
t.Errorf("error leaks absolute path: %q", msg)
}
if strings.ContainsRune(msg, filepath.Separator) {
t.Errorf("error contains path separator (likely leaks a path): %q", msg)
}
}
16 changes: 10 additions & 6 deletions internal/attachment/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -471,7 +471,7 @@ func GCWithRefs(ctx context.Context, workspace string, opts GCOptions) (GCResult
continue
}
abs := filepath.Join(dayPath, name)
metaPath := metaPathFor(abs)
metaPath := MetaPathFor(abs)

keep, reason, err := shouldKeepAttachment(metaPath, dayTime, uploadCutoff, refCutoffMS)
if err != nil {
Expand Down Expand Up @@ -626,10 +626,12 @@ func loadMetaFile(path string) (*Meta, error) {
return &m, nil
}

// metaPathFor returns the sibling .meta path for an attachment
// MetaPathFor returns the sibling .meta path for an attachment
// payload file. Matches the layout Persist creates: strips the
// payload extension and appends ".meta".
func metaPathFor(absPayload string) string {
// payload extension and appends ".meta". Exported so the tracker
// shares one implementation instead of mirroring it (a divergence
// would silently split the .meta namespace between GC and tracker).
func MetaPathFor(absPayload string) string {
base := filepath.Base(absPayload)
if idx := strings.LastIndex(base, "."); idx > 0 {
return filepath.Join(filepath.Dir(absPayload), base[:idx]+".meta")
Expand Down Expand Up @@ -661,8 +663,10 @@ func UpdateMetaFile(metaPath string, mutate func(*Meta) bool) (bool, error) {
if m == nil {
// Legacy attachment with no meta; we cannot append references
// without inventing upload metadata, so we refuse rather than
// write a partial sidecar.
return false, fmt.Errorf("meta sidecar missing: %s", metaPath)
// write a partial sidecar. Keep the path out of the error
// string (package policy: workspace paths are operator-only).
// The caller already has metaPath for its OnMetaWriteError log.
return false, errors.New("meta sidecar missing")
}
if !mutate(m) {
return false, nil
Expand Down
24 changes: 9 additions & 15 deletions internal/attachment/tracker/tracker.go
Original file line number Diff line number Diff line change
Expand Up @@ -467,12 +467,17 @@ func (t *Tracker) handleBump(job trackerJob) {
t.pendingSize.Add(1)
continue
}
// Keep the highest timeMS observed and push the flushAt
// deadline out — coalesce semantics.
// Keep the highest timeMS observed, but DO NOT reset flushAt
// [R202606c-GO-002]: a key that is bumped faster than the
// coalesce window must still flush at its first deadline.
// Pushing flushAt out on every bump let a hot key starve
// forever (no .meta write until Stop/Flush), during which the
// GC reads stale on-disk meta and can delete a still-referenced
// attachment. Keep the original deadline so every key lands on
// disk within at most one CoalesceWindow of entering pending.
if job.timeMS > prev.timeMS {
prev.timeMS = job.timeMS
}
prev.flushAt = flushAt
t.pending[key] = prev
}
}
Expand Down Expand Up @@ -586,7 +591,7 @@ func (t *Tracker) flushAll() {
// at ERROR level to avoid log spam for legitimate churn (e.g. a
// file deleted between persist and bump).
func (t *Tracker) applyBump(key coalesceKey, bump pendingBump) {
metaPath := metaPathFor(key.absPath)
metaPath := attachment.MetaPathFor(key.absPath)
changed, err := attachment.UpdateMetaFile(metaPath, func(m *attachment.Meta) bool {
addedRef := m.AddReference(key.keyhash)
// Always advance LastReferencedAt to max(current, bump) —
Expand Down Expand Up @@ -643,14 +648,3 @@ func resolveAttachmentPath(workspace, p string) string {
}
return filepath.Join(workspace, cleaned)
}

// metaPathFor mirrors attachment.metaPathFor but is re-exported so
// tracker tests can round-trip without crossing the package
// boundary. Strip the payload extension, append .meta.
func metaPathFor(payload string) string {
base := filepath.Base(payload)
if idx := strings.LastIndex(base, "."); idx > 0 {
return filepath.Join(filepath.Dir(payload), base[:idx]+".meta")
}
return payload + ".meta"
}
88 changes: 88 additions & 0 deletions internal/attachment/tracker/tracker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -637,6 +637,94 @@ func TestStatsPendingNoRace(t *testing.T) {
wg.Wait()
}

// TestHandleBump_DoesNotDeferFlushDeadline (R202606c-GO-002) pins the
// fix for the starvation bug: a key bumped repeatedly faster than the
// coalesce window must still flush at its FIRST deadline, not have its
// flushAt pushed out on every bump. Before the fix, a hot key never
// reached flushDue until Stop/Flush, leaving its .meta stale on disk
// long enough for the GC to delete a still-referenced attachment.
//
// We drive handleBump directly on the test goroutine after stopping the
// worker (same pattern as the SecondSelectHonorsClose tests) so reads of
// t.pending are race-free, and inject a mutable clock so the deadline is
// deterministic.
func TestHandleBump_DoesNotDeferFlushDeadline(t *testing.T) {
ws := t.TempDir()
rel, _ := writeAttachment(t, ws, time.Now().Format("2006-01-02"), "hot",
attachment.Meta{UploadedAt: time.Now()})

var clockMu sync.Mutex
nowT := time.Unix(1_000_000, 0)
clock := func() time.Time {
clockMu.Lock()
defer clockMu.Unlock()
return nowT
}
advance := func(d time.Duration) {
clockMu.Lock()
defer clockMu.Unlock()
nowT = nowT.Add(d)
}

window := time.Second
tr, err := NewTracker(Options{
Workspaces: func(string) string { return ws },
CoalesceWindow: window,
ChannelBuffer: 64,
Clock: clock,
Observer: &countingObs{},
})
if err != nil {
t.Fatal(err)
}
// Stop the worker so we can drive handleBump synchronously and read
// t.pending without racing the run goroutine.
stopCtx, cancel := context.WithTimeout(context.Background(), 3*time.Second)
defer cancel()
if err := tr.Stop(stopCtx); err != nil {
t.Fatalf("Stop: %v", err)
}

abs := filepath.Join(ws, filepath.FromSlash(rel))
key := coalesceKey{keyhash: "kh", absPath: abs}

// First bump at T0 → flushAt should be T0+window.
tr.handleBump(trackerJob{kind: jobKindBump, keyhash: "kh",
absPaths: []string{rel}, timeMS: 100})
prev, ok := tr.pending[key]
if !ok {
t.Fatalf("key not pending after first bump")
}
wantDeadline := time.Unix(1_000_000, 0).Add(window)
if !prev.flushAt.Equal(wantDeadline) {
t.Fatalf("first flushAt=%v want %v", prev.flushAt, wantDeadline)
}

// Several more bumps, each advancing the clock by less than the
// window. The deadline must NOT move.
for i := 0; i < 5; i++ {
advance(window / 4)
tr.handleBump(trackerJob{kind: jobKindBump, keyhash: "kh",
absPaths: []string{rel}, timeMS: int64(101 + i)})
got := tr.pending[key]
if !got.flushAt.Equal(wantDeadline) {
t.Fatalf("flushAt moved on bump %d: got %v want %v (first deadline must hold)",
i, got.flushAt, wantDeadline)
}
// timeMS still tracks the latest observed value.
if want := int64(101 + i); got.timeMS != want {
t.Fatalf("timeMS=%d want %d", got.timeMS, want)
}
}

// Once the clock passes the first deadline, flushDue must drain it.
advance(window)
tr.flushDue()
if _, still := tr.pending[key]; still {
t.Fatalf("key still pending after deadline elapsed; flush was starved")
}
}

// TestPendingSize_TracksMapLen pins the invariant that pendingSize
// stays in sync with len(t.pending) across the three mutation
// paths (insert, update, delete). A divergence — e.g. forgetting
Expand Down
35 changes: 28 additions & 7 deletions internal/dashboard/project/files_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -199,17 +199,38 @@ func (h *Handlers) HandleFilesList(w http.ResponseWriter, r *http.Request) {
}

// Dirs first, then names (case-insensitive) so the listing reads like a
// file manager regardless of the OS readdir order.
sort.Slice(entries, func(i, j int) bool {
if entries[i].IsDir != entries[j].IsDir {
return entries[i].IsDir
}
return strings.ToLower(entries[i].Name) < strings.ToLower(entries[j].Name)
})
// file manager regardless of the OS readdir order. Pre-lowercase names once
// (paired with their entry) so the comparator avoids two strings.ToLower
// allocations per comparison — ~22k comparisons for a 2000-entry dir.
sortEntries(entries)

httputil.WriteJSON(w, map[string]any{
"dir": cleanDir,
"entries": entries,
"truncated": truncated,
})
}

// sortEntries orders entries dirs-first, then by case-insensitive name.
// It pre-computes the lowercased names once (paired with their entry so the
// pairing survives sort swaps) rather than calling strings.ToLower twice per
// comparison.
func sortEntries(entries []listEntry) {
type entryWithLower struct {
entry listEntry
lower string
}
paired := make([]entryWithLower, len(entries))
for i := range entries {
paired[i] = entryWithLower{entry: entries[i], lower: strings.ToLower(entries[i].Name)}
}
sort.Slice(paired, func(i, j int) bool {
if paired[i].entry.IsDir != paired[j].entry.IsDir {
return paired[i].entry.IsDir
}
return paired[i].lower < paired[j].lower
})
for i := range paired {
entries[i] = paired[i].entry
}
}
83 changes: 83 additions & 0 deletions internal/dashboard/project/files_list_sort_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
package project

import (
"sort"
"strings"
"testing"
)

// oldSortEntries is the original in-place comparator (two ToLower per compare).
// Kept here only to pin that the optimized sortEntries produces an identical
// ordering (R202606c-PERF-001).
func oldSortEntries(entries []listEntry) {
sort.Slice(entries, func(i, j int) bool {
if entries[i].IsDir != entries[j].IsDir {
return entries[i].IsDir
}
return strings.ToLower(entries[i].Name) < strings.ToLower(entries[j].Name)
})
}

func namesOf(entries []listEntry) []string {
out := make([]string, len(entries))
for i, e := range entries {
out[i] = e.Name
}
return out
}

func TestSortEntries_MatchesOldComparator(t *testing.T) {
t.Parallel()
mixed := []listEntry{
{Name: "Zebra.txt"},
{Name: "alpha", IsDir: true},
{Name: "Bravo", IsDir: true},
{Name: "apple.go"},
{Name: "APPLE.md"},
{Name: "charlie", IsDir: true},
{Name: "README"},
{Name: "readme.lower"},
{Name: "zeta.txt"},
{Name: ".hidden"},
{Name: ".Config", IsDir: true},
}

// Independent copies for each comparator.
want := make([]listEntry, len(mixed))
copy(want, mixed)
oldSortEntries(want)

got := make([]listEntry, len(mixed))
copy(got, mixed)
sortEntries(got)

wantNames := namesOf(want)
gotNames := namesOf(got)
if len(wantNames) != len(gotNames) {
t.Fatalf("length mismatch: want %d got %d", len(wantNames), len(gotNames))
}
for i := range wantNames {
if wantNames[i] != gotNames[i] {
t.Fatalf("order mismatch at %d: want %v, got %v", i, wantNames, gotNames)
}
}

// Sanity: all dirs precede all files.
seenFile := false
for _, e := range got {
if !e.IsDir {
seenFile = true
} else if seenFile {
t.Fatalf("dir %q appears after a file — dirs-first violated: %v", e.Name, gotNames)
}
}
}

func TestSortEntries_Empty(t *testing.T) {
t.Parallel()
var entries []listEntry
sortEntries(entries) // must not panic
if len(entries) != 0 {
t.Fatalf("expected empty, got %v", entries)
}
}
5 changes: 4 additions & 1 deletion internal/platform/weixin/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,10 @@ func (c *apiClient) post(ctx context.Context, endpoint string, body any) ([]byte
return nil, fmt.Errorf("read body: %w", err)
}
if resp.StatusCode != http.StatusOK {
return nil, fmt.Errorf("http %d: %s", resp.StatusCode, string(data))
// data is the raw iLink API response body and may contain C1/bidi/
// control bytes; sanitize before embedding in the error string so it
// cannot poison structured logs / terminal rendering at the caller.
return nil, fmt.Errorf("http %d: %s", resp.StatusCode, osutil.SanitizeForLog(string(data), 256))
}
return data, nil
}
Expand Down
Loading