From a71cf5b2264256e8780d153015513e83d368ca4c Mon Sep 17 00:00:00 2001 From: Naman-Nirwan Date: Tue, 8 Sep 2026 12:34:41 +0530 Subject: [PATCH 1/6] fix for the back in the logviewer, wantBack flag added --- modules/pipeline/logviewer.go | 10 ++++++++-- pkg/cmdctx/cmdctx.go | 6 ++++++ pkg/registry/uitableview.go | 8 ++++++-- 3 files changed, 20 insertions(+), 4 deletions(-) diff --git a/modules/pipeline/logviewer.go b/modules/pipeline/logviewer.go index 20b158e..fe7da6c 100644 --- a/modules/pipeline/logviewer.go +++ b/modules/pipeline/logviewer.go @@ -408,6 +408,12 @@ func (m logViewModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { ss.cancel() } return m, tea.Quit + case "b": + for _, ss := range m.activeStreams { + ss.cancel() + } + m.ctx.UIWantBack = true + return m, tea.Quit case "s": if m.state == lvStateReady && m.selectedUUID != "" { node := m.selectedNode() @@ -875,9 +881,9 @@ func (m logViewModel) renderSplit(b *strings.Builder) { } // help line: left side is fixed, right side shows poll state / scroll % - helpLeft := " ↑/↓ select · l/d/i/o/tab tab · pgup/pgdn scroll · r refresh · q quit" + helpLeft := " ↑/↓ select · l/d/i/o/tab tab · pgup/pgdn scroll · r refresh · b back · q quit" if m.activeTab == tabLogs || m.activeTab == tabInputs || m.activeTab == tabOutputs { - helpLeft = " ↑/↓ select · l/d/i/o/tab tab · pgup/pgdn scroll · r refresh · s save · q quit" + helpLeft = " ↑/↓ select · l/d/i/o/tab tab · pgup/pgdn scroll · r refresh · s save · b back · q quit" } var helpRight string diff --git a/pkg/cmdctx/cmdctx.go b/pkg/cmdctx/cmdctx.go index 52ecdbb..265bec6 100644 --- a/pkg/cmdctx/cmdctx.go +++ b/pkg/cmdctx/cmdctx.go @@ -240,6 +240,12 @@ type Ctx struct { // RestoreListPos is the cursor row (within the first-loaded page) to seed a // freshly built table with when replaying a popped UILink's ListPos. RestoreListPos int + // UIWantBack is read by finishUIExit right after a "view" ui_command's + // handler returns, to decide whether to pop UIHistory and redraw the caller + // (true) or quit outright (false, the default). A view handler that wants + // its screen's own "b" key to behave like the rest of --ui's back + // navigation must set this to true before returning — it is not automatic. + UIWantBack bool } // ScopedAuth returns Auth adjusted for Level: "org" clears ProjectID, "account" diff --git a/pkg/registry/uitableview.go b/pkg/registry/uitableview.go index 3bfc428..5a90936 100644 --- a/pkg/registry/uitableview.go +++ b/pkg/registry/uitableview.go @@ -1310,8 +1310,12 @@ func finishUIExit(ctx *cmdctx.Ctx, fm uiTableModel) error { if err := ctx.Resolver.RunUIHandler(ctx, fm.launchUIHandlerFn); err != nil { return err } - if link, ok := ctx.PopUILink(); ok { - return dispatchLink(ctx, &link) + // UIWantBack defaults to false: unless the handler explicitly asked to go + // back (e.g. its own "b" key), quitting its screen quits outright. + if ctx.UIWantBack { + if link, ok := ctx.PopUILink(); ok { + return dispatchLink(ctx, &link) + } } return nil } From a549dae9aa7fd67fab44781908c225f6b079622c Mon Sep 17 00:00:00 2001 From: Naman-Nirwan Date: Tue, 8 Sep 2026 12:48:38 +0530 Subject: [PATCH 2/6] Unit tests updated for the changes --- pkg/registry/uitableview_test.go | 37 ++++++++++++++++++++++++++++---- 1 file changed, 33 insertions(+), 4 deletions(-) diff --git a/pkg/registry/uitableview_test.go b/pkg/registry/uitableview_test.go index 817e40a..f5b5350 100644 --- a/pkg/registry/uitableview_test.go +++ b/pkg/registry/uitableview_test.go @@ -235,10 +235,12 @@ func TestFinishUIExit_ViewHop_ResumesLeftScreen(t *testing.T) { launchUIId: "child-1", launchUIHandlerFn: "noop_handler", } - // The handler pushes the screen it's leaving, runs "noop_handler" (returns nil), then - // pops that same entry back off to resume it via dispatchLink — which doesn't resolve - // against an empty Registry; the resulting error is expected and irrelevant here. Only - // the net stack effect (resume, not leak or exit) is under test. + // The handler pushes the screen it's leaving, runs "noop_handler" (returns nil), then sets + // ctx.UIWantBack (as a view handler's own "b" key would) so finishUIExit pops that same + // entry back off to resume it via dispatchLink — which doesn't resolve against an empty + // Registry; the resulting error is expected and irrelevant here. Only the net stack effect + // (resume, not leak or exit) is under test. + ctx.UIWantBack = true _ = finishUIExit(ctx, fm) if len(ctx.UIHistory) != 1 || ctx.UIHistory[0].Id != "prev-id" { @@ -246,6 +248,33 @@ func TestFinishUIExit_ViewHop_ResumesLeftScreen(t *testing.T) { } } +func TestFinishUIExit_ViewHop_QuitOnlyByDefault(t *testing.T) { + r := New() + r.RegisterWorkflow("noop_handler", func(*cmdctx.Ctx) error { return nil }) + ctx := &cmdctx.Ctx{ + Verb: VerbGet, + Noun: "thing", + Id: "child-1", + Resolver: r, + UIHistory: []cmdctx.UILink{{Verb: VerbGet, Noun: "thing", Id: "prev-id"}}, + } + fm := uiTableModel{ + detailOnly: true, + launchUIId: "child-1", + launchUIHandlerFn: "noop_handler", + } + // The handler returns nil without setting ctx.UIWantBack (the default), so the view + // hop's own push is left in place and there is no pop/resume — quitting the handler's + // screen quits outright rather than resuming the caller. + if err := finishUIExit(ctx, fm); err != nil { + t.Fatalf("finishUIExit: %v", err) + } + + if len(ctx.UIHistory) != 2 { + t.Fatalf("UIHistory len = %d, want 2 (view hop pushed, no pop without UIWantBack)", len(ctx.UIHistory)) + } +} + func TestFinishUIExit_NoHopNoPush(t *testing.T) { ctx := &cmdctx.Ctx{Verb: VerbList, Noun: "thing", Resolver: New()} fm := uiTableModel{} From 4d4a85a7721aee3d7e88956299de61d91dc919f3 Mon Sep 17 00:00:00 2001 From: Naman-Nirwan Date: Tue, 8 Sep 2026 13:28:24 +0530 Subject: [PATCH 3/6] fix the page offset problem, working e2e --- pkg/cmdctx/cmdctx.go | 4 ++++ pkg/cmdctx/uilink.go | 1 + pkg/registry/buildctx.go | 1 + pkg/registry/uitableview.go | 4 +++- pkg/registry/uitableview_test.go | 36 ++++++++++++++++++++++++++++++++ 5 files changed, 45 insertions(+), 1 deletion(-) diff --git a/pkg/cmdctx/cmdctx.go b/pkg/cmdctx/cmdctx.go index 265bec6..3b1cf90 100644 --- a/pkg/cmdctx/cmdctx.go +++ b/pkg/cmdctx/cmdctx.go @@ -240,6 +240,10 @@ type Ctx struct { // RestoreListPos is the cursor row (within the first-loaded page) to seed a // freshly built table with when replaying a popped UILink's ListPos. RestoreListPos int + // RestorePage is the page number to fetch first when replaying a popped + // UILink's Page, so RestoreListPos is applied against the same page the + // cursor was originally captured on. + RestorePage int // UIWantBack is read by finishUIExit right after a "view" ui_command's // handler returns, to decide whether to pop UIHistory and redraw the caller // (true) or quit outright (false, the default). A view handler that wants diff --git a/pkg/cmdctx/uilink.go b/pkg/cmdctx/uilink.go index fa52e6d..4ce5c99 100644 --- a/pkg/cmdctx/uilink.go +++ b/pkg/cmdctx/uilink.go @@ -30,6 +30,7 @@ type UILink struct { Screen UIScreenKind ListPos int + Page int } // PushUILink appends link to the back-navigation stack (LIFO — PopUILink pops diff --git a/pkg/registry/buildctx.go b/pkg/registry/buildctx.go index b2ca474..1953a3e 100644 --- a/pkg/registry/buildctx.go +++ b/pkg/registry/buildctx.go @@ -381,6 +381,7 @@ func buildLinkCtx(ctx *cmdctx.Ctx, link *cmdctx.UILink, targetCs *spec.CommandSp } else { newCtx.ParentId = link.Id newCtx.RestoreListPos = link.ListPos + newCtx.RestorePage = link.Page } return newCtx, nil } diff --git a/pkg/registry/uitableview.go b/pkg/registry/uitableview.go index 5a90936..5cbbd00 100644 --- a/pkg/registry/uitableview.go +++ b/pkg/registry/uitableview.go @@ -184,6 +184,7 @@ func newUITableModel( hasSearch: hasSearch, getCs: getCs, uiCommands: uiCommands, + page: ctx.RestorePage, restoreCursor: ctx.RestoreListPos, } } @@ -345,7 +346,7 @@ func (m uiTableModel) Init() tea.Cmd { if m.detailOnly { return m.fetchDetail(m.detail.id) } - return m.fetchPage(0) + return m.fetchPage(m.page) } func (m uiTableModel) fetchPage(page int) tea.Cmd { @@ -1283,6 +1284,7 @@ func currentScreenLink(ctx *cmdctx.Ctx, fm uiTableModel) cmdctx.UILink { // "b" always resumes the list, never the detail overlay, and detailOnly // screens (Case 4) never populate fm.t, so its Cursor() is a natural 0 there. ListPos: fm.t.Cursor(), + Page: fm.page, } return link } diff --git a/pkg/registry/uitableview_test.go b/pkg/registry/uitableview_test.go index f5b5350..5d1c976 100644 --- a/pkg/registry/uitableview_test.go +++ b/pkg/registry/uitableview_test.go @@ -169,6 +169,17 @@ func TestCurrentScreenLink_DetailOnlyScreenHasZeroListPos(t *testing.T) { } } +func TestCurrentScreenLink_CapturesPage(t *testing.T) { + ctx := &cmdctx.Ctx{Verb: VerbList, Noun: "thing", ParentId: "parent-1"} + table := tui.NewTable(nil, 5, 40) + fm := uiTableModel{t: table, page: 2} + + link := currentScreenLink(ctx, fm) + if link.Page != 2 { + t.Fatalf("Page = %d, want 2", link.Page) + } +} + func TestCurrentScreenLink_CapturesSearchTermIntoFlagValues(t *testing.T) { ctx := &cmdctx.Ctx{Verb: VerbList, Noun: "thing", ParentId: "parent-1", FlagValues: map[string]any{"other": "x"}} table := tui.NewTable(nil, 5, 40) @@ -303,6 +314,20 @@ func TestBuildLinkCtx_TableScreen_CarriesListPosToRestoreListPos(t *testing.T) { } } +func TestBuildLinkCtx_TableScreen_CarriesPageToRestorePage(t *testing.T) { + ctx := &cmdctx.Ctx{Context: context.Background(), Resolver: New()} + link := &cmdctx.UILink{Verb: VerbList, Noun: "thing", Id: "parent-1", Screen: cmdctx.ScreenTable, ListPos: 4, Page: 2} + targetCs := &spec.CommandSpec{Verb: VerbList, Noun: "thing", NoAuth: true} + + newCtx, err := buildLinkCtx(ctx, link, targetCs) + if err != nil { + t.Fatalf("buildLinkCtx: %v", err) + } + if newCtx.RestorePage != 2 { + t.Fatalf("RestorePage = %d, want 2", newCtx.RestorePage) + } +} + func TestBuildLinkCtx_DetailScreen_DoesNotSetRestoreListPos(t *testing.T) { ctx := &cmdctx.Ctx{Context: context.Background(), Resolver: New()} link := &cmdctx.UILink{Verb: VerbGet, Noun: "thing", Id: "child-1", Screen: cmdctx.ScreenDetailForGet, ListPos: 4} @@ -317,6 +342,17 @@ func TestBuildLinkCtx_DetailScreen_DoesNotSetRestoreListPos(t *testing.T) { } } +func TestNewUITableModel_SeedsPageFromCtxRestorePage(t *testing.T) { + ctx := &cmdctx.Ctx{RestorePage: 2, RestoreListPos: 4} + m := newUITableModel(ctx, nil, nil, nil, nil, "title", 80, 24, nil) + if m.page != 2 { + t.Fatalf("page = %d, want 2 (seeded from ctx.RestorePage)", m.page) + } + if m.restoreCursor != 4 { + t.Fatalf("restoreCursor = %d, want 4 (seeded from ctx.RestoreListPos)", m.restoreCursor) + } +} + func TestApplyPage_FirstLoadRestoresCursor_SubsequentLoadsGotoTop(t *testing.T) { rows := make([]tui.Row, 10) for i := range rows { From 01485b813fe8fe77f13c08784494bef28fc508e5 Mon Sep 17 00:00:00 2001 From: Naman-Nirwan Date: Wed, 9 Sep 2026 00:56:45 +0530 Subject: [PATCH 4/6] Added the offset inplace of the page and listpos --- pkg/cmdctx/cmdctx.go | 12 ++--- pkg/cmdctx/uilink.go | 8 +-- pkg/registry/buildctx.go | 3 +- pkg/registry/uitableview.go | 17 +++--- pkg/registry/uitableview_test.go | 91 ++++++++++++++++++-------------- 5 files changed, 72 insertions(+), 59 deletions(-) diff --git a/pkg/cmdctx/cmdctx.go b/pkg/cmdctx/cmdctx.go index 3b1cf90..90bc7af 100644 --- a/pkg/cmdctx/cmdctx.go +++ b/pkg/cmdctx/cmdctx.go @@ -237,13 +237,11 @@ type Ctx struct { // UIHistory is the --ui back-navigation stack: one UILink pushed per Hop // (link/up/view), popped by the "b" key. Session-lifetime only. UIHistory []UILink - // RestoreListPos is the cursor row (within the first-loaded page) to seed a - // freshly built table with when replaying a popped UILink's ListPos. - RestoreListPos int - // RestorePage is the page number to fetch first when replaying a popped - // UILink's Page, so RestoreListPos is applied against the same page the - // cursor was originally captured on. - RestorePage int + // RestoreOffset is the absolute row index to restore when replaying a + // popped UILink's Offset. It is resolved against the current pageSize + // (page = RestoreOffset / pageSize, cursor = RestoreOffset % pageSize), + // since the terminal may have been resized since it was captured. + RestoreOffset int // UIWantBack is read by finishUIExit right after a "view" ui_command's // handler returns, to decide whether to pop UIHistory and redraw the caller // (true) or quit outright (false, the default). A view handler that wants diff --git a/pkg/cmdctx/uilink.go b/pkg/cmdctx/uilink.go index 4ce5c99..4d100dc 100644 --- a/pkg/cmdctx/uilink.go +++ b/pkg/cmdctx/uilink.go @@ -28,9 +28,11 @@ type UILink struct { Profile, Org, Project string FlagValues map[string]any - Screen UIScreenKind - ListPos int - Page int + Screen UIScreenKind + // Offset is the absolute row index (page*pageSize + cursor row) the user + // was on when this Link was captured — resize-safe, unlike a raw page + // number or in-page cursor alone. + Offset int } // PushUILink appends link to the back-navigation stack (LIFO — PopUILink pops diff --git a/pkg/registry/buildctx.go b/pkg/registry/buildctx.go index 1953a3e..24e3176 100644 --- a/pkg/registry/buildctx.go +++ b/pkg/registry/buildctx.go @@ -380,8 +380,7 @@ func buildLinkCtx(ctx *cmdctx.Ctx, link *cmdctx.UILink, targetCs *spec.CommandSp } } else { newCtx.ParentId = link.Id - newCtx.RestoreListPos = link.ListPos - newCtx.RestorePage = link.Page + newCtx.RestoreOffset = link.Offset } return newCtx, nil } diff --git a/pkg/registry/uitableview.go b/pkg/registry/uitableview.go index 5cbbd00..e39f686 100644 --- a/pkg/registry/uitableview.go +++ b/pkg/registry/uitableview.go @@ -134,8 +134,8 @@ type uiTableModel struct { width int height int - // listpos restore — seeded from ctx.RestoreListPos, applied once on the - // first page load only (later page loads always GotoTop as before). + // cursor restore — derived from ctx.RestoreOffset (against pageSize), applied + // once on the first page load only (later page loads always GotoTop as before). restoreCursor int restoreApplied bool } @@ -156,6 +156,8 @@ func newUITableModel( getCs *spec.CommandSpec, ) uiTableModel { pageSize := tableHeight(termHeight) + page := ctx.RestoreOffset / pageSize + restoreCursor := ctx.RestoreOffset % pageSize colDefs := placeholderColumns(tspec, termWidth) t := tui.NewTable(colDefs, pageSize, termWidth) @@ -184,8 +186,8 @@ func newUITableModel( hasSearch: hasSearch, getCs: getCs, uiCommands: uiCommands, - page: ctx.RestorePage, - restoreCursor: ctx.RestoreListPos, + page: page, + restoreCursor: restoreCursor, } } @@ -1279,12 +1281,11 @@ func currentScreenLink(ctx *cmdctx.Ctx, fm uiTableModel) cmdctx.UILink { Org: org, Project: project, FlagValues: fv, - Screen: screen, - // ListPos is always captured from the underlying table, even mid detail-flip: + Screen: screen, + // Offset is always captured from the underlying table, even mid detail-flip: // "b" always resumes the list, never the detail overlay, and detailOnly // screens (Case 4) never populate fm.t, so its Cursor() is a natural 0 there. - ListPos: fm.t.Cursor(), - Page: fm.page, + Offset: fm.page*fm.pageSize + fm.t.Cursor(), } return link } diff --git a/pkg/registry/uitableview_test.go b/pkg/registry/uitableview_test.go index 5d1c976..55eb921 100644 --- a/pkg/registry/uitableview_test.go +++ b/pkg/registry/uitableview_test.go @@ -121,7 +121,7 @@ func TestFinishUIExit_PushesLinkOnViewHop(t *testing.T) { } } -func TestCurrentScreenLink_CapturesListPosOnTable(t *testing.T) { +func TestCurrentScreenLink_CapturesOffsetOnTable(t *testing.T) { ctx := &cmdctx.Ctx{Verb: VerbList, Noun: "thing", ParentId: "parent-1"} table := tui.NewTable(nil, 5, 40) rows := make([]tui.Row, 10) @@ -133,12 +133,12 @@ func TestCurrentScreenLink_CapturesListPosOnTable(t *testing.T) { fm := uiTableModel{t: table} link := currentScreenLink(ctx, fm) - if link.ListPos != 4 { - t.Fatalf("ListPos = %d, want 4", link.ListPos) + if link.Offset != 4 { + t.Fatalf("Offset = %d, want 4", link.Offset) } } -func TestCurrentScreenLink_CapturesListPosEvenMidDetailFlip(t *testing.T) { +func TestCurrentScreenLink_CapturesOffsetEvenMidDetailFlip(t *testing.T) { // "b" always resumes the underlying list, never the detail overlay, so an // in-place detail flip (detailMode true, detailOnly false) over a table must // still capture that table's live cursor. @@ -153,30 +153,36 @@ func TestCurrentScreenLink_CapturesListPosEvenMidDetailFlip(t *testing.T) { fm := uiTableModel{t: table, detailMode: true, detailOnly: false} link := currentScreenLink(ctx, fm) - if link.ListPos != 4 { - t.Fatalf("ListPos = %d, want 4 (mid-flip should still capture the list cursor)", link.ListPos) + if link.Offset != 4 { + t.Fatalf("Offset = %d, want 4 (mid-flip should still capture the list cursor)", link.Offset) } } -func TestCurrentScreenLink_DetailOnlyScreenHasZeroListPos(t *testing.T) { +func TestCurrentScreenLink_DetailOnlyScreenHasZeroOffset(t *testing.T) { // Case 4 detail-only Hops never populate fm.t, so its Cursor() is naturally 0. ctx := &cmdctx.Ctx{Verb: VerbGet, Noun: "thing", Id: "child-1"} fm := uiTableModel{detailMode: true, detailOnly: true} link := currentScreenLink(ctx, fm) - if link.ListPos != 0 { - t.Fatalf("ListPos = %d, want 0 (detail-only screens have no underlying table)", link.ListPos) + if link.Offset != 0 { + t.Fatalf("Offset = %d, want 0 (detail-only screens have no underlying table)", link.Offset) } } -func TestCurrentScreenLink_CapturesPage(t *testing.T) { +func TestCurrentScreenLink_CapturesOffsetAcrossPages(t *testing.T) { ctx := &cmdctx.Ctx{Verb: VerbList, Noun: "thing", ParentId: "parent-1"} table := tui.NewTable(nil, 5, 40) - fm := uiTableModel{t: table, page: 2} + rows := make([]tui.Row, 10) + for i := range rows { + rows[i] = tui.Row{"x"} + } + table.SetRows(rows) + table.SetCursor(3) + fm := uiTableModel{t: table, page: 2, pageSize: 20} link := currentScreenLink(ctx, fm) - if link.Page != 2 { - t.Fatalf("Page = %d, want 2", link.Page) + if link.Offset != 43 { + t.Fatalf("Offset = %d, want 43 (page 2 * pageSize 20 + cursor 3)", link.Offset) } } @@ -297,59 +303,66 @@ func TestFinishUIExit_NoHopNoPush(t *testing.T) { } } -func TestBuildLinkCtx_TableScreen_CarriesListPosToRestoreListPos(t *testing.T) { +func TestBuildLinkCtx_TableScreen_CarriesOffsetToRestoreOffset(t *testing.T) { ctx := &cmdctx.Ctx{Context: context.Background(), Resolver: New()} - link := &cmdctx.UILink{Verb: VerbList, Noun: "thing", Id: "parent-1", Screen: cmdctx.ScreenTable, ListPos: 4} + link := &cmdctx.UILink{Verb: VerbList, Noun: "thing", Id: "parent-1", Screen: cmdctx.ScreenTable, Offset: 44} targetCs := &spec.CommandSpec{Verb: VerbList, Noun: "thing", NoAuth: true} newCtx, err := buildLinkCtx(ctx, link, targetCs) if err != nil { t.Fatalf("buildLinkCtx: %v", err) } - if newCtx.RestoreListPos != 4 { - t.Fatalf("RestoreListPos = %d, want 4", newCtx.RestoreListPos) + if newCtx.RestoreOffset != 44 { + t.Fatalf("RestoreOffset = %d, want 44", newCtx.RestoreOffset) } if newCtx.ParentId != "parent-1" { t.Fatalf("ParentId = %q, want parent-1", newCtx.ParentId) } } -func TestBuildLinkCtx_TableScreen_CarriesPageToRestorePage(t *testing.T) { +func TestBuildLinkCtx_DetailScreen_DoesNotSetRestoreOffset(t *testing.T) { ctx := &cmdctx.Ctx{Context: context.Background(), Resolver: New()} - link := &cmdctx.UILink{Verb: VerbList, Noun: "thing", Id: "parent-1", Screen: cmdctx.ScreenTable, ListPos: 4, Page: 2} - targetCs := &spec.CommandSpec{Verb: VerbList, Noun: "thing", NoAuth: true} - - newCtx, err := buildLinkCtx(ctx, link, targetCs) - if err != nil { - t.Fatalf("buildLinkCtx: %v", err) - } - if newCtx.RestorePage != 2 { - t.Fatalf("RestorePage = %d, want 2", newCtx.RestorePage) - } -} - -func TestBuildLinkCtx_DetailScreen_DoesNotSetRestoreListPos(t *testing.T) { - ctx := &cmdctx.Ctx{Context: context.Background(), Resolver: New()} - link := &cmdctx.UILink{Verb: VerbGet, Noun: "thing", Id: "child-1", Screen: cmdctx.ScreenDetailForGet, ListPos: 4} + link := &cmdctx.UILink{Verb: VerbGet, Noun: "thing", Id: "child-1", Screen: cmdctx.ScreenDetailForGet, Offset: 4} targetCs := &spec.CommandSpec{Verb: VerbGet, Noun: "thing", NoAuth: true} newCtx, err := buildLinkCtx(ctx, link, targetCs) if err != nil { t.Fatalf("buildLinkCtx: %v", err) } - if newCtx.RestoreListPos != 0 { - t.Fatalf("RestoreListPos = %d, want 0 (detail screens have no list cursor to restore)", newCtx.RestoreListPos) + if newCtx.RestoreOffset != 0 { + t.Fatalf("RestoreOffset = %d, want 0 (detail screens have no list cursor to restore)", newCtx.RestoreOffset) } } -func TestNewUITableModel_SeedsPageFromCtxRestorePage(t *testing.T) { - ctx := &cmdctx.Ctx{RestorePage: 2, RestoreListPos: 4} +func TestNewUITableModel_SeedsPageAndCursorFromCtxRestoreOffset(t *testing.T) { + // termHeight 24 -> pageSize = tableHeight(24) = 24-uiOverheadLines-1. Derive + // the expected pageSize the same way the model does, so this test doesn't + // hardcode uiOverheadLines. + pageSize := tableHeight(24) + offset := 2*pageSize + 4 + ctx := &cmdctx.Ctx{RestoreOffset: offset} m := newUITableModel(ctx, nil, nil, nil, nil, "title", 80, 24, nil) if m.page != 2 { - t.Fatalf("page = %d, want 2 (seeded from ctx.RestorePage)", m.page) + t.Fatalf("page = %d, want 2 (derived from ctx.RestoreOffset / pageSize)", m.page) } if m.restoreCursor != 4 { - t.Fatalf("restoreCursor = %d, want 4 (seeded from ctx.RestoreListPos)", m.restoreCursor) + t.Fatalf("restoreCursor = %d, want 4 (derived from ctx.RestoreOffset %% pageSize)", m.restoreCursor) + } +} + +func TestNewUITableModel_RestoreOffsetSurvivesPageSizeChange(t *testing.T) { + // Capture at one pageSize, restore at a different (e.g. post-resize) + // pageSize, and confirm the absolute row is still targeted correctly. + capturedPageSize := 10 + capturedPage, capturedCursor := 2, 3 + offset := capturedPage*capturedPageSize + capturedCursor // absolute row 23 + + ctx := &cmdctx.Ctx{RestoreOffset: offset} + m := newUITableModel(ctx, nil, nil, nil, nil, "title", 80, 24, nil) + + restoredPageSize := m.pageSize + if got := m.page*restoredPageSize + m.restoreCursor; got != offset { + t.Fatalf("restored absolute row = %d, want %d (offset must survive pageSize change)", got, offset) } } From 87605f5052f2aa8dbc9db25c0a510a906d9bc2ca Mon Sep 17 00:00:00 2001 From: Naman-Nirwan Date: Wed, 9 Sep 2026 01:22:15 +0530 Subject: [PATCH 5/6] formatting problem resolved --- pkg/registry/uitableview.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/registry/uitableview.go b/pkg/registry/uitableview.go index e39f686..b9a977f 100644 --- a/pkg/registry/uitableview.go +++ b/pkg/registry/uitableview.go @@ -1281,7 +1281,7 @@ func currentScreenLink(ctx *cmdctx.Ctx, fm uiTableModel) cmdctx.UILink { Org: org, Project: project, FlagValues: fv, - Screen: screen, + Screen: screen, // Offset is always captured from the underlying table, even mid detail-flip: // "b" always resumes the list, never the detail overlay, and detailOnly // screens (Case 4) never populate fm.t, so its Cursor() is a natural 0 there. From 7580c85fab72ce98210b722d087a5847e5dc80c2 Mon Sep 17 00:00:00 2001 From: Naman-Nirwan Date: Thu, 10 Sep 2026 18:26:30 +0530 Subject: [PATCH 6/6] made the file flag optional, because of weird yaml structures are possible, we need set and del --- pkg/spec/gitops.spec.yaml | 77 +++++++++++++++++++++++++++++++++------ 1 file changed, 65 insertions(+), 12 deletions(-) diff --git a/pkg/spec/gitops.spec.yaml b/pkg/spec/gitops.spec.yaml index 730e4af..805117d 100644 --- a/pkg/spec/gitops.spec.yaml +++ b/pkg/spec/gitops.spec.yaml @@ -137,6 +137,7 @@ nouns: fields: - id: name expr: it.name + mutable_path: metadata.name - id: agent expr: it.agentIdentifier - id: cluster @@ -147,6 +148,11 @@ nouns: expr: it.app.spec.project - id: namespace expr: it.app.spec.destination.namespace + mutable_path: spec.destination.namespace + - id: destination_server + label: Destination Server + expr: it.app.spec.destination.server + mutable_path: spec.destination.server - id: sync_status label: Sync expr: it.app.status.sync.status @@ -159,10 +165,13 @@ nouns: label: Repo URL expr: it.app.spec.source.repoURL width_max: 60 + mutable_path: spec.source.repoURL - id: path expr: it.app.spec.source.path + mutable_path: spec.source.path - id: target_revision expr: it.app.spec.source.targetRevision + mutable_path: spec.source.targetRevision - id: created expr: it.createdAt ?? it.app.creationTimestampTs - id: updated @@ -177,11 +186,17 @@ nouns: expr: it.identifier - id: name expr: it.cluster.name + mutable_path: name - id: agent expr: it.agentIdentifier - id: server expr: it.cluster.server width_max: 60 + mutable_path: server + - id: connection_type + label: Connection Type + expr: it.cluster.config.clusterConnectionType ?? "" + mutable_path: config.clusterConnectionType - id: connection label: Connection expr: it.cluster.connectionState.status @@ -203,14 +218,17 @@ nouns: expr: it.identifier - id: name expr: 'it.repository.name != "" ? it.repository.name : it.identifier' + mutable_path: name - id: agent expr: it.agentIdentifier - id: repo_url label: Repo URL expr: it.repository.repo width_max: 60 + mutable_path: repo - id: type expr: it.repository.type + mutable_path: type - id: connection label: Connection expr: it.repository.connectionState.status @@ -220,6 +238,7 @@ nouns: - id: connection_type label: Connection Type expr: it.repository.connectionType ?? "" + mutable_path: connectionType - id: org_id expr: it.orgIdentifier - id: project_id @@ -419,8 +438,10 @@ commands: verb: create noun: gitops_application requires_id: true - short: "Create a GitOps application: harness create gitops_application -f app.yaml --cluster [--repo ]" + short: "Create a GitOps application: harness create gitops_application --set name= repo_url= path= destination_server= namespace= --cluster [--repo ], or -f app.yaml" handler_type: endpoint + flags_builtin: + set: true flags: - name: cluster description: "GitOps cluster identifier (required)" @@ -433,7 +454,10 @@ commands: endpoint: method: POST path: /gitops/api/v1/agents/{{ctx.id}}/applications - file_body: required + file_body: optional + create_strategy: set-fields + create_body_init: + metadata.name: coalesce(ctx.setArgs.name, ctx.id) create_body_wrap: application query_params: orgIdentifier: 'auth.org != "" ? auth.org : nil' @@ -449,8 +473,11 @@ commands: noun: gitops_application id_parts: 2 id_label: "" - short: "Update a GitOps application: harness update gitops_application -f app.yaml [--cluster ] [--repo ]" + short: "Update a GitOps application: harness update gitops_application --set repo_url= path= namespace= [--cluster ] [--repo ], or -f app.yaml" handler_type: endpoint + flags_builtin: + set: true + del: true flags: - name: cluster description: "GitOps cluster identifier (when changing cluster/destination binding)" @@ -462,7 +489,10 @@ commands: endpoint: method: PUT path: /gitops/api/v1/agents/{{ctx.idParts[0]}}/applications/{{ctx.idParts[1]}} - file_body: required + file_body: optional + update_strategy: get-then-put + get_path: /gitops/api/v1/agents/{{ctx.idParts[0]}}/applications/{{ctx.idParts[1]}} + update_body_pick: "{metadata: {name: it.app.metadata.name, labels: it.app.metadata.labels ?? {}}, spec: {source: it.app.spec.source, destination: it.app.spec.destination, project: it.app.spec.project}}" update_body_wrap: application query_params: orgIdentifier: 'auth.org != "" ? auth.org : nil' @@ -615,8 +645,10 @@ commands: noun: gitops_cluster id_parts: 2 id_label: "" - short: "Register a cluster on a GitOps agent: harness create gitops_cluster -f cluster.yaml [--upsert]" + short: "Register a cluster on a GitOps agent: harness create gitops_cluster --set server= connection_type=IN_CLUSTER, or -f cluster.yaml [--upsert]" handler_type: endpoint + flags_builtin: + set: true flags: - name: upsert description: Update existing cluster if identifier already exists @@ -624,7 +656,10 @@ commands: endpoint: method: POST path: /gitops/api/v1/agents/{{ctx.idParts[0]}}/clusters - file_body: required + file_body: optional + create_strategy: set-fields + create_body_init: + name: coalesce(ctx.setArgs.name, ctx.idParts[1]) create_body_wrap: cluster query_params: identifier: ctx.idParts[1] @@ -641,8 +676,11 @@ commands: noun: gitops_cluster id_parts: 2 id_label: "" - short: "Update a GitOps cluster: harness update gitops_cluster -f cluster.yaml [--force-update]" + short: "Update a GitOps cluster: harness update gitops_cluster --set server= connection_type=IN_CLUSTER, or -f cluster.yaml [--force-update]" handler_type: endpoint + flags_builtin: + set: true + del: true flags: - name: force-update description: Force update even when server-side validation would reject changes @@ -650,7 +688,10 @@ commands: endpoint: method: PUT path: /gitops/api/v1/agents/{{ctx.idParts[0]}}/clusters/{{ctx.idParts[1]}} - file_body: required + file_body: optional + update_strategy: get-then-put + get_path: /gitops/api/v1/agents/{{ctx.idParts[0]}}/clusters/{{ctx.idParts[1]}} + update_body_pick: "{name: it.cluster.name, server: it.cluster.server, config: it.cluster.config}" update_body_wrap: cluster query_params: orgIdentifier: 'auth.org != "" ? auth.org : nil' @@ -733,8 +774,10 @@ commands: noun: gitops_repository id_parts: 2 id_label: "" - short: "Register a Git repo on a GitOps agent: harness create gitops_repository -f repo.yaml [--upsert] [--repo-creds-id ]" + short: "Register a Git repo on a GitOps agent: harness create gitops_repository --set repo_url= type=git connection_type=HTTPS_ANONYMOUS, or -f repo.yaml [--upsert] [--repo-creds-id ]" handler_type: endpoint + flags_builtin: + set: true flags: - name: upsert description: Update existing repository if identifier already exists @@ -744,7 +787,11 @@ commands: endpoint: method: POST path: /gitops/api/v1/agents/{{ctx.idParts[0]}}/repositories - file_body: required + file_body: optional + create_strategy: set-fields + create_body_init: + name: coalesce(ctx.setArgs.name, ctx.idParts[1]) + type: coalesce(ctx.setArgs.type, '"git"') create_body_wrap: repo query_params: identifier: ctx.idParts[1] @@ -762,12 +809,18 @@ commands: noun: gitops_repository id_parts: 2 id_label: "" - short: "Update a GitOps repository: harness update gitops_repository -f repo.yaml" + short: "Update a GitOps repository: harness update gitops_repository --set repo_url= type=git connection_type=HTTPS_ANONYMOUS, or -f repo.yaml" handler_type: endpoint + flags_builtin: + set: true + del: true endpoint: method: PUT path: /gitops/api/v1/agents/{{ctx.idParts[0]}}/repositories/{{ctx.idParts[1]}} - file_body: required + file_body: optional + update_strategy: get-then-put + get_path: /gitops/api/v1/agents/{{ctx.idParts[0]}}/repositories/{{ctx.idParts[1]}} + update_body_pick: "{name: it.repository.name, repo: it.repository.repo, type: it.repository.type, connectionType: it.repository.connectionType}" update_body_wrap: repo query_params: orgIdentifier: 'auth.org != "" ? auth.org : nil'