Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/gop/gop0.md
Original file line number Diff line number Diff line change
Expand Up @@ -59,3 +59,4 @@ The numerical index contains a table of all GOPs, ordered by number.
| 1 | Core CLI Primitives | Active | Informational | Core |
| 2 | Login Expression Detection in Execution Loop for Console Streamers | Accepted | Standards Track | Drivers/Transports |
| 3 | Repeated Question Detection in Execution Loop | Accepted | Standards Track | Core |
| 4 | Recovering Command Echo After a Transient Prompt | Accepted | Standards Track | Core |
95 changes: 95 additions & 0 deletions docs/gop/gop4.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
# GOP 4 – Recovering Command Echo After a Transient Prompt

- **Author:** Reydant
- **Status:** Accepted
- **Type:** Standards Track
- **Topic:** Core
- **Created:** 2026-08-12
- **Last-Modified:** 2026-08-12

## Abstract

Some interactive CLIs redraw their prompt while they receive a command. If a transport read ends at that redraw, `GenericExecute` can match the prompt before it has observed the complete command echo and return an `EchoReadException`.

This GOP adds recovery for that condition. When a prompt is matched before the command echo, gnetcli preserves the consumed bytes by prepending them to the connector's unread buffer, then continues reading. This lets the normal echo matcher evaluate the preserved bytes together with later terminal output.

## Motivation

RouterOS redraws the command line while each character is being entered. A stream can contain data such as:

```
\r[robot@router] > /user exp
```

At a network-read boundary, the `\r[robot@router] > ` part is indistinguishable from a final prompt. The command echo is incomplete, however, and more redraw data or the submitted command echo may arrive later.

Previously, matching this prompt before the expected echo caused command execution to fail immediately with `echo read error`.

## Failure dialog from RouterOS logs

The following is a shortened and sanitized dialog from the original failure. Runs of terminal-padding spaces are replaced with `…`; `\x1b[K` clears the rest of the line.

```text
client -> /user export\n

device -> \r[robot@router] > /user exp\r\r[robot@router] > <space>
^ prompt matcher stops here

device -> \x1b[K\r[robot@router] > /user ex\r\r[robot@router] > /user ex…
device -> \x1b[K\r[robot@router] > /user exp\r\r[robot@router] > /user exp…
```

The first read ends immediately after a redraw prompt. At that point `/user exp` is only a partial line, so it cannot match the expected `/user export\r\n` echo. Before GOP 4, `GenericExecute` returned `EchoReadException` at the marked prompt and never read the later redraw data.

## Proposal

`streamer.Connector` provides:

```go
PrependBuffer(data []byte) error
```

Each connector prepends `data` to its internal unread buffer. The next `Read` or `ReadTo` consumes these bytes before reading from the network.

During `GenericExecute`:

1. gnetcli writes the command and waits for echo, prompt, pager, question, login, and callbacks as usual.
2. If the complete echo is matched, execution proceeds normally.
3. If a prompt is matched before echo, gnetcli attempts to reconstruct the echo from the bytes before that prompt.
4. If reconstruction fails, gnetcli records the associated `EchoReadException`, prepends those bytes through `PrependBuffer`, and continues reading with the same expression set.
5. If the complete echo later matches, normal execution resumes. If reading ends or times out first, gnetcli returns the saved echo error; a device error detected in a read timeout still takes precedence.

The behavior applies to all `streamer.Connector` implementations. SSH returns an error if `PrependBuffer` is called before its shell session is initialized; `GenericExecute` propagates that failure.

## Compatibility

- Normal command execution is unchanged when echo is received before prompt.
- `Read(n)` retains its existing meaning: it returns immediately when at least `n` bytes are already buffered, including prepended bytes. This is required for protocol readers such as NETCONF chunk framing.
- `PrependBuffer` is a new method on the public `streamer.Connector` interface. Third-party connector implementations must add it.

## Rationale

The connector already owns unread transport bytes. Returning consumed pre-prompt data to that buffer preserves the existing command-execution matcher order and avoids adding a separate pre-echo state machine to `GenericExecute`.

The alternative of disabling prompt matching until echo is found avoids the premature match, but changes matching behavior and requires managing separate matcher states. Buffer restoration instead retries with the original expressions and preserves the normal execution flow.

## Alternatives

### Explicit pre-echo state machine

`GenericExecute` could track separate phases such as `awaitEchoWithPrompt`, `awaitEchoWithoutPrompt`, and `afterEcho`. After a prompt arrives before echo, it would rebuild its expression list without the prompt matcher; after echo is found, it would rebuild the normal prompt-first matcher.

This makes the execution phases explicit, but it adds state transitions, duplicates matcher construction, and requires care to restore the correct matcher after every transition, including questions and callbacks. It also changes the expressions that are active during execution.

The accepted approach keeps the existing matcher set and returns consumed bytes to the transport's unread buffer. The next read evaluates the original expressions against the complete reconstructed stream, so the recovery logic stays localized to the prompt-before-echo case.

## Testing Plan

- RouterOS mock tests split a redraw sequence at a prompt boundary and verify that the complete command echo is later recognized.
- Streamer tests verify that buffered data is consumed before network data and that `Read(n)` preserves its total-buffer size semantics.

## References

- `pkg/device/genericcli/genericcli.go` — `GenericExecute` echo recovery.
- `pkg/streamer/streamer.go` — `Connector` and unread-buffer handling.
- `pkg/device/ros/device_mock_test.go` — RouterOS redraw regression test.
2 changes: 1 addition & 1 deletion internal/gvendor/gvendor.go
Original file line number Diff line number Diff line change
Expand Up @@ -459,7 +459,7 @@ func (m *connWrapper) Close() error {

func (m *connWrapper) ReadTo(ctx context.Context, expr expr.Expr) (streamer.ReadRes, error) {
m.log.Debug("read to", zap.String("expr", expr.Repr()))
res, extra, read, err := streamer.GenericReadX(ctx, m.stdoutBufferExtra, m.stdoutBuffer, defaultReadSize, readTimeout, expr, 0, 0)
res, extra, read, err := streamer.GenericReadX(ctx, m.stdoutBufferExtra, m.stdoutBuffer, defaultReadSize, readTimeout, streamer.WithRegExpr(expr))
m.stdoutBufferExtra = extra
if err != nil {
return nil, err
Expand Down
17 changes: 12 additions & 5 deletions pkg/device/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,9 @@ func ThrowExecException(data string) error {
}

type EchoReadException struct {
lastRead []byte
promptFound bool // indicates if we found prompt after echo read error
lastRead []byte
promptFound bool // indicates if we found prompt after echo read error
questionFound bool // indicates if we found question after echo error
}

func (e *EchoReadException) Error() string {
Expand All @@ -39,10 +40,16 @@ func (e *EchoReadException) PromptFound() bool {
return e.promptFound
}

func ThrowEchoReadException(lastRead []byte, promptFound bool) error {
// QuestionFound indicates if gnetcli succeeded in reading question after echo read failure
func (e *EchoReadException) QuestionFound() bool {
return e.questionFound
}

func ThrowEchoReadException(lastRead []byte, promptFound bool, questionFound bool) error {
return &EchoReadException{
lastRead: lastRead,
promptFound: promptFound,
lastRead: lastRead,
promptFound: promptFound,
questionFound: questionFound,
}
}

Expand Down
123 changes: 74 additions & 49 deletions pkg/device/genericcli/genericcli.go
Original file line number Diff line number Diff line change
Expand Up @@ -593,6 +593,8 @@ func GenericExecute(command cmd.Cmd, connector streamer.Connector, cli GenericCL
if len(cmdQuestions) > 0 {
questions = append(cmdQuestions, questions...)
}

exprsAdd, exprsAddMap := command.GetExprCallback()
checkExprs := []expr.NamedExpr{
{Name: echoExprName, Exprs: []expr.Expr{expCmdEcho}},
{Name: promptExprName, Exprs: []expr.Expr{cli.prompt}},
Expand All @@ -603,14 +605,18 @@ func GenericExecute(command cmd.Cmd, connector streamer.Connector, cli GenericCL
checkExprs = append(checkExprs, expr.NamedExpr{Name: loginExprName, Exprs: []expr.Expr{cli.login}})
}
exprs := expr.NewSimpleExprListNamedOrdered(checkExprs)

exprsAdd, exprsAddMap := command.GetExprCallback()
callbackPatternStart := 0
for _, namedExpr := range checkExprs {
callbackPatternStart += len(namedExpr.Exprs)
}
for _, exprCB := range exprsAdd {
exprs.Add("cb", expr.NewSimpleExpr().FromPattern(exprCB))
}

cbLimit := 100
seenEcho := false
var lastQuestion []byte // GOP3
var lastPromptBeforeEchoError error
repeatedQuestionCount := 0
for { // pager loop
match, err := connector.ReadTo(ctx, exprs)
Expand All @@ -623,6 +629,15 @@ func GenericExecute(command cmd.Cmd, connector streamer.Connector, cli GenericCL
return nil, outputErr
}
}
// This case means we got prompt without echo previously.
// This could mean 2 separate problems, which are hard to distinguish:
// 1) Device redraws terminal, partially echoing; we got chunk ending on prompt before device wrote full echo
// 2) Prompt/echo is configured incorrect for this device.
// For the 1) case we prepend existing buffer and retry read until we read echo. If we receive read error - it was actually 2) case - so we return original error.
// gop4 for more details.
if lastPromptBeforeEchoError != nil {
return nil, lastPromptBeforeEchoError
}
return nil, err
}
matchId := match.GetPatternNo()
Expand All @@ -631,65 +646,71 @@ func GenericExecute(command cmd.Cmd, connector streamer.Connector, cli GenericCL
if matchName == echoExprName {
seenEcho = true
exprs.Delete(echoExprName)
callbackPatternStart--
lastPromptBeforeEchoError = nil
continue
}
mbefore := match.GetBefore()
if !seenEcho {
if matchName == questionExprName { // caught question before echo
// check for echo, drop it and proceed with question
termParsedEcho, err := terminal.ParseDropLastReturn(mbefore)
if err != nil {
return nil, fmt.Errorf("echo terminal parse error %w", err)
}
mres, ok := exprs.Match(termParsedEcho)
if !ok {
return nil, device.ThrowEchoReadException(mbefore, true)
}
if exprs.GetName(mres.PatternNo) == echoExprName {
seenEcho = true
}
mbefore = termParsedEcho[mres.End:]
}
}

if !seenEcho {
promptFound := matchName == promptExprName
// case where we caught prompt before echo because of term codes in echo
if len(mbefore) < 2 || !promptFound { // don't bother to do complex logic
return nil, device.ThrowEchoReadException(mbefore, promptFound)
}

termParsedEcho, err := terminal.ParseDropLastReturn(mbefore)
seenPrompt := matchName == promptExprName
seenQuestion := matchName == questionExprName
checkEcho := func(mBefore []byte) ([]byte, error) {
// Check for echo, drop it, and continue handling the matched expression.
termParsedEcho, err := terminal.ParseDropLastReturn(mBefore)
if err != nil {
return nil, fmt.Errorf("echo terminal parse error %w", err)
return nil, fmt.Errorf("echo terminal before question parse error %w", err)
}
mres, ok := exprs.Match(termParsedEcho)
if !ok {
// prompt expression may consume newline from echo, but it must be presented in echo
if mbefore[len(mbefore)-1] != '\n' {
mbefore = append(mbefore, '\n')
}
termParsedEcho, err = terminal.ParseDropLastReturn(mbefore)
if err != nil {
return nil, fmt.Errorf("echo terminal parse error %w", err)
}
mres, ok = exprs.Match(termParsedEcho)
if !ok {
return nil, device.ThrowEchoReadException(mbefore, promptFound)
}
return nil, device.ThrowEchoReadException(mbefore, seenPrompt, seenQuestion)
}
// assuring that it is echo
if exprs.GetName(mres.PatternNo) != echoExprName {
return nil, device.ThrowEchoReadException(mbefore, promptFound)
}
if mres.End > len(termParsedEcho) {
return nil, errors.New("termParsedEcho len less than mres.End")
return nil, device.ThrowEchoReadException(mbefore, seenPrompt, seenQuestion)
}
seenEcho = true
exprs.Delete(echoExprName)
// delete echo
mbefore = termParsedEcho[mres.End:]
callbackPatternStart--
lastPromptBeforeEchoError = nil
return termParsedEcho[mres.End:], nil
}
if !seenEcho {
if len(mbefore) < 2 {
return nil, device.ThrowEchoReadException(mbefore, seenPrompt, seenQuestion)
}
switch {
case matchName == questionExprName:
// check for echo, drop it and proceed with question
mBefore, err := checkEcho(mbefore)
if err != nil {
return nil, err
}
mbefore = mBefore
case matchName == promptExprName:
mBefore, err := checkEcho(mbefore)
// prompt expression may consume newline from echo, but it must be presented in echo
if err != nil && mbefore[len(mbefore)-1] != '\n' {
mBefore, err = checkEcho(append(mbefore, '\n'))
}
// GOP 4: a CLI may redraw its prompt before the complete echo arrives.
// Preserve consumed bytes and retry the normal matcher with them prepended.
if err != nil {
lastPromptBeforeEchoError = err
err := connector.PrependBuffer(mbefore)
if err != nil {
return nil, fmt.Errorf(
"prepend consumed output after prompt-before-echo: %w; %w",
err,
device.ThrowEchoReadException(mbefore, true, false),
)
}
continue
}

mbefore = mBefore
default:
return nil, device.ThrowEchoReadException(mbefore, false, false)
}
}

if matchName == promptExprName {
buffer.Write(mbefore)
if store, ok := match.GetMatchedGroups()["store"]; ok {
Expand Down Expand Up @@ -732,7 +753,11 @@ func GenericExecute(command cmd.Cmd, connector streamer.Connector, cli GenericCL
return nil, fmt.Errorf("callback limit")
}
cbLimit--
wr := exprsAddMap[exprsAdd[matchId-3]]
callbackIndex := matchId - callbackPatternStart
if callbackIndex < 0 || callbackIndex >= len(exprsAdd) {
return nil, fmt.Errorf("invalid callback pattern index %d", matchId)
}
wr := exprsAddMap[exprsAdd[callbackIndex]]
logger.Debug("write callback result")
err := connector.Write([]byte(wr))
if err != nil {
Expand Down
30 changes: 30 additions & 0 deletions pkg/device/genericcli/genericcli_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -314,3 +314,33 @@ func TestQuestionWithAnswerNotSendNL(t *testing.T) {
require.NoError(t, resErr)
require.Equal(t, cmdRes, []cmd.CmdRes{cmd.NewCmdRes(nil)})
}

func TestCommandCallbackWithAdditionalQuestionExpression(t *testing.T) {
logger := zap.Must(zap.NewDevelopmentConfig().Build())
dialog := [][]gmock.Action{
{
gmock.Send("<device>"),
gmock.Expect("test\n"),
gmock.SendEcho("test\r\n"),
gmock.Send("callback"),
gmock.Expect("answer"),
gmock.Send("done\r\n<device>"),
gmock.Close(),
},
}

command := cmd.NewCmd(
"test",
cmd.WithAddAnswers(cmd.NewAnswer("unused-question", "unused", true)),
cmd.WithExprCallback(cmd.NewExprCallback("callback", "answer")),
)
cmdRes, resErr, serverErr, err := gmock.RunCmd(func(connector streamer.Connector) device.Device {
dev := newDevice(fullQuestion, connector, logger)
return &dev
}, gmock.ConcatMultipleSlices(dialog), []cmd.Cmd{command}, logger)

require.NoError(t, err)
require.NoError(t, serverErr)
require.NoError(t, resErr)
require.Equal(t, []cmd.CmdRes{cmd.NewCmdRes([]byte("done"))}, cmdRes)
}
2 changes: 1 addition & 1 deletion pkg/device/juniper/device_mock_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ func TestInvalidShowCommandsWithException(t *testing.T) {
},
everyDayByeBye,
},
err: device.ThrowEchoReadException([]byte("dis \r\n ^\r\nunknown command.\r\n"), true),
err: device.ThrowEchoReadException([]byte("dis \r\n ^\r\nunknown command.\r\n"), true, false),
},
}

Expand Down
Loading
Loading